-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
Organise imports should be a refactoring/code action #46647
Comments
This was my initial proposal too. However I now think that command makes more sense as organize imports is a file wide action (and will eventually be triggered on save) However we could also surface this as a code action to improve discoverability. I've opened microsoft/TypeScript#22909 to track this |
You mean like format document ;-) ? I disagree with 'upstream'-labelling because it is us exposing the functionality in VS Code and us to decide how that happens - as code action or command. The approach taken is wrong and reminds me of the formatting-via-command-story (not as provider) for which we went great lengths to educate extension authors. Now we set the same bad sample and even today things aren't working properly: in my install I have the Java extension which uses the same anti-pattern (mostly likely because code action weren't ready yet) and the assigned keybinding always runs the Java-command, never the TypeScript command... |
For reference - we blogged about this and tracked down every single formatter-command-extension with issues/PRs |
I’m still not convinced that code actions are the right ux. The intended usage of organize imports is very much like format in that it is independent of cursor position and run on the entire current document. And format—whether implemented using providers or actual commands—is a command from the user’s perspective. I’ve already submitted a pr against java for this. I agree that having an organize imports command per lanague is not scalable but think we should look at other approaches, such as a new provider type, if this ends up being a problem |
Well, it will allow us to have something like
Yes - everything is a command even IntelliSense but the point of well-know-thanks-to-api-commands is better UX integration. Now we have picked the worst in the face of a generic code actions infrastructure that would allow for better UI. What don't we add a new code action kind which about 'sugar', e.g. Format, Sort Members, Organise Imports, Align Whitespaces, Insert Copyright Header, etc.? |
My issue was closed as a duplicate of this. I don't believe it is but it is related. To keep everything in one place I'll paste my specific issue here so it doesn't get lost: The "Remove Unnecessary Usings" lightbulb for C# is super cool. Love it. However, I have roll over the unnecessary using before I can see that I actually have unnecessary usings. It would be great if:
Love your work by the way - loving coding C# in VS Code |
@johnnyreilly The lightbulb is controlled by the Omnisharp/C#-extension, so ultimately that must change. This issue is about show-casing how organise imports et all can be integrated into VS Code best. E.g. similar to how it is done in Eclipse (have a source and refactor menu) |
Fixes microsoft#47845 Fixes microsoft#46647 - Defines a new standard `SourceOrganizeImports` `CodeActionKind` to be used to implement organize imports in a consistent way. - Add a new `Organize imports` command and keybinding that executes these actions. - Move over the existing js/ts organize imports command to use the new code action kind
) * Move TS/JS to use organize imports code action Fixes #47845 Fixes #46647 - Defines a new standard `SourceOrganizeImports` `CodeActionKind` to be used to implement organize imports in a consistent way. - Add a new `Organize imports` command and keybinding that executes these actions. - Move over the existing js/ts organize imports command to use the new code action kind * Use supportedCodeActions context key * Document code action kind values * Fix regular expression Make sure we only match whole scopes and not `unicorn.source.organizeImports`
re #46589
After the investment in code actions, the refactoring menu, and code action keybindings I was quite surprised to see that organise imports isn't a code action but a normal command. I think that makes it harder to discover and also encourages homegrown code action instead of the more stream lined approach.
The text was updated successfully, but these errors were encountered: