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

"See code in" example links incorrectly group multiple examples, don't linkify entire path #4181

Closed
gspencergoog opened this issue Sep 26, 2022 · 2 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Milestone

Comments

@gspencergoog
Copy link

gspencergoog commented Sep 26, 2022

When navigating to an example in the Flutter repo source code in the form of:

/// {@tool dartpad}
/// This example shows various [ClipRRect]s applied to containers.
///
/// ** See code in examples/api/lib/widgets/basic/clip_rrect.0.dart **
/// {@end-tool}
/// ...
/// {@tool dartpad}
/// This example shows a [ClipRRect] that adds round corners to an image.
///
/// ** See code in examples/api/lib/widgets/basic/clip_rrect.1.dart **
/// {@end-tool}

If there are multiple examples in the comment then it says that there are two possible "definitions" for the destination, when the analyzer should be supplying the full path to the destination under the pointer, and so VSCode should be linkifying the entire path, not just the path element.

Screenshot from 2022-09-26 16-26-33

If there is only one example in the dartdoc block, then it more correctly links to just the link under the pointer:

Screenshot from 2022-09-26 16-27-08

However, it seems to me that VSCode should be rendering the links in another color (the "link" color that is used by symbol refs in brackets) so that users even know that they can click on them. It's not very discoverable to have to ctrl-click on the path in a comment, since it's not expected that it is a symbol.

It also seems like in that screenshot that the tooltip/hover text is showing an empty tip, which also shouldn't happen.

To Reproduce
Steps to reproduce the behavior:

  1. Open flutter/packages/flutter/lib/src/widgets/basic.dart in the Flutter repo.
  2. Point at the path in the "See code in" marker in the dartdoc block for ClipRRect.
  3. If you press Ctrl/Cmd while pointing at parts of the path, it will only highlight that part of the path, and will think there are two destinations.

Expected behavior

  1. Draw the link in the "link" color, even when not pointing at it.
  2. When pointing at it with Ctrl/Cmd down, linkify the entire path, not just the component under the pointer.
  3. When clicking on the path, it should just navigate without having to disambiguate, even if there are multiple examples.
  4. When pointing at a link, don't show an empty tooltip.

Please complete the following information:

  • Operating System and version: Debian GNU/Linux rodete 5.18.16-1rodete1-amd64
  • VS Code version: 1.71.2
  • Dart extension version: v3.48.4
  • Flutter extension version: v3.48.0
  • Dart/Flutter SDK version: Dart version 2.19.0 (build 2.19.0-229.0.dev)/Flutter version 3.4.0-29.0.pre.111
@DanTup
Copy link
Member

DanTup commented Sep 28, 2022

  1. Wrong highlight range when there are multiple results
    This one seems to be a VS Code bug (I've verified the ranges the server returns are correct) - Wrong ctrl+click underline with multiple definitions microsoft/vscode#160958
  2. Highlight links before hovering/holding ctrl
    I've split this to Improve colouring of clickable path links in dartdocs #4186
  3. Shouldn't need to disambiguate between links in the same dartdoc when you're hovering on a specific one
    Got a fix open at https://dart-review.googlesource.com/c/sdk/+/261365/.
  4. Empty tooltips
    I've split this to Ctrl+hover over file paths shows empty tooltip #4187

I'll use this issue for (3) above, since the others have their own issues and that seems like the biggest issue.

@DanTup DanTup added this to the v3.50.0 milestone Sep 28, 2022
@DanTup DanTup added in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server labels Sep 28, 2022
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 28, 2022
…d range

This fixes one of the issues noted in Dart-Code/Dart-Code#4181, where all example regions would be returned for a code block instead of only those that matched the requested region.

This was because the code that handles comments didn't go through helpers in `_DartNavigationCollector` that does the filtering.

Change-Id: I58931c02f0e4f538c246dc8fc0c3e57895cad5fb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261365
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
@DanTup
Copy link
Member

DanTup commented Sep 29, 2022

dart-lang/sdk@dd12cfd fixes the issue with multiple results being returned instead of only the one at the location requested. It's an SDK change, so will show up based on your SDK version (not Dart-Code version).

@DanTup DanTup closed this as completed Sep 29, 2022
@DanTup DanTup added the relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available label Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Projects
None yet
Development

No branches or pull requests

2 participants