Skip to content

Fix many library sidebars, and search #3865

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

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Sep 3, 2024

The library sidebars on container (e.g. Class), top-level variable, top-level function, and typedefs were totally busted, as a result of d629e1e.

Fixes #3859 and fixes #3861.

The fix is to allow a Library's canonicalModelElement to be it's canonical library, instead of null. This involves adding a few checks for "am I calculating something for a Library?" and moving some "is this library considered public" logic out of Element.hasPrivateName and into ModelElement.isPublic. That corrects the issue of an exported Class's (for example) enclosing element's URI, for the search results.

Once all of that is fixed, we also correct the aboveSidebarPath implementation for a few elements, to point to the canonical library's sidebar path.

This fix highlights an issue with wildcard-variables, so one test is newly marked as failing.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

srawlins added a commit to srawlins/dartdoc that referenced this pull request Sep 3, 2024
An unrelated fix, dart-lang#3865, revealed that
the wildcard-variables experiment was not actually enabled in tests; the SDK
constraint used in test packages needs to be '^3.6.0'. With that fix, there are
some questions about how much dartdoc should be doing to keep wildcards
"non-binding" and how much the analyzer should be doing. So that is raised in
dart-lang#3769 (comment).
@srawlins
Copy link
Member Author

srawlins commented Sep 3, 2024

The bots are going wild... failures but no logs ☹️ . But this is ready for review, @dart-lang/analyzer-team

The library sidebars on container (e.g. Class), top-level variable, top-level
function, and typedefs were totally busted, as a result of
d629e1e.

Fixes dart-lang#3859 and fixes
dart-lang#3861.

The fix is to allow a Library's `canonicalModelElement` to be it's canonical
library, instead of `null`. This involves adding a few checks for "am I
calculating something for a Library?" and moving some "is this library
considered public" logic out of `Element.hasPrivateName` and into
`ModelElement.isPublic`. That corrects the issue of an exported Class's (for
example) enclosing element's URI, for the search results.

Once all of that is fixed, we also correct the `aboveSidebarPath`
implementation for a few elements, to point to the _canonical_ library's
sidebar path.

This fix highlights an issue with wildcard-variables, so one test is newly
marked as failing.
@srawlins srawlins merged commit 4a6062e into dart-lang:main Sep 3, 2024
9 checks passed
@srawlins srawlins deleted the fix-exported-libs branch September 3, 2024 22:07
srawlins added a commit that referenced this pull request Sep 4, 2024
An unrelated fix, #3865, revealed that
the wildcard-variables experiment was not actually enabled in tests; the SDK
constraint used in test packages needs to be '^3.6.0'. With that fix, there are
some questions about how much dartdoc should be doing to keep wildcards
"non-binding" and how much the analyzer should be doing. So that is raised in
#3769 (comment).
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.

Top right search bar not functioning correctly Regression or bug with pub.dev: "failed to load sidebar"
2 participants