Feature custom themes logic (#2639)

Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
Ali Al Jufairi
2025-07-20 16:51:18 +09:00
committed by GitHub
parent c0bfa388c5
commit 76b935d598
19 changed files with 1313 additions and 341 deletions

View File

@@ -95,7 +95,10 @@ describe('Settings Loading and Merging', () => {
expect(settings.system.settings).toEqual({});
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual({});
expect(settings.merged).toEqual({
customThemes: {},
mcpServers: {},
});
expect(settings.errors.length).toBe(0);
});
@@ -124,7 +127,11 @@ describe('Settings Loading and Merging', () => {
expect(settings.system.settings).toEqual(systemSettingsContent);
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual(systemSettingsContent);
expect(settings.merged).toEqual({
...systemSettingsContent,
customThemes: {},
mcpServers: {},
});
});
it('should load user settings if only user file exists', () => {
@@ -153,7 +160,11 @@ describe('Settings Loading and Merging', () => {
);
expect(settings.user.settings).toEqual(userSettingsContent);
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual(userSettingsContent);
expect(settings.merged).toEqual({
...userSettingsContent,
customThemes: {},
mcpServers: {},
});
});
it('should load workspace settings if only workspace file exists', () => {
@@ -180,7 +191,11 @@ describe('Settings Loading and Merging', () => {
);
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
expect(settings.merged).toEqual(workspaceSettingsContent);
expect(settings.merged).toEqual({
...workspaceSettingsContent,
customThemes: {},
mcpServers: {},
});
});
it('should merge user and workspace settings, with workspace taking precedence', () => {
@@ -215,6 +230,8 @@ describe('Settings Loading and Merging', () => {
sandbox: true,
coreTools: ['tool1'],
contextFileName: 'WORKSPACE_CONTEXT.md',
customThemes: {},
mcpServers: {},
});
});
@@ -262,6 +279,8 @@ describe('Settings Loading and Merging', () => {
coreTools: ['tool1'],
contextFileName: 'WORKSPACE_CONTEXT.md',
allowMCPServers: ['server1', 'server2'],
customThemes: {},
mcpServers: {},
});
});
@@ -373,6 +392,134 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockReturnValue('{}');
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.telemetry).toBeUndefined();
expect(settings.merged.customThemes).toEqual({});
expect(settings.merged.mcpServers).toEqual({});
});
it('should merge MCP servers correctly, with workspace taking precedence', () => {
(mockFsExistsSync as Mock).mockReturnValue(true);
const userSettingsContent = {
mcpServers: {
'user-server': {
command: 'user-command',
args: ['--user-arg'],
description: 'User MCP server',
},
'shared-server': {
command: 'user-shared-command',
description: 'User shared server config',
},
},
};
const workspaceSettingsContent = {
mcpServers: {
'workspace-server': {
command: 'workspace-command',
args: ['--workspace-arg'],
description: 'Workspace MCP server',
},
'shared-server': {
command: 'workspace-shared-command',
description: 'Workspace shared server config',
},
},
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.user.settings).toEqual(userSettingsContent);
expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
expect(settings.merged.mcpServers).toEqual({
'user-server': {
command: 'user-command',
args: ['--user-arg'],
description: 'User MCP server',
},
'workspace-server': {
command: 'workspace-command',
args: ['--workspace-arg'],
description: 'Workspace MCP server',
},
'shared-server': {
command: 'workspace-shared-command',
description: 'Workspace shared server config',
},
});
});
it('should handle MCP servers when only in user settings', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
);
const userSettingsContent = {
mcpServers: {
'user-only-server': {
command: 'user-only-command',
description: 'User only server',
},
},
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.mcpServers).toEqual({
'user-only-server': {
command: 'user-only-command',
description: 'User only server',
},
});
});
it('should handle MCP servers when only in workspace settings', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH,
);
const workspaceSettingsContent = {
mcpServers: {
'workspace-only-server': {
command: 'workspace-only-command',
description: 'Workspace only server',
},
},
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.mcpServers).toEqual({
'workspace-only-server': {
command: 'workspace-only-command',
description: 'Workspace only server',
},
});
});
it('should have mcpServers as empty object if not in any settings file', () => {
(mockFsExistsSync as Mock).mockReturnValue(false); // No settings files exist
(fs.readFileSync as Mock).mockReturnValue('{}');
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.mcpServers).toEqual({});
});
it('should handle JSON parsing errors gracefully', () => {
@@ -410,7 +557,10 @@ describe('Settings Loading and Merging', () => {
// Check that settings are empty due to parsing errors
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual({});
expect(settings.merged).toEqual({
customThemes: {},
mcpServers: {},
});
// Check that error objects are populated in settings.errors
expect(settings.errors).toBeDefined();
@@ -451,10 +601,13 @@ describe('Settings Loading and Merging', () => {
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
// @ts-expect-error: dynamic property for test
expect(settings.user.settings.apiKey).toBe('user_api_key_from_env');
// @ts-expect-error: dynamic property for test
expect(settings.user.settings.someUrl).toBe(
'https://test.com/user_api_key_from_env',
);
// @ts-expect-error: dynamic property for test
expect(settings.merged.apiKey).toBe('user_api_key_from_env');
delete process.env.TEST_API_KEY;
});
@@ -483,6 +636,7 @@ describe('Settings Loading and Merging', () => {
expect(settings.workspace.settings.nested.value).toBe(
'workspace_endpoint_from_env',
);
// @ts-expect-error: dynamic property for test
expect(settings.merged.endpoint).toBe('workspace_endpoint_from_env/api');
delete process.env.WORKSPACE_ENDPOINT;
});
@@ -512,13 +666,16 @@ describe('Settings Loading and Merging', () => {
const settings = loadSettings(MOCK_WORKSPACE_DIR);
// @ts-expect-error: dynamic property for test
expect(settings.user.settings.configValue).toBe(
'user_value_for_user_read',
);
// @ts-expect-error: dynamic property for test
expect(settings.workspace.settings.configValue).toBe(
'workspace_value_for_workspace_read',
);
// Merged should take workspace's resolved value
// @ts-expect-error: dynamic property for test
expect(settings.merged.configValue).toBe(
'workspace_value_for_workspace_read',
);
@@ -600,13 +757,16 @@ describe('Settings Loading and Merging', () => {
const settings = loadSettings(MOCK_WORKSPACE_DIR);
// @ts-expect-error: dynamic property for test
expect(settings.system.settings.configValue).toBe(
'system_value_for_system_read',
);
// @ts-expect-error: dynamic property for test
expect(settings.workspace.settings.configValue).toBe(
'workspace_value_for_workspace_read',
);
// Merged should take workspace's resolved value
// Merged should take system's resolved value
// @ts-expect-error: dynamic property for test
expect(settings.merged.configValue).toBe('system_value_for_system_read');
// Restore original environment variable state

View File

@@ -19,6 +19,7 @@ import {
import stripJsonComments from 'strip-json-comments';
import { DefaultLight } from '../ui/themes/default-light.js';
import { DefaultDark } from '../ui/themes/default.js';
import { CustomTheme } from '../ui/themes/theme.js';
export const SETTINGS_DIRECTORY_NAME = '.gemini';
export const USER_SETTINGS_DIR = path.join(homedir(), SETTINGS_DIRECTORY_NAME);
@@ -56,6 +57,7 @@ export interface AccessibilitySettings {
export interface Settings {
theme?: string;
customThemes?: Record<string, CustomTheme>;
selectedAuthType?: AuthType;
sandbox?: boolean | string;
coreTools?: string[];
@@ -84,6 +86,7 @@ export interface Settings {
// UI setting. Does not display the ANSI-controlled terminal title.
hideWindowTitle?: boolean;
hideTips?: boolean;
hideBanner?: boolean;
@@ -132,10 +135,24 @@ export class LoadedSettings {
}
private computeMergedSettings(): Settings {
const system = this.system.settings;
const user = this.user.settings;
const workspace = this.workspace.settings;
return {
...this.user.settings,
...this.workspace.settings,
...this.system.settings,
...user,
...workspace,
...system,
customThemes: {
...(user.customThemes || {}),
...(workspace.customThemes || {}),
...(system.customThemes || {}),
},
mcpServers: {
...(user.mcpServers || {}),
...(workspace.mcpServers || {}),
...(system.mcpServers || {}),
},
};
}
@@ -152,13 +169,12 @@ export class LoadedSettings {
}
}
setValue(
setValue<K extends keyof Settings>(
scope: SettingScope,
key: keyof Settings,
value: string | Record<string, MCPServerConfig> | undefined,
key: K,
value: Settings[K],
): void {
const settingsFile = this.forScope(scope);
// @ts-expect-error - value can be string | Record<string, MCPServerConfig>
settingsFile.settings[key] = value;
this._merged = this.computeMergedSettings();
saveSettings(settingsFile);