MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-04-05 02:40:55 -07:00
Code Issues Packages Projects Releases Wiki Activity

feat(settings): rename negative settings to positive naming (disable* -> enable*) (#14142)

Browse Source 
Co-authored-by: jacob314 <jacob314@gmail.com>
This commit is contained in:
Alexander Farber
2026-01-16 23:33:49 +01:00
committed by GitHub
parent f2d3b76389
commit 608da23393
25 changed files with 785 additions and 243 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
packages/cli/src/utils/handleAutoUpdate.test.ts
View File

@@ -14,7 +14,6 @@ import EventEmitter from 'node:events';
import type { ChildProcess } from 'node:child_process';
import { handleAutoUpdate, setUpdateHandler } from './handleAutoUpdate.js';
import { MessageType } from '../ui/types.js';
import { mergeSettings } from '../config/settings.js';
vi.mock('./installationInfo.js', async () => {
const actual = await vi.importActual('./installationInfo.js');
@@ -50,9 +49,16 @@ describe('handleAutoUpdate', () => {
message: 'An update is available!',
};
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockSettings = {
merged: defaultMergedSettings,
merged: {
general: {
enableAutoUpdate: true,
enableAutoUpdateNotification: true,
},
tools: {
sandbox: false,
},
},
} as LoadedSettings;
mockChildProcess = Object.assign(new EventEmitter(), {
@@ -79,8 +85,8 @@ describe('handleAutoUpdate', () => {
expect(mockSpawn).not.toHaveBeenCalled();
});
it('should do nothing if update nag is disabled', () => {
mockSettings.merged.general.disableUpdateNag = true;
it('should do nothing if update prompts are disabled', () => {
mockSettings.merged.general.enableAutoUpdateNotification = false;
handleAutoUpdate(mockUpdateInfo, mockSettings, '/root', mockSpawn);
expect(mockGetInstallationInfo).not.toHaveBeenCalled();
expect(updateEventEmitter.emit).not.toHaveBeenCalled();
@@ -88,7 +94,7 @@ describe('handleAutoUpdate', () => {
});
it('should emit "update-received" but not update if auto-updates are disabled', () => {
mockSettings.merged.general.disableAutoUpdate = true;
mockSettings.merged.general.enableAutoUpdate = false;
mockGetInstallationInfo.mockReturnValue({
updateCommand: 'npm i -g @google/gemini-cli@latest',
updateMessage: 'Please update manually.',

6
packages/cli/src/utils/handleAutoUpdate.ts
View File

@@ -30,13 +30,13 @@ export function handleAutoUpdate(
return;
}
if (settings.merged.general.disableUpdateNag) {
if (!settings.merged.general.enableAutoUpdateNotification) {
return;
}
const installationInfo = getInstallationInfo(
projectRoot,
settings.merged.general.disableAutoUpdate,
settings.merged.general.enableAutoUpdate,
);
if (
@@ -58,7 +58,7 @@ export function handleAutoUpdate(
if (
!installationInfo.updateCommand ||
settings.merged.general.disableAutoUpdate
!settings.merged.general.enableAutoUpdate
) {
return;
}

48
packages/cli/src/utils/installationInfo.test.ts
View File

@@ -60,7 +60,7 @@ describe('getInstallationInfo', () => {
it('should return UNKNOWN when cliPath is not available', () => {
process.argv[1] = '';
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.UNKNOWN);
});
@@ -71,7 +71,7 @@ describe('getInstallationInfo', () => {
throw error;
});
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.UNKNOWN);
expect(debugLogger.log).toHaveBeenCalledWith(error);
@@ -84,7 +84,7 @@ describe('getInstallationInfo', () => {
);
mockedIsGitRepository.mockReturnValue(true);
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.UNKNOWN);
expect(info.isGlobal).toBe(false);
@@ -98,7 +98,7 @@ describe('getInstallationInfo', () => {
process.argv[1] = npxPath;
mockedRealPathSync.mockReturnValue(npxPath);
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.NPX);
expect(info.isGlobal).toBe(false);
@@ -110,7 +110,7 @@ describe('getInstallationInfo', () => {
process.argv[1] = pnpxPath;
mockedRealPathSync.mockReturnValue(pnpxPath);
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.PNPX);
expect(info.isGlobal).toBe(false);
@@ -125,7 +125,7 @@ describe('getInstallationInfo', () => {
throw new Error('Command failed');
});
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.BUNX);
expect(info.isGlobal).toBe(false);
@@ -141,7 +141,7 @@ describe('getInstallationInfo', () => {
mockedRealPathSync.mockReturnValue(cliPath);
mockedExecSync.mockReturnValue(Buffer.from('gemini-cli')); // Simulate successful command
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(mockedExecSync).toHaveBeenCalledWith(
'brew list -1 | grep -q "^gemini-cli$"',
@@ -165,7 +165,7 @@ describe('getInstallationInfo', () => {
throw new Error('Command failed');
});
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(mockedExecSync).toHaveBeenCalledWith(
'brew list -1 | grep -q "^gemini-cli$"',
@@ -184,13 +184,15 @@ describe('getInstallationInfo', () => {
throw new Error('Command failed');
});
const info = getInstallationInfo(projectRoot, false);
// isAutoUpdateEnabled = true -> "Attempting to automatically update"
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.PNPM);
expect(info.isGlobal).toBe(true);
expect(info.updateCommand).toBe('pnpm add -g @google/gemini-cli@latest');
expect(info.updateMessage).toContain('Attempting to automatically update');
const infoDisabled = getInstallationInfo(projectRoot, true);
// isAutoUpdateEnabled = false -> "Please run..."
const infoDisabled = getInstallationInfo(projectRoot, false);
expect(infoDisabled.updateMessage).toContain('Please run pnpm add');
});
@@ -202,7 +204,8 @@ describe('getInstallationInfo', () => {
throw new Error('Command failed');
});
const info = getInstallationInfo(projectRoot, false);
// isAutoUpdateEnabled = true -> "Attempting to automatically update"
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.YARN);
expect(info.isGlobal).toBe(true);
expect(info.updateCommand).toBe(
@@ -210,7 +213,8 @@ describe('getInstallationInfo', () => {
);
expect(info.updateMessage).toContain('Attempting to automatically update');
const infoDisabled = getInstallationInfo(projectRoot, true);
// isAutoUpdateEnabled = false -> "Please run..."
const infoDisabled = getInstallationInfo(projectRoot, false);
expect(infoDisabled.updateMessage).toContain('Please run yarn global add');
});
@@ -222,13 +226,15 @@ describe('getInstallationInfo', () => {
throw new Error('Command failed');
});
const info = getInstallationInfo(projectRoot, false);
// isAutoUpdateEnabled = true -> "Attempting to automatically update"
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.BUN);
expect(info.isGlobal).toBe(true);
expect(info.updateCommand).toBe('bun add -g @google/gemini-cli@latest');
expect(info.updateMessage).toContain('Attempting to automatically update');
const infoDisabled = getInstallationInfo(projectRoot, true);
// isAutoUpdateEnabled = false -> "Please run..."
const infoDisabled = getInstallationInfo(projectRoot, false);
expect(infoDisabled.updateMessage).toContain('Please run bun add');
});
@@ -243,7 +249,7 @@ describe('getInstallationInfo', () => {
(p) => p === path.join(projectRoot, 'yarn.lock'),
);
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.YARN);
expect(info.isGlobal).toBe(false);
@@ -261,7 +267,7 @@ describe('getInstallationInfo', () => {
(p) => p === path.join(projectRoot, 'pnpm-lock.yaml'),
);
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.PNPM);
expect(info.isGlobal).toBe(false);
@@ -278,7 +284,7 @@ describe('getInstallationInfo', () => {
(p) => p === path.join(projectRoot, 'bun.lockb'),
);
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.BUN);
expect(info.isGlobal).toBe(false);
@@ -293,7 +299,7 @@ describe('getInstallationInfo', () => {
});
mockedExistsSync.mockReturnValue(false); // No lockfiles
const info = getInstallationInfo(projectRoot, false);
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.NPM);
expect(info.isGlobal).toBe(false);
@@ -307,13 +313,15 @@ describe('getInstallationInfo', () => {
throw new Error('Command failed');
});
const info = getInstallationInfo(projectRoot, false);
// isAutoUpdateEnabled = true -> "Attempting to automatically update"
const info = getInstallationInfo(projectRoot, true);
expect(info.packageManager).toBe(PackageManager.NPM);
expect(info.isGlobal).toBe(true);
expect(info.updateCommand).toBe('npm install -g @google/gemini-cli@latest');
expect(info.updateMessage).toContain('Attempting to automatically update');
const infoDisabled = getInstallationInfo(projectRoot, true);
// isAutoUpdateEnabled = false -> "Please run..."
const infoDisabled = getInstallationInfo(projectRoot, false);
expect(infoDisabled.updateMessage).toContain('Please run npm install');
});
});

26
packages/cli/src/utils/installationInfo.ts
View File

@@ -33,7 +33,7 @@ export interface InstallationInfo {
export function getInstallationInfo(
projectRoot: string,
isAutoUpdateDisabled: boolean,
isAutoUpdateEnabled: boolean,
): InstallationInfo {
const cliPath = process.argv[1];
if (!cliPath) {
@@ -103,9 +103,9 @@ export function getInstallationInfo(
packageManager: PackageManager.PNPM,
isGlobal: true,
updateCommand,
updateMessage: isAutoUpdateDisabled
? `Please run ${updateCommand} to update`
: 'Installed with pnpm. Attempting to automatically update now...',
updateMessage: isAutoUpdateEnabled
? 'Installed with pnpm. Attempting to automatically update now...'
: `Please run ${updateCommand} to update`,
};
}
@@ -116,9 +116,9 @@ export function getInstallationInfo(
packageManager: PackageManager.YARN,
isGlobal: true,
updateCommand,
updateMessage: isAutoUpdateDisabled
? `Please run ${updateCommand} to update`
: 'Installed with yarn. Attempting to automatically update now...',
updateMessage: isAutoUpdateEnabled
? 'Installed with yarn. Attempting to automatically update now...'
: `Please run ${updateCommand} to update`,
};
}
@@ -136,9 +136,9 @@ export function getInstallationInfo(
packageManager: PackageManager.BUN,
isGlobal: true,
updateCommand,
updateMessage: isAutoUpdateDisabled
? `Please run ${updateCommand} to update`
: 'Installed with bun. Attempting to automatically update now...',
updateMessage: isAutoUpdateEnabled
? 'Installed with bun. Attempting to automatically update now...'
: `Please run ${updateCommand} to update`,
};
}
@@ -169,9 +169,9 @@ export function getInstallationInfo(
packageManager: PackageManager.NPM,
isGlobal: true,
updateCommand,
updateMessage: isAutoUpdateDisabled
? `Please run ${updateCommand} to update`
: 'Installed with npm. Attempting to automatically update now...',
updateMessage: isAutoUpdateEnabled
? 'Installed with npm. Attempting to automatically update now...'
: `Please run ${updateCommand} to update`,
};
} catch (error) {
debugLogger.log(error);

70
packages/cli/src/utils/settingsUtils.test.ts
View File

@@ -120,13 +120,13 @@ describe('SettingsUtils', () => {
description: 'Accessibility settings.',
showInDialog: false,
properties: {
disableLoadingPhrases: {
enableLoadingPhrases: {
type: 'boolean',
label: 'Disable Loading Phrases',
label: 'Enable Loading Phrases',
category: 'UI',
requiresRestart: true,
default: false,
description: 'Disable loading phrases for accessibility',
default: true,
description: 'Enable loading phrases during operations.',
showInDialog: true,
},
},
@@ -284,18 +284,18 @@ describe('SettingsUtils', () => {
it('should handle nested settings correctly', () => {
const settings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: true } },
ui: { accessibility: { enableLoadingPhrases: false } },
});
const mergedSettings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: false } },
ui: { accessibility: { enableLoadingPhrases: true } },
});
const value = getEffectiveValue(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
mergedSettings,
);
expect(value).toBe(true);
expect(value).toBe(false);
});
it('should return undefined for invalid settings', () => {
@@ -315,7 +315,7 @@ describe('SettingsUtils', () => {
it('should return all setting keys', () => {
const keys = getAllSettingKeys();
expect(keys).toContain('test');
expect(keys).toContain('ui.accessibility.disableLoadingPhrases');
expect(keys).toContain('ui.accessibility.enableLoadingPhrases');
});
});
@@ -342,9 +342,9 @@ describe('SettingsUtils', () => {
describe('isValidSettingKey', () => {
it('should return true for valid setting keys', () => {
expect(isValidSettingKey('ui.requiresRestart')).toBe(true);
expect(
isValidSettingKey('ui.accessibility.disableLoadingPhrases'),
).toBe(true);
expect(isValidSettingKey('ui.accessibility.enableLoadingPhrases')).toBe(
true,
);
});
it('should return false for invalid setting keys', () => {
@@ -357,7 +357,7 @@ describe('SettingsUtils', () => {
it('should return correct category for valid settings', () => {
expect(getSettingCategory('ui.requiresRestart')).toBe('UI');
expect(
getSettingCategory('ui.accessibility.disableLoadingPhrases'),
getSettingCategory('ui.accessibility.enableLoadingPhrases'),
).toBe('UI');
});
@@ -391,7 +391,7 @@ describe('SettingsUtils', () => {
const uiSettings = categories['UI'];
const uiKeys = uiSettings.map((s) => s.key);
expect(uiKeys).toContain('ui.requiresRestart');
expect(uiKeys).toContain('ui.accessibility.disableLoadingPhrases');
expect(uiKeys).toContain('ui.accessibility.enableLoadingPhrases');
expect(uiKeys).not.toContain('ui.theme'); // This is now marked false
});
@@ -421,7 +421,7 @@ describe('SettingsUtils', () => {
const keys = booleanSettings.map((s) => s.key);
expect(keys).toContain('ui.requiresRestart');
expect(keys).toContain('ui.accessibility.disableLoadingPhrases');
expect(keys).toContain('ui.accessibility.enableLoadingPhrases');
expect(keys).not.toContain('privacy.usageStatisticsEnabled');
expect(keys).not.toContain('security.auth.selectedType'); // Advanced setting
expect(keys).not.toContain('security.auth.useExternal'); // Advanced setting
@@ -454,7 +454,7 @@ describe('SettingsUtils', () => {
expect(dialogKeys).toContain('ui.requiresRestart');
// Should include nested settings marked for dialog
expect(dialogKeys).toContain('ui.accessibility.disableLoadingPhrases');
expect(dialogKeys).toContain('ui.accessibility.enableLoadingPhrases');
// Should NOT include settings marked as hidden
expect(dialogKeys).not.toContain('ui.theme'); // Hidden
@@ -601,14 +601,14 @@ describe('SettingsUtils', () => {
it('should return true when value differs from default', () => {
expect(isSettingModified('ui.requiresRestart', true)).toBe(true);
expect(
isSettingModified('ui.accessibility.disableLoadingPhrases', true),
isSettingModified('ui.accessibility.enableLoadingPhrases', false),
).toBe(true);
});
it('should return false when value matches default', () => {
expect(isSettingModified('ui.requiresRestart', false)).toBe(false);
expect(
isSettingModified('ui.accessibility.disableLoadingPhrases', false),
isSettingModified('ui.accessibility.enableLoadingPhrases', true),
).toBe(false);
});
});
@@ -628,11 +628,11 @@ describe('SettingsUtils', () => {
it('should return true for nested settings that exist', () => {
const settings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: true } },
ui: { accessibility: { enableLoadingPhrases: true } },
});
expect(
settingExistsInScope(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
),
).toBe(true);
@@ -642,7 +642,7 @@ describe('SettingsUtils', () => {
const settings = makeMockSettings({});
expect(
settingExistsInScope(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
),
).toBe(false);
@@ -652,7 +652,7 @@ describe('SettingsUtils', () => {
const settings = makeMockSettings({ ui: { accessibility: {} } });
expect(
settingExistsInScope(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
),
).toBe(false);
@@ -674,25 +674,25 @@ describe('SettingsUtils', () => {
it('should set nested setting value', () => {
const pendingSettings = makeMockSettings({});
const result = setPendingSettingValue(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
true,
pendingSettings,
);
expect(result.ui?.accessibility?.disableLoadingPhrases).toBe(true);
expect(result.ui?.accessibility?.enableLoadingPhrases).toBe(true);
});
it('should preserve existing nested settings', () => {
const pendingSettings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: false } },
ui: { accessibility: { enableLoadingPhrases: false } },
});
const result = setPendingSettingValue(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
true,
pendingSettings,
);
expect(result.ui?.accessibility?.disableLoadingPhrases).toBe(true);
expect(result.ui?.accessibility?.enableLoadingPhrases).toBe(true);
});
it('should not mutate original settings', () => {
@@ -1029,7 +1029,7 @@ describe('SettingsUtils', () => {
const settings = makeMockSettings({}); // nested setting doesn't exist
const result = isDefaultValue(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
);
expect(result).toBe(true);
@@ -1037,11 +1037,11 @@ describe('SettingsUtils', () => {
it('should return false when nested setting exists in scope', () => {
const settings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: true } },
ui: { accessibility: { enableLoadingPhrases: true } },
}); // nested setting exists
const result = isDefaultValue(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
);
expect(result).toBe(false);
@@ -1079,14 +1079,14 @@ describe('SettingsUtils', () => {
it('should return false for nested settings that exist in scope', () => {
const settings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: true } },
ui: { accessibility: { enableLoadingPhrases: true } },
});
const mergedSettings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: true } },
ui: { accessibility: { enableLoadingPhrases: true } },
});
const result = isValueInherited(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
mergedSettings,
);
@@ -1096,11 +1096,11 @@ describe('SettingsUtils', () => {
it('should return true for nested settings that do not exist in scope', () => {
const settings = makeMockSettings({});
const mergedSettings = makeMockSettings({
ui: { accessibility: { disableLoadingPhrases: true } },
ui: { accessibility: { enableLoadingPhrases: true } },
});
const result = isValueInherited(
'ui.accessibility.disableLoadingPhrases',
'ui.accessibility.enableLoadingPhrases',
settings,
mergedSettings,
);