Skip to content

Tidy some comments #3741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Tidy some comments #3741

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6198a91
Tidy some comments:
srawlins Apr 3, 2024
f33ae35
feedback
srawlins Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions lib/src/model/documentation_comment.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,6 @@ mixin DocumentationComment

List<DocumentationComment>? _documentationFrom;

/// The [ModelElement](s) from which we will get documentation.
///
/// Can be more than one if this is a [Field] composing documentation from
/// multiple [Accessor]s.
///
/// This will walk up the inheritance hierarchy to find docs, if the current
/// class doesn't have docs for this element.
@override
List<DocumentationComment> get documentationFrom =>
_documentationFrom ??= () {
Expand Down Expand Up @@ -72,11 +65,11 @@ mixin DocumentationComment
/// like `///`, `//`, `/*`, `*/`.
String get documentationComment => element.documentationComment ?? '';

/// True if [this] has a synthetic/inherited or local documentation
/// comment. False otherwise.
/// Whether `this` has a synthetic/inherited or local documentation comment,
/// and false otherwise.
bool get hasDocumentationComment => element.documentationComment != null;

/// Returns true if the raw documentation comment has a 'nodoc' indication.
/// Whether the raw documentation comment has a 'nodoc' indication.
Copy link
Member

@kallentu kallentu Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of saying "has a 'nodoc' indication", can we just write out what 'nodoc' means and what it does? Otherwise, someone reading it would have to search to find what 'nodoc' is anyways.

Copy link
Member Author

@srawlins srawlins Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that is weird verbiage. Done!

late final bool hasNodoc = () {
if (packageGraph.configSetsNodocFor(element.source!.fullName)) {
return true;
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/inheritable.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ mixin Inheritable on ContainerMember {

Inheritable? get overriddenElement;

/// True if this [Inheritable] is overriding a superclass.
/// Whether this [Inheritable] is overriding a member from a superclass.
Copy link
Member

@kallentu kallentu Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also mention the relationship between isOverride and isInherited like you mentioned before in GVC? Might be good information for later.

Copy link
Member Author

@srawlins srawlins Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

late final bool isOverride = () {
// The canonical version of the enclosing element -- not
// [canonicalEnclosingElement], as that is the element enclosing the
Expand Down
7 changes: 7 additions & 0 deletions lib/src/model/locatable.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ import 'package:analyzer/dart/element/element.dart' show Element;

/// Something that can be located for warning purposes.
mixin Locatable {
/// The model element(s) from which we will get documentation.
Copy link
Member

@kallentu kallentu Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not link [ModelElement] and the other references in backticks like the original comment?

Copy link
Member Author

@srawlins srawlins Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, I don't remember why I changed this, but it's not correct anyhow; they're Locatables, including some not-ModelElement things, like Category. Changed.

///
/// Can be more than one if this is a `Field` composing documentation from
/// multiple `Accessor`s.
///
/// This will walk up the inheritance hierarchy to find docs, if the current
/// class doesn't have docs for this element.
List<Locatable> get documentationFrom;

/// True if documentationFrom contains only one item, [this].
Expand Down