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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve settings descriptions for actions triggered on save #230052

Merged
merged 4 commits into from
Oct 24, 2024

Conversation

elias-pap
Copy link
Contributor

closes #230051

@elias-pap
Copy link
Contributor Author

@microsoft-github-policy-service agree

@elias-pap elias-pap marked this pull request as ready for review September 28, 2024 18:32
Copy link
Contributor

@justschen justschen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also take a look at editor.codeActions.triggerOnFocusChange, which will save in certain scenarios:

  1. auto save is afterDelay
  2. editor code actions on save are set to always.
  3. focus is changed

note that this only works atm for editor code actions on save, and not notebooks.

@elias-pap elias-pap changed the title fix: improve settings descriptions for actions trigged on save fix: improve settings descriptions for actions triggered on save Oct 11, 2024
@elias-pap elias-pap force-pushed the improve-settings-descriptions branch from 22bef86 to 2de9ac0 Compare October 11, 2024 22:18
@elias-pap
Copy link
Contributor Author

elias-pap commented Oct 11, 2024

Please also take a look at editor.codeActions.triggerOnFocusChange, which will save in certain scenarios:

Not sure what you mean. This setting does not "save" in certain scenarios, it only triggers the code actions under some conditions, one of them being auto save is set to afterDelay. So the save happens automatically after delay, not from this setting. Also, the code actions run automatically, but not because (or at the time of) the editor is being saved.

@justschen
Copy link
Contributor

justschen commented Oct 20, 2024

@elias-pap sorry, that's what I meant. I added that setting a few iterations ago since users with afterDelay on wanted to still be able to run code actions.

my point of bringing it up is because we don't mention that this setting which will run code actions when the file isn't saved explicitly.

maybe adding a mention of this setting somehow could be helpful, although it is getting a bit verbose

@elias-pap
Copy link
Contributor Author

elias-pap commented Oct 24, 2024

@elias-pap sorry, that's what I meant. I added that setting a few iterations ago since users with afterDelay on wanted to still be able to run code actions.

Right, that's what I had guessed.

my point of bringing it up is because we don't mention that this setting which will run code actions when the file isn't saved explicitly.

maybe adding a mention of this setting somehow could be helpful, although it is getting a bit verbose

I agree that it's a bit overkill. These settings are called "formatOnSave" and "codeActionsOnSave", so it's better to focus on what happens on save instead of repeating descriptions from other settings. The word "only" can be removed to avoid contradictions ("will only be run when the file is saved explicitly" -> "will be run when the file is saved explicitly"), and that leaves room for other settings (like the one you mention) to trigger code actions as well.

The point of this fix is to clarify that the user can both have autoSave -> afterDelay enabled, and format the file (or run code actions) by saving manually, at the same time.

@vs-code-engineering vs-code-engineering bot added this to the October 2024 milestone Oct 24, 2024
Copy link
Contributor

@justschen justschen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! thanks for the patience, looks good to me 👍

@justschen justschen enabled auto-merge (squash) October 24, 2024 21:19
@justschen justschen merged commit 56c9d0a into microsoft:main Oct 24, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing settings descriptions about actions and auto save
6 participants