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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code actions "Move to file" and "Move to new file" are shown everywhere #56416

Closed
jrieken opened this issue Oct 25, 2023 · 7 comments 路 Fixed by #57080
Closed

Code actions "Move to file" and "Move to new file" are shown everywhere #56416

jrieken opened this issue Oct 25, 2023 · 7 comments 路 Fixed by #57080
Labels
VS Code Priority Critical issues that VS Code needs fixed in the current TypeScript milestone

Comments

@jrieken
Copy link
Member

jrieken commented Oct 25, 2023

  • open this file
  • select data
  • select light bulb
  • 馃悰 there is entries for "Move to file" and "Move to new file"
Screenshot 2023-10-25 at 08 13 20
@justschen justschen self-assigned this Oct 26, 2023
@justschen
Copy link
Contributor

It's unrelated to editor.codeActionWidget.includeNearbyQuickFixes, but this does happen in every file.

imo, maybe the lightbulb shouldn't show for non-fix actions? if it's only move..., i can see how that can be annoying.

vs like how we have this one to indicate quickfixable actions.
Screenshot 2023-10-25 at 6 12 59鈥疨M

@jrieken
Copy link
Member Author

jrieken commented Oct 26, 2023

imo, maybe the lightbulb shouldn't show for non-fix actions? if it's only move..., i can see how that can be annoying.

I wouldn't declare this as a generic light bulb issue, yet. Having refactoring sold by it does makes sense but it shouldn't be as spammy as it is today with 'Move...'. It seem they show unconditionally (like for loop counters or semicolons) and would benefit from some self-control, e.g only show when on a method, function, class etc ppp

@justschen
Copy link
Contributor

justschen commented Oct 26, 2023

@jrieken atm, it is just shown in every non-empty line in a method, class, function, etc. and in some files, that means the entire file and nearly every line.

Could maybe play around with only having it to show up at the line of function/class/methoid declaration, but not in the code body? Not sure if @mjbvz has any opinions on this.

@mjbvz mjbvz transferred this issue from microsoft/vscode Nov 15, 2023
@mjbvz
Copy link
Contributor

mjbvz commented Nov 15, 2023

Some ideas:

  • Only show the lightbulb (implicit code action) when on declarations
  • Maybe still keep showing these actions if refactorings are explicitly requested
  • In the refactoring label, show the name of what is going to be moved. Right now it makes is seem like the loop itself will get moved

@jrieken
Copy link
Member Author

jrieken commented Nov 16, 2023

Only show the lightbulb (implicit code action) when on declarations

馃挴 on that - showing the code action further down is noise and lowers my trust in light bulb being helpful (and not ad-space)

@mjbvz mjbvz added the VS Code Priority Critical issues that VS Code needs fixed in the current TypeScript milestone label Nov 20, 2023
@aiday-mar
Copy link
Contributor

HI @sandersn, I would like to have a go at this PR. Could I have a pointer to place in the code where the change should be, or discuss this issue with one of your team members in a meeting? We are currently looking into displaying code actions in the light-bulb menu in VS Code on empty lines too, and currently we are seeing the Move to file and Move to new file actions appearing very often. I had initially prepared a PR to patch this issue on the VS Code side, but we think ideally this fix should come directly from TypeScript

@sandersn
Copy link
Member

@aiday-mar I believe @navya9singh and @andrewbranch have worked the most on Move To File.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VS Code Priority Critical issues that VS Code needs fixed in the current TypeScript milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants