-
Notifications
You must be signed in to change notification settings - Fork 31k
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
Allow codeActionsOnSave to trigger for autoSave #190516
Allow codeActionsOnSave to trigger for autoSave #190516
Conversation
Requesting @mattbierner since you worked on this last - if there's a better way to do this, I can give it a deeper look 👍 |
See #155059 for a discussion around this. I'm concerned that users will accidentally enable it and then be confused by the behavior (such as unused imports getting silently removed after a delay) Instead of enabling this by default, maybe we could need an additional setting just for code actions that should be run on autosave, or something like: "editor.codeActionsOnSave": {
"source.fixAll": { "onAutosave": true}
} AFAIK we also don't support running format on autosave. Maybe we could look into enabling that too in a consistent way |
src/vs/workbench/contrib/codeActions/browser/codeActionsContribution.ts
Outdated
Show resolved
Hide resolved
There's a weird issue with |
src/vs/workbench/contrib/codeActions/browser/codeActionsContribution.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/codeActions/browser/codeActionsContribution.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts
Outdated
Show resolved
Hide resolved
const settingsOverrides = { overrideIdentifier: textEditorModel.getLanguageId(), resource: model.resource }; | ||
const setting = this.configurationService.getValue<{ [kind: string]: boolean } | string[]>('editor.codeActionsOnSave', settingsOverrides); | ||
// Keeping string | boolean until fully deprecating boolean options | ||
const setting = this.configurationService.getValue<{ [kind: string]: string | boolean } | string[]>('editor.codeActionsOnSave', settingsOverrides); | ||
if (!setting) { | ||
return undefined; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of handling the boolean values below, can you add logic here that converts the old boolean settings to the new enum values: false -> "never"
, true -> "explict"
. That should simplify the logic
tagging @Yoyokrazy for updates (pushed the code we wrote to this PR) |
Great work, thanks for the fix! Just curious will this go into next release? @justschen |
awesome, thanks for the heads up! |
Addresses #163920
Removing the code block check allows
codeActionsOnSave
andcreateCodeActionsOnSave
to run on auto save.Works for all autoSave settings including
afterDelay
,onFocusChange
, andonWindowChange
(Main change was commenting out the check, other changes with the imports at the top are just a result of the autoSave running organize imports on the imports in that file when the setting for
"editor.codeActionsOnSave": { "source.organizeImports": true }
is on. )