refactor: refactor settings to a nested structure (#7244)

This commit is contained in:
Gal Zahavi
2025-08-27 18:39:45 -07:00
committed by GitHub
parent b8a7bfd136
commit f22263c9e8
41 changed files with 2852 additions and 1424 deletions

View File

@@ -31,7 +31,7 @@ describe('AuthDialog', () => {
const settings: LoadedSettings = new LoadedSettings(
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
@@ -40,16 +40,21 @@ describe('AuthDialog', () => {
},
{
settings: {
selectedAuthType: AuthType.USE_GEMINI,
security: {
auth: {
selectedType: AuthType.USE_GEMINI,
},
},
},
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame } = renderWithProviders(
@@ -72,8 +77,8 @@ describe('AuthDialog', () => {
const settings: LoadedSettings = new LoadedSettings(
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
@@ -83,15 +88,16 @@ describe('AuthDialog', () => {
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame } = renderWithProviders(
@@ -110,8 +116,8 @@ describe('AuthDialog', () => {
const settings: LoadedSettings = new LoadedSettings(
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
@@ -121,15 +127,16 @@ describe('AuthDialog', () => {
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame } = renderWithProviders(
@@ -148,8 +155,8 @@ describe('AuthDialog', () => {
const settings: LoadedSettings = new LoadedSettings(
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
@@ -159,15 +166,16 @@ describe('AuthDialog', () => {
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame } = renderWithProviders(
@@ -187,8 +195,8 @@ describe('AuthDialog', () => {
const settings: LoadedSettings = new LoadedSettings(
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
@@ -198,15 +206,16 @@ describe('AuthDialog', () => {
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame } = renderWithProviders(
@@ -221,8 +230,8 @@ describe('AuthDialog', () => {
const settings: LoadedSettings = new LoadedSettings(
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
@@ -232,15 +241,16 @@ describe('AuthDialog', () => {
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame } = renderWithProviders(
@@ -257,8 +267,8 @@ describe('AuthDialog', () => {
const settings: LoadedSettings = new LoadedSettings(
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
@@ -268,15 +278,16 @@ describe('AuthDialog', () => {
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame } = renderWithProviders(
@@ -296,7 +307,7 @@ describe('AuthDialog', () => {
const onSelect = vi.fn();
const settings: LoadedSettings = new LoadedSettings(
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
@@ -305,18 +316,19 @@ describe('AuthDialog', () => {
},
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame, stdin, unmount } = renderWithProviders(
@@ -340,7 +352,7 @@ describe('AuthDialog', () => {
const onSelect = vi.fn();
const settings: LoadedSettings = new LoadedSettings(
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
@@ -349,18 +361,19 @@ describe('AuthDialog', () => {
},
{
settings: {
selectedAuthType: undefined,
customThemes: {},
security: { auth: { selectedType: undefined } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { lastFrame, stdin, unmount } = renderWithProviders(
@@ -387,7 +400,7 @@ describe('AuthDialog', () => {
const onSelect = vi.fn();
const settings: LoadedSettings = new LoadedSettings(
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
{
@@ -396,18 +409,19 @@ describe('AuthDialog', () => {
},
{
settings: {
selectedAuthType: AuthType.USE_GEMINI,
customThemes: {},
security: { auth: { selectedType: AuthType.LOGIN_WITH_GOOGLE } },
ui: { customThemes: {} },
mcpServers: {},
},
path: '',
},
{
settings: { customThemes: {}, mcpServers: {} },
settings: { ui: { customThemes: {} }, mcpServers: {} },
path: '',
},
[],
true,
new Set(),
);
const { stdin, unmount } = renderWithProviders(

View File

@@ -83,8 +83,8 @@ export function AuthDialog({
];
const initialAuthIndex = items.findIndex((item) => {
if (settings.merged.selectedAuthType) {
return item.value === settings.merged.selectedAuthType;
if (settings.merged.security?.auth?.selectedType) {
return item.value === settings.merged.security.auth.selectedType;
}
const defaultAuthType = parseDefaultAuthType(
@@ -119,7 +119,7 @@ export function AuthDialog({
if (errorMessage) {
return;
}
if (settings.merged.selectedAuthType === undefined) {
if (settings.merged.security?.auth?.selectedType === undefined) {
// Prevent exiting if no auth method is set
setErrorMessage(
'You must select an auth method to proceed. Press Ctrl+C twice to exit.',

View File

@@ -53,7 +53,7 @@ export function EditorSettingsDialog({
editorSettingsManager.getAvailableEditorDisplays();
const currentPreference =
settings.forScope(selectedScope).settings.preferredEditor;
settings.forScope(selectedScope).settings.general?.preferredEditor;
let editorIndex = currentPreference
? editorItems.findIndex(
(item: EditorDisplay) => item.type === currentPreference,
@@ -87,20 +87,26 @@ export function EditorSettingsDialog({
selectedScope === SettingScope.User
? SettingScope.Workspace
: SettingScope.User;
if (settings.forScope(otherScope).settings.preferredEditor !== undefined) {
if (
settings.forScope(otherScope).settings.general?.preferredEditor !==
undefined
) {
otherScopeModifiedMessage =
settings.forScope(selectedScope).settings.preferredEditor !== undefined
settings.forScope(selectedScope).settings.general?.preferredEditor !==
undefined
? `(Also modified in ${otherScope})`
: `(Modified in ${otherScope})`;
}
let mergedEditorName = 'None';
if (
settings.merged.preferredEditor &&
isEditorAvailable(settings.merged.preferredEditor)
settings.merged.general?.preferredEditor &&
isEditorAvailable(settings.merged.general?.preferredEditor)
) {
mergedEditorName =
EDITOR_DISPLAY_NAMES[settings.merged.preferredEditor as EditorType];
EDITOR_DISPLAY_NAMES[
settings.merged.general?.preferredEditor as EditorType
];
}
return (

View File

@@ -40,7 +40,7 @@ const createMockSettings = (
) =>
new LoadedSettings(
{
settings: { customThemes: {}, mcpServers: {}, ...systemSettings },
settings: { ui: { customThemes: {} }, mcpServers: {}, ...systemSettings },
path: '/system/settings.json',
},
{
@@ -49,18 +49,23 @@ const createMockSettings = (
},
{
settings: {
customThemes: {},
ui: { customThemes: {} },
mcpServers: {},
...userSettings,
},
path: '/user/settings.json',
},
{
settings: { customThemes: {}, mcpServers: {}, ...workspaceSettings },
settings: {
ui: { customThemes: {} },
mcpServers: {},
...workspaceSettings,
},
path: '/workspace/settings.json',
},
[],
true,
new Set(),
);
vi.mock('../contexts/SettingsContext.js', async () => {
@@ -156,7 +161,11 @@ describe('SettingsDialog', () => {
) =>
new LoadedSettings(
{
settings: { customThemes: {}, mcpServers: {}, ...systemSettings },
settings: {
ui: { customThemes: {} },
mcpServers: {},
...systemSettings,
},
path: '/system/settings.json',
},
{
@@ -165,18 +174,23 @@ describe('SettingsDialog', () => {
},
{
settings: {
customThemes: {},
ui: { customThemes: {} },
mcpServers: {},
...userSettings,
},
path: '/user/settings.json',
},
{
settings: { customThemes: {}, mcpServers: {}, ...workspaceSettings },
settings: {
ui: { customThemes: {} },
mcpServers: {},
...workspaceSettings,
},
path: '/workspace/settings.json',
},
[],
true,
new Set(),
);
describe('Initial Rendering', () => {
@@ -392,11 +406,11 @@ describe('SettingsDialog', () => {
// Wait for initial render
await waitFor(() => {
expect(lastFrame()).toContain('Hide Window Title');
expect(lastFrame()).toContain('Vim Mode');
});
// The UI should show the settings section is active and scope section is inactive
expect(lastFrame()).toContain('● Hide Window Title'); // Settings section active
expect(lastFrame()).toContain('● Vim Mode'); // Settings section active
expect(lastFrame()).toContain(' Apply To'); // Scope section inactive
// This test validates the initial state - scope selection behavior
@@ -814,11 +828,11 @@ describe('SettingsDialog', () => {
// Wait for initial render
await waitFor(() => {
expect(lastFrame()).toContain('Hide Window Title');
expect(lastFrame()).toContain('Vim Mode');
});
// Verify initial state: settings section active, scope section inactive
expect(lastFrame()).toContain('● Hide Window Title'); // Settings section active
expect(lastFrame()).toContain('● Vim Mode'); // Settings section active
expect(lastFrame()).toContain(' Apply To'); // Scope section inactive
// This test validates the rendered UI structure for tab navigation
@@ -876,12 +890,12 @@ describe('SettingsDialog', () => {
// Wait for initial render
await waitFor(() => {
expect(lastFrame()).toContain('Hide Window Title');
expect(lastFrame()).toContain('Vim Mode');
});
// Verify the complete UI is rendered with all necessary sections
expect(lastFrame()).toContain('Settings'); // Title
expect(lastFrame()).toContain('● Hide Window Title'); // Active setting
expect(lastFrame()).toContain('● Vim Mode'); // Active setting
expect(lastFrame()).toContain('Apply To'); // Scope section
expect(lastFrame()).toContain('1. User Settings'); // Scope options
expect(lastFrame()).toContain(

View File

@@ -153,7 +153,7 @@ export function SettingsDialog({
);
// Special handling for vim mode to sync with VimModeContext
if (key === 'vimMode' && newValue !== vimEnabled) {
if (key === 'general.vimMode' && newValue !== vimEnabled) {
// Call toggleVimEnabled to sync the VimModeContext local state
toggleVimEnabled().catch((error) => {
console.error('Failed to toggle vim mode:', error);

View File

@@ -46,13 +46,13 @@ export function ThemeDialog({
// Track the currently highlighted theme name
const [highlightedThemeName, setHighlightedThemeName] = useState<
string | undefined
>(settings.merged.theme || DEFAULT_THEME.name);
>(settings.merged.ui?.theme || DEFAULT_THEME.name);
// Generate theme items filtered by selected scope
const customThemes =
selectedScope === SettingScope.User
? settings.user.settings.customThemes || {}
: settings.merged.customThemes || {};
? settings.user.settings.ui?.customThemes || {}
: settings.merged.ui?.customThemes || {};
const builtInThemes = themeManager
.getAvailableThemes()
.filter((theme) => theme.type !== 'custom');
@@ -76,7 +76,7 @@ export function ThemeDialog({
const [selectInputKey, setSelectInputKey] = useState(Date.now());
// Find the index of the selected theme, but only if it exists in the list
const selectedThemeName = settings.merged.theme || DEFAULT_THEME.name;
const selectedThemeName = settings.merged.ui?.theme || DEFAULT_THEME.name;
const initialThemeIndex = themeItems.findIndex(
(item) => item.value === selectedThemeName,
);
@@ -128,7 +128,7 @@ export function ThemeDialog({
// Generate scope message for theme setting
const otherScopeModifiedMessage = getScopeMessageForSetting(
'theme',
'ui.theme',
selectedScope,
settings,
);