Skip to content
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.

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

Add a hideConstantImplementations dartdoc directive #3398

Merged
merged 18 commits into from
May 4, 2023

Conversation

jcollins-g
Copy link
Contributor

Fixes #2657.

Without trying to solve #3397 this PR does also move a few things around to make it more clear where the various directives are currently implemented as adding yet another one via buildDocumentationAddition was starting to look messy.

Here is a sample screenshot of what Icons would look like with the new directive:

Screenshot 2023-04-25 at 2 05 39 PM

```dart
/// This is truly an amazing library.
/// {@hideConstantImplementations}
library my_library;
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think we're steering away from named libraries like this; we can use library; in Dart 2.19.


final _canonicalRegExp = RegExp(r'{@canonicalFor\s([^}]+)}');

/// Used by [Library] to implement the canonicalFor directive.
Copy link
Member

Choose a reason for hiding this comment

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

Can we wrap 'canonicalFor' in some delimiter like backticks?

/// Mixin implementing dartdoc categorization for ModelElements.
mixin Categorization implements ModelElement {
/// Mixin parsing the `@category` directive for ModelElements.
mixin Categorization on DocumentationComment implements Indexable {
Copy link
Member

Choose a reason for hiding this comment

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

Nice cleanup!


/// [true] if the {@hideConstantImplementations} dartdoc directive is present
/// in the documentation for this class.
bool get hideConstantImplementations {
Copy link
Member

Choose a reason for hiding this comment

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

The name here makes it sound like a function, like calling hideConstantImplementations will do something or set something. What about hasHideConstantImplementations, like how the analyzer APIs are named?

https://dart.dev/guides/language/effective-dart/design#prefer-a-non-imperative-verb-phrase-for-a-boolean-property-or-variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return _hideConstantImplementations!;
}

/// Hides [hideConstantImplementations] from doc while leaving a note to
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, is this reference right? This function doesn't hide the bool getter, hideConstantImplementations. Maybe "Hides {@hideConstantImplementations} ..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

lib/src/model/documentation_comment.dart Show resolved Hide resolved
@@ -266,6 +267,10 @@ mixin GetterSetterCombo on ModelElement {

bool get writeOnly => hasPublicSetter && !hasPublicGetter;

/// True if the @hideConstantImplementations directive is present
/// in the defining enclosing element.
bool get hideConstantImplementation;
Copy link
Member

Choose a reason for hiding this comment

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

Same naming for here, maybe hasHide... or shouldHide...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@jcollins-g jcollins-g merged commit 24af8a1 into dart-lang:main May 4, 2023
@jcollins-g jcollins-g deleted the hide-implementations branch May 4, 2023 16:54
@jcollins-g
Copy link
Contributor Author

The grind.dart changes are due to some yaks that needed shaving: the name ProcessException was being excessively reused and there was a bug causing reentrancy in the flutter repo setup for grind compare-flutter-warnings which I use for benchmarking and general comparison tasks between dartdoc revisions.

auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 8, 2023
…c pages (#128442)

This updates dartdoc to 6.3.0.  Release notes are available, here:

https://github.com/dart-lang/dartdoc/releases/tag/v6.3.0

Most important for Flutter are the reduction in the size of generated HTML files (dart-lang/dartdoc#3384) and a new dartdoc directive to hide constant implementations from indicated classes (dart-lang/dartdoc#3398), which fixes the longstanding issue (dart-lang/dartdoc#2657).

I've also added the api documentation zip to `.gitignore` and the `{@hideConstantImplementations}` dartdoc directive to the motivating example.  A screenshot:

![Screenshot 2023-06-07 at 9 54 58 AM](https://github.com/flutter/flutter/assets/14116827/1ad9c1f0-b224-462f-a8e3-706d9858f0d8)

I assert that this change to icons.dart should be test-exempt as existing tests cover whether or not dartdoc directives are recognized or are leaking into HTML, and the impact of adding the directive was tested in dart-lang/dartdoc#3398.
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Sep 27, 2023
Directive introduced in dart-lang/dartdoc#3398
but not documented yet. Still experimental, but used by Flutter.

Change-Id: I277927a54caf926b0b3d13db589486465ab9358d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328180
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide implementation of static constants
2 participants