Open yjbanov opened 4 months ago
Yes, this is a request for the analysis server, and yes, if we made this change it would work in all of the IDEs that support navigation through the analysis server.
I agree that it would be good to have better support for navigating to the overridden method.
The one downside I can think of to using ctrl-click navigation for that purpose is that it makes the feature inconsistent. Everywhere else (including other annotations) it will take you to the declaration of the identifier, but here it wouldn't. I don't know how jarring that behavior would be for newer users.
We've been discussing how to provide good navigation when augmentations are enabled and one of the ideas that was proposed was to use code lenses to navigate to important targets, including to overridden methods. That solution leaves ctrl-click navigation consistent while providing more options for navigating to where users might want to go (including being able to navigate both directions in the override chain).
As noted in that comment, there are already commands to support some of this navigation, but they aren't very discoverable.
TIL what "CodeLens" is, so this is all based on very early impression. I'm open to alternatives to ctrl-click, but I have concerns about CodeLens specifically:
Why not click on the member name? Currently, it does nothing (VS Code). If it is overridden, go to the original member.
Is this a proprietary VSCode feature? If so, how can it cover all editors?
No, it's supported by LSP, so any editor using LSP can support this feature. (Not all LSP-based editors choose to support it, but there's not much we can do about that.)
IntelliJ and Android Studio don't use LSP, but they both have different support for this feature (icons in the editor's left gutter) that the analysis server has been supporting for many years. VS Code is actually behind IntelliJ in this case.
It injects copious amounts of UI into the editor with no user action. That seems excessive.
That's a fair concern. I believe that the current plan is to have a user-setting to enable or disable those code lenses, and to have commands on the pop-up menu for all of these options. Menu items aren't as convenient, but they are less invasive.
It might be worth opening an issue with the VS Code team asking for a cleaner UI.
It seems all CodeLenses need to be precomputed across the entire file to generate these extra links. This makes me worried about performance.
That's also a fair concern, and we'll be keeping a close eye on the performance characteristics. My expectation, partially based on the experiments that @DanTup wrote up, is that it's not likely to be an issue. Hopefully those aren't "famous last words".
Why not click on the member name? Currently, it does nothing (VS Code). If it is overridden, go to the original member.
That would also be a viable option, but it only handles one direction of navigation, and it's useful to be able to navigate both ways (up and down the override chain).
We will also need to support navigation across the 'augmentation' chain if we decide to support augmentations and macros. Having a consistent UX for navigation might trump many other considerations.
FWIW, we do have a "Go to Super" command + context menu entry that will take you where you want in the example above:
Why not click on the member name? Currently, it does nothing (VS Code). If it is overridden, go to the original member.
Ctrl+Click on a declaration actually does do something in VS Code. By default it will try to "go to definition" and when we return a definition that matches where you invoked it, it will fall back to "go to references". So in your example code above, if there are calls to foo()
it should show those results. This behaviour can be configured in VS Code (although the options are limited to other similar commands):
Unfortuantely we can't really change the behaviour here much because VS Code doesn't ask us to "handle a Ctrl+Click here", it just asks us "Where is the definition of the reference at this location", which is a generic request it uses in many places. Returning the overridden member instead could cause other functionality that uses "definition" to behave incorrectly. While it's more limiting, this does also ensure more consistency between different languages because the kind of navigation is configured once centrally.
I did file an issue to about making "Super" a first-class feature in VS Code (so it might have better UI), but unfortunately it was rejected (https://github.com/microsoft/vscode/issues/47126).
After playing around with it a bit, I think the current state of things is sufficient. I see that the "Go to Super Class/Member" is also available from the keyboard-invocable menu (Ctrl + Shift + P in VSCode), so this action can be performed without much extra UI, or having to do much mouse/trackpad work.
That said, for all intents and purposes @override
is part of the language, even if it's technically not according to the language team. Perhaps it should formally become a modifier keyword, like @required
did. There are several places in Dart where keywords could be excellent navigation anchors, such as:
override void foo() {}
var bar = getBar();
final baz = getBaz();
override
could navigate to the super method, and var
and final
could navigate to the inferred type declaration.
As for this issue, I'm OK with closing it given the existing functionality.
There's a proposal for override becoming a language feature: https://github.com/dart-lang/language/blob/main/working/1610%20-%20override/proposal.md.
...
var
andfinal
could navigate to the inferred type declaration.
I like that idea.
FWIW, in LSP we already support "Go to Type Definition" which you can invoke on a variable (not only where it's declared) to jump directly to the type :-)
I think this is a feature request for the Dart analyzer, but maybe VSCode (I'd love this to work in all editors though).
When I Ctrl + Click on the
@override
annotation, it take me to the definition of the@override
annotation. By now I know what@override
means. The vastly more important use-case is to be able to go from the overriding method to the overridden method. So I think control-clicking on this annotation to take you to the overridden method.To reproduce, use this code:
Ctrl + Click on
@override
, observe.