Annoying "lightbulb" whenever placing cursor into constant js/ts expression

[weinand]

• VSCode Version: Insiders
• OS Version: macOS

Steps:

• open js or ts file
• place cursor in any constant expression (e.g. in the "1.16.6" below)

Observe: a "lightbulb" appears:

IMO this is an idiotic suggestion and I find this quite annoying.

Do we have telemetry info that shows that users are actually using this?

[mjbvz]

@weinand This was just added by TS 2.5. Showing extract method in this location looks like a TS bug to me, but I also think we need to think about what we want the UX to be in other case. I'd argue that we should only show the lightbulb when the user has made a text selection. The CodeActionProvider does not currently support controlling whether or not the light bulb is shown. This may require an API change similar to what I've proposed in another issue: #33049 (comment)

[weinand]

See #33282

[jrieken]

This is a general issue with the light bulb and how we check if it should be shown. There two things;

1. The "Quick Fix"-command which is bound to Ctrl/Cmd+.
2. The lightbulb that is trying to sell this feature

To show the light bulb we monitor what is happening in the editor, ask silently for code actions, and iff there are any then we show the light bulb. The monitor-logic today is the following: Check to show lightbulb when the cursor is on a diagnostic, (2) check to show lightbulb when we have a non-empty selection on a "word". The latter is what is annoying and the plan is to change it to check to show lightbulb only when there is non-empty selection.

[rcjsuen]

In Eclipse the light bulb decoration is always there regardless of where the text cursor is. As a result, I was initially very surprised to see that the light bulb only appeared in VS Code when the text cursor entered the range the area that the diagnostic underlined.

I feel that it will become very difficult for new users to realize that quick fixes are available as users will now be forced to make a text selection where the diagnostic is to get the light bulb to appear. The fact that you currently have to place your cursor where the diagnostic is to "check for the existence of quick fixes" is already non-obvious to users in my opinion.

Whereas in Eclipse I can immediately tell visually if quick fixes are available, in VS Code I have to click on every diagnostic to verify this. With the suggested upcoming new behaviour, I will have to double-click every diagnostic to select a word or click inside the diagnostic and then hit Ctrl+. to test if a quick fix is available.

[Gama11]

I'm also not excited about code actions becoming less discoverable by requiring a selection - the easy discoverability is what makes code actions so useful to begin with. IntelliJ also doesn't require a selection, and for some code actions, requiring a selection really wouldn't make much sense:

There is one thing that annoys me about some code actions that are almost always available, which is them getting in the way of breakpoints (#25174). This can be solved differently though, IntelliJ solves this by showing the light bulb in the editor area, not the gutter:

[jrieken]

@rcjsuen and @Gama11 - Thanks for the input! Let me try to slice and dice this into pieces

In Eclipse the light bulb decoration is always there regardless of where the text cursor is. As a result, I was initially very surprised to see that the light bulb only appeared in VS Code when the text cursor entered the range the area that the diagnostic underlined.

Agreed. That is useful and something we will look into for #25149

I feel that it will become very difficult for new users to realize that quick fixes are available as users will now be forced to make a text selection where the diagnostic is to get the light bulb to appear

Nothing will change there: when you are inside an error/warning having an empty selection is enough. Again, we can improve this by showing the lightbulb for errors/warnings more often, e.g. when you are on the line.

This is about getting a lightbulb while you type and, worst, the lightbulb preventing you from setting breakpoints. @Gama11 presents a nice sample for having a lightbulb even for empty selections and while I agree on the usefulness, the lightbulb/breakpoint conflict still exists.

To put this in perspective, this is a "tactical" fix for the August release. We want to ship with TypeScript refactorings but we don't want to steel the ability to set breakpoints. So, while we work on #25174 we will have the slightly changed lightbulb behaviour - not because that's the right thing but because it is less bad.

This is how it looks/behaves with my changes:

[Gama11]

We want to ship with TypeScript refactorings but we don't want to steel the ability to set breakpoints.

That's fine and all, but this affects more than just TypeScript. I can't say I like the idea of a temporary (as it seems to be), "tactical fix" (which is really just an embellishment for "hack") for TypeScript affecting all other language servers with code actions.

In the Haxe extension, we've had an "Extract Variable" code action for a while, but one of the reasons we have decided not to include it in releases yet because of this exact issue (it getting it in the way).

It might necessary for code actions to have flag that determines whether they are "selection-based" - some, like "Extract Method" or "Extract Variable" probably don't make sense without a selection. Which raises the question whether they should be code actions in the first place - the other alternative that I can see is to not implement these as code actions, but commands. This is what IntelliJ does - it doesn't implement these as code actions:

The more I think about it, the more I think that's the right approach, because code actions are usually "quick fixes" where the language server can easily tell that something could be improved a bit (remove unused code, replace anonymous function with short lambda, etc). They don't require input from the user, like a selection.

Refactorings like extracting methods or variables are something that require more "intent" by the user. They're usually something you specifically request the editor to do, because they're too complex for the editor to come up with good suggestions. With code actions, it's the other way around: you "stumble" over them and (hopefully) think "good point editor, let's make that a bit nicer".

[mjbvz]

Thanks @jrieken! With the workaround, should we keep this bug open? In my testing, the original issue seems mitigated

---

From this discussion, I that our refactoring story needs some more thought. I have opened #33555 to track issues / missing features with our current refactoring support and linked back to the comments here from that issue. Feel free to add your thoughts to the new issue as well

[jrieken]

Yeah, I'll close this and we will continue the conceptual work in #33555 and the UX work will happen in #25174

[jrieken]

That also means that we will restore the lightbulb with empty selection when having a less annoying UX