Repository navigation
Move Code Action Constraints #58387
Description
Acknowledgement
- I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.
Comment
related #56416 and https://github.com/microsoft/TypeScript/pull/57080/files
We had some discussion within the VS Code team regarding the changes Aiday Marlen Kyzy (@aiday-mar) made and how we wanted to make the constraints tighter.
the current sentiment after discussion:
- when moving or clicking through code (ie, not selecting a range of code), users likely will not want to see the
Move to...code actions. however, it should be available to them if they manually select (viaRefactororCmdCtrl + .)- this fixes the annoyance of having the lightbulb widget shown virtually everywhere in the file when typing or moving trough code.
- however, if the cursor is at the top scoping level or on its identifier, we can show the
Move to...actions.
Reasoning behind not having the lightbulb be so aggressive in suggesting code actions is that the lightbulb loses value (ie, blue lightbulb means there are fixes, but yellow lightbulb is super general). Even if users don't explicitly hit the lightbulb button, when they see certain changes in the lightbulb, they might hit CmdCtrl + . to trigger the code actions.
when actually selecting a range of code, I believe Aiday Marlen Kyzy (@aiday-mar) had some proposed fixes for the constraints as well, similarly looking at scoping.
In our latest stable release (1.89.0), i added a good amount of telemetry tracking a few things that might help us make a more informed and concrete decision if we want to move forward with this:
- when the lightbulb is clicked -> which code actions and providers are shown in that code action menu
- when the code action menu is exited (and the source of the menu was the lightbulb) -> if a code action was selected or if it was just quit out.
- when code actions are applied -> we can see how often
move to...is being used when the source is the lightbulb widget. - in insiders only: how often the lightbulb widget shows up when there is only a single code action in that list (ie, only
move to file)
currently have a draft PR for our built in extension, but as Matt mentioned, likely would be best to fix here so the behavior is consistent across all editors.
I'll likely take a look at making a PR here, so any help would be appreciated!
Let me know if there are any thoughts on this, if we think it's too drastic, if we want to see more telemetry, etc.
cc. Matt Bierner (@mjbvz) navya9singh Andrew Branch (@andrewbranch) Aiday Marlen Kyzy (@aiday-mar)
Activity
DanielRosenwasser commentedon May 1, 2024
Conceptually some of this sounds right, but it'd help to have an idea of what is still annoying currently since #57080 was merged, and what this issue's proposing. For example:
This one I'd like to have a much better intuition around.
justschen commentedon May 2, 2024
The main feedback is around the lightbulb widget (which could indicate that at it's core, it could be a VS Code issue that should be fixed on our side) showing up virtually everywhere because the
Move tocode actions contributed by TS are shown virtually everywhere!As you know, VS Code automatically requests code actions at each cursor move, typing, etc, and it just happens that
Move toshows up because it is a valid action in most code blocks.Ideally, there are constraints around aggressively showing and having code actions everywhere, especially after telemetry suggests it's not being used in the automatically, and only when explicitly clicking
refactororCtrlCmd + .aiday-mar commentedon May 13, 2024
Hi Justin Chen (@justschen), Daniel Rosenwasser (@DanielRosenwasser) thank you for opening this issue. We currently believe the action is returned too often even after the changes, and after discussing with the team, we believe a good condition for showing the action would be the following.
Make the action appear when both ends of the primary selection are on a top-level scoping symbol such as
interface,class,module,functionor on its identifier, or completely enclosing a top-level scope.justschen commentedon May 15, 2024
Notes from TS editor sync:
lightbulb->move to....invokedvsimplicitand how they actually work.TLDR: do not show if automatically triggered (then lightbulb will also not show), only if requested from
RefactororCtrl + .add that to current selection logic formove to filewill not change, can be applied tomove to new fileuntil it is removed.DanielRosenwasser commentedon Jun 19, 2024
Looks like #58548 fixes this, but the GitHub "closes" pattern didn't kick in because of the ##.
Feel free to reopen, but otherwise thanks Justin Chen (@justschen)!