From efec63658a423608aeed61882522294cf4da626c Mon Sep 17 00:00:00 2001 From: Yashodip More <113869613+yashodipmore@users.noreply.github.com> Date: Thu, 5 Mar 2026 02:09:37 +0530 Subject: [PATCH] chore(cli): enable deprecated settings removal by default (#20682) --- packages/cli/src/config/settings.test.ts | 51 +++++++++++++++--------- packages/cli/src/config/settings.ts | 5 +-- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 8fd0bd81b0..5589ef11ba 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -2162,7 +2162,7 @@ describe('Settings Loading and Merging', () => { } }); - it('should prioritize new settings over deprecated ones and respect removeDeprecated flag', () => { + it('should remove deprecated settings by default and prioritize new ones', () => { const userSettingsContent = { general: { disableAutoUpdate: true, @@ -2177,27 +2177,11 @@ describe('Settings Loading and Merging', () => { }; const loadedSettings = createMockSettings(userSettingsContent); - const setValueSpy = vi.spyOn(loadedSettings, 'setValue'); - // 1. removeDeprecated = false (default) + // Default is now removeDeprecated = true migrateDeprecatedSettings(loadedSettings); - // Should still have old settings - expect( - loadedSettings.forScope(SettingScope.User).settings.general, - ).toHaveProperty('disableAutoUpdate'); - expect( - ( - loadedSettings.forScope(SettingScope.User).settings.context as { - fileFiltering: { disableFuzzySearch: boolean }; - } - ).fileFiltering, - ).toHaveProperty('disableFuzzySearch'); - - // 2. removeDeprecated = true - migrateDeprecatedSettings(loadedSettings, true); - // Should remove disableAutoUpdate and trust enableAutoUpdate: true expect(setValueSpy).toHaveBeenCalledWith(SettingScope.User, 'general', { enableAutoUpdate: true, @@ -2209,6 +2193,37 @@ describe('Settings Loading and Merging', () => { }); }); + it('should preserve deprecated settings when removeDeprecated is explicitly false', () => { + const userSettingsContent = { + general: { + disableAutoUpdate: true, + enableAutoUpdate: true, + }, + context: { + fileFiltering: { + disableFuzzySearch: false, + enableFuzzySearch: false, + }, + }, + }; + + const loadedSettings = createMockSettings(userSettingsContent); + + migrateDeprecatedSettings(loadedSettings, false); + + // Should still have old settings since removeDeprecated = false + expect( + loadedSettings.forScope(SettingScope.User).settings.general, + ).toHaveProperty('disableAutoUpdate'); + expect( + ( + loadedSettings.forScope(SettingScope.User).settings.context as { + fileFiltering: { disableFuzzySearch: boolean }; + } + ).fileFiltering, + ).toHaveProperty('disableFuzzySearch'); + }); + it('should trigger migration automatically during loadSettings', () => { mockFsExistsSync.mockImplementation( (p: fs.PathLike) => p === USER_SETTINGS_PATH, diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 4e9faf5767..21dd3eb35f 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -796,14 +796,13 @@ export function loadSettings( /** * Migrates deprecated settings to their new counterparts. * - * TODO: After a couple of weeks (around early Feb 2026), we should start removing - * the deprecated settings from the settings files by default. + * Deprecated settings are removed from settings files by default. * * @returns true if any changes were made and need to be saved. */ export function migrateDeprecatedSettings( loadedSettings: LoadedSettings, - removeDeprecated = false, + removeDeprecated = true, ): boolean { let anyModified = false; const systemWarnings: Map = new Map();