Verify fix/revision to Code Actions on Save

[justschen]

tests #194031 and new changes from #194409. related to #194397

added back old boolean values (since we reverted the changes for migration) in addition to supporting new enum values.

This is for both Code Actions in notebooks and in the editor:

1. Find notebook.codeActionsOnSave or editor.codeActionsOnSave
2. test each enum's behavior. note that always currently does not support code actions on auto save after delay. (easiest code actions to test would be source.fixAll and source.organizeImports. other code actions like source.fixAll.eslint and source.removeUnusedImports are supported as well.
3. test boolean behavior. (true and explicit should be the same, false and never should be the same. there are descriptions about future deprecation and behavior as well!)

[bhavyaus]

Closing this as completed. Waiting on insiders to verify.

[vscodenpa]

Issue marked as unreleased but unable to locate closing commit in issue timeline. You can manually reference a commit by commenting \closedWith someCommitSha, or directly add the insiders-released label if you know this has already been releaased

[chrmarti]

@justschen For notebooks I see only two choices, is this expected?

[justschen]

Yep! That's because you already inserted the quotations, so it only gives the string results. When no input or anything has been typed yet, you should get the following on intellisense. Should be the same for editor as well.

[chrmarti]

Verified for editor code actions. I can't notebook code actions to work. @Yoyokrazy Could you provide steps to use "notebook.codeActionsOnSave" with Python notebooks? (Or some other language that is easy to set up.)

[chrmarti]

Got it working with a Github Issue Notebook (thanks @jrieken):

  "notebook.codeActionsOnSave": {
    "notebook.source.normalizeVariableNames":true
  }

true also triggers it on auto save, which seems inconsistent with editor.codeActionsOnSave given that there is no "always" for notebooks and true for editors is the same as "explicit".

Otherwise works as described.

[justschen]

@Yoyokrazy suggested change to support backward compatibility:

if (context.reason === SaveReason.AUTO) {
	//saveTrigger = 'always';
	return undefined;
} 

or just remove entirely since it would be under the umbrella with SaveReason.FOCUS_CHANGE and SaveReason.WINDOW_CHANGE) to return undefined and exit early without applying code actions.

atm, this code includes all code actions when the value is true, even when saveTrigger (SAVE_REASON....) is auto.

const includedActions = allCodeActions
	.filter(x => setting[x.value] === saveTrigger || setting[x.value] === true);

[Yoyokrazy]

Thanks for the catch @chrmarti, PR mentioned here addresses this and SaveReason.FOCUS_CHANGE + SaveReason.WINDOW_CHANGE support will be added during debt week to finalize all of this.

[dockleryxk]

@justschen is there any chance of changing the behavior to run the actions in order when using an object, or will that only be for using an array for the setting?

I know the docs currently say to use an array when you need to execute in order, but that limits the actions to "explicit"

[justschen]

@dockleryxk Regarding the fixes around #194861, this is something we may look into! atm, as you can see from #194861 (comment), we do explicitly force fixAll to run first, but may change the schema to support more custom orderings in the future to align with the new Code Actions on Save enums.

[dockleryxk]

@justschen Sweet!!! I bring this up because a common workflow I've seen is running format with prettier and then following it with eslint to make some tweaks.

Either way, thank you for going back and fixing the array version of the setting in #194870 :)