Keep getting a change on workspace open

[jrieken]

• I guess this a regression from Migrating over settings for Code Actions on Save to Enums #194345
• Have workspace setting like below
• Reload the window
• 🐛 you get change in .vscode/settings.json which is a spread of those compact settings

Before

 "[typescript][javascript]": {
    "editor.insertSpaces": false,
    "editor.defaultFormatter": "vscode.typescript-language-features",
    "editor.formatOnSave": true,
    "editor.codeActionsOnSave": {
      "source.organizeImports": "always"
    }
  },

After

{
  "[typescript][javascript]": {
    "editor.insertSpaces": false,
    "editor.defaultFormatter": "vscode.typescript-language-features",
    "editor.formatOnSave": true,
    "editor.codeActionsOnSave": {
      "source.organizeImports": "always"
    }
  },
// ....
  "[javascript]": {
    "editor.codeActionsOnSave": {
      "source.organizeImports": "always"
    }
  },
  "[typescript]": {
    "editor.codeActionsOnSave": {
      "source.organizeImports": "always"
    }
  },
}

[bpasero]

✋ I actually thought this was intentional and have pushed the change to vscode-docs@vnext branch.

[sandy081]

@justschen There should not be migration happening in this case right?

[sandy081]

I would suggest to check the new value and write only if it is changed.

[justschen]

Sounds look. Looking into this atm.

[justschen]

Resolved the settings changing in scenarios where migration is not needed, but in scenarios where migration is needed, the above change still happens. Ends up looking like this:

{
	"files.autoSave": "onFocusChange",
    "editor.accessibilitySupport": "on",
	"workbench.colorTheme": "Solarized Light",
	"editor.detectIndentation": false,
	"editor.codeActionWidget.showInlineQuickfixes": true,
	"editor.codeActionWidget.includeNearbyQuickfixes": true,
	"[typescript][javascript]": {
		"editor.insertSpaces": false,
		"editor.defaultFormatter": "vscode.typescript-language-features",
		"editor.formatOnSave": true,
		"editor.codeActionsOnSave": {
		  "source.organizeImports": true
		}
	},
   "editor.codeActionsOnSave": {
		"source.removeUnusedImports": "explicit"
	},
	"[javascript]": {
		"editor.codeActionsOnSave": {
			"source.removeUnusedImports": "explicit",
			"source.organizeImports": "explicit"
		}
	},
	"[typescript]": {
		"editor.codeActionsOnSave": {
			"source.removeUnusedImports": "explicit",
			"source.organizeImports": "explicit"
		}
	},
}

Digging into it further, this is because registerEditorSettingMigration doesn't have the concept of anything outside of it's current scope (in this case, editor.codeActionsOnSave), seems to find the setting multiple times for each language.

@sandy081 @jrieken Not sure if there is a better way to handle the migrations but would appreciate some thoughts/insight - some of the more complex examples in https://github.com/microsoft/vscode/blob/main/src/vs/editor/browser/config/migrateOptions.ts are still a tad different from what we want to accomplish here.

[sandy081]

@justschen I think you are doing the right thing here.

The scenario you are hitting here is a limitation with the IConfigurationService.updateValue api. This is true with all editor settings migrations so it is not new with your migration.

I would recommend you to go with your change ie., do migration only when value has changed.

Filed separate issue to fix this in the update method - #194427 - I would prefer to fix this in next milestone because 1. It is not a regression 2. It is not a simple fix to do and therefore risky to do in the endgame.

[sandy081]

{
	"[typescript][javascript]": {
		"editor.insertSpaces": false,
		"editor.defaultFormatter": "vscode.typescript-language-features",
		"editor.formatOnSave": true,
		"editor.codeActionsOnSave": {
		  "source.organizeImports": true
		}
	},
	"[javascript]": {
		"editor.codeActionsOnSave": {
			"source.removeUnusedImports": "explicit",
			"source.organizeImports": "explicit"
		}
	},
	"[typescript]": {
		"editor.codeActionsOnSave": {
			"source.removeUnusedImports": "explicit",
			"source.organizeImports": "explicit"
		}
	},
}

To confirm, even though value is migrated to each language independently, the effective value is correct and will be the migrated value. I mean, the value of editor.codeActionsOnSave for typescript and javascript languages will be

{
        "source.removeUnusedImports": "explicit",
        "source.organizeImports": "explicit"
}

[justschen]

Yes - original override with [typescript] [javascript] is unchanged, but the new settings written are changed properly from boolean to respective enum.

[justschen]

closing and deferring rest of issue to #194427

[justschen]

@joyceerhl repro:

setting is this:

 "[typescript][javascript]": {
            "editor.insertSpaces": false,
            "editor.defaultFormatter": "vscode.typescript-language-features",
            "editor.formatOnSave": true,
            "editor.codeActionsOnSave": {
                "source.organizeImports": true,
                "source.fixAll": false
            }
          },

should end up like this without duplicating the settings:

 "[typescript][javascript]": {
            "editor.insertSpaces": false,
            "editor.defaultFormatter": "vscode.typescript-language-features",
            "editor.formatOnSave": true,
            "editor.codeActionsOnSave": {
                "source.organizeImports": "explicit",
                "source.fixAll": "never"
            }
          },

example with multiple:

        "[typescript][javascript]": {
            "editor.insertSpaces": false,
            "editor.defaultFormatter": "vscode.typescript-language-features",
            "editor.formatOnSave": true,
            "editor.codeActionsOnSave": {
                "source.organizeImports": false,
                "source.fixAll": "never"
            }
          },
        "[python]": {
            "editor.codeActionsOnSave": {
                "source.organizeImports": true
            }
        },
        "editor.codeActionsOnSave": {
            "source.organizeImports": true
        },

expected to turn into:

        "[typescript][javascript]": {
            "editor.insertSpaces": false,
            "editor.defaultFormatter": "vscode.typescript-language-features",
            "editor.formatOnSave": true,
            "editor.codeActionsOnSave": {
                "source.organizeImports": "never",
                "source.fixAll": "never"
            }
          },
        "[python]": {
            "editor.codeActionsOnSave": {
                "source.organizeImports": "explicit"
            }
        },
        "editor.codeActionsOnSave": {
            "source.organizeImports": "explicit"
        },

[sandy081]

Found following bug while verifying - #199736