From b8fa38a6e8f60acf6489fde545a91d3ba4c395d7 Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Fri, 9 May 2025 10:20:08 -0700 Subject: [PATCH] feat: Improve theme not found handling Modify to return a boolean instead of throwing an error when a theme is not found. Update CLI startup and hook to handle the boolean return value for more graceful error handling. --- packages/cli/src/gemini.ts | 17 +------ packages/cli/src/ui/hooks/useThemeCommand.ts | 52 ++++---------------- packages/cli/src/ui/themes/theme-manager.ts | 13 ++++- 3 files changed, 23 insertions(+), 59 deletions(-) diff --git a/packages/cli/src/gemini.ts b/packages/cli/src/gemini.ts index 32e34c20..1f48a99b 100644 --- a/packages/cli/src/gemini.ts +++ b/packages/cli/src/gemini.ts @@ -24,23 +24,10 @@ async function main() { const settings = loadSettings(process.cwd()); const config = await loadCliConfig(settings.merged); if (settings.merged.theme) { - try { - themeManager.setActiveTheme(settings.merged.theme); - } catch (error: unknown) { + if (!themeManager.setActiveTheme(settings.merged.theme)) { // If the theme is not found during initial load, log a warning and continue. // The useThemeCommand hook in App.tsx will handle opening the dialog. - if ( - error instanceof Error && - error.message.includes('Theme') && - error.message.includes('not found') - ) { - console.warn( - `Warning: ${error instanceof Error ? error.message : String(error)}`, - ); - } else { - // Re-throw other errors to be caught by the main catch block - throw error; - } + console.warn(`Warning: Theme "${settings.merged.theme}" not found.`); } } diff --git a/packages/cli/src/ui/hooks/useThemeCommand.ts b/packages/cli/src/ui/hooks/useThemeCommand.ts index b1ae170f..d8b0ef6b 100644 --- a/packages/cli/src/ui/hooks/useThemeCommand.ts +++ b/packages/cli/src/ui/hooks/useThemeCommand.ts @@ -32,28 +32,12 @@ export const useThemeCommand = ( // Apply initial theme on component mount useEffect(() => { - try { - themeManager.setActiveTheme(effectiveTheme); - setThemeError(null); // Clear any previous theme error on success - } catch (error: unknown) { + if (!themeManager.setActiveTheme(effectiveTheme)) { // If theme is not found during initial load, open the theme selection dialog and set error message - if ( - error instanceof Error && - error.message.includes('Theme') && - error.message.includes('not found') - ) { - setIsThemeDialogOpen(true); - setThemeError( - `Error: ${error instanceof Error ? error.message : String(error)}`, - ); - } else { - console.error( - `Error setting initial theme: ${error instanceof Error ? error.message : String(error)}`, - ); - setThemeError( - `Error setting initial theme: ${error instanceof Error ? error.message : String(error)}`, - ); - } + setIsThemeDialogOpen(true); + setThemeError(`Theme "${effectiveTheme}" not found.`); + } else { + setThemeError(null); // Clear any previous theme error on success } }, [effectiveTheme, setThemeError]); // Re-run if effectiveTheme or setThemeError changes @@ -63,29 +47,13 @@ export const useThemeCommand = ( const applyTheme = useCallback( (themeName: string | undefined) => { - try { - themeManager.setActiveTheme(themeName); + if (!themeManager.setActiveTheme(themeName)) { + // If theme is not found, open the theme selection dialog and set error message + setIsThemeDialogOpen(true); + setThemeError(`Theme "${themeName}" not found.`); + } else { setForceRender((v) => v + 1); // Trigger potential re-render setThemeError(null); // Clear any previous theme error on success - } catch (error: unknown) { - // If theme is not found, open the theme selection dialog and set error message - if ( - error instanceof Error && - error.message.includes('Theme') && - error.message.includes('not found') - ) { - setIsThemeDialogOpen(true); - setThemeError( - `Error: ${error instanceof Error ? error.message : String(error)}`, - ); - } else { - console.error( - `Error setting theme: ${error instanceof Error ? error.message : String(error)}`, - ); - setThemeError( - `Error setting theme: ${error instanceof Error ? error.message : String(error)}`, - ); - } } }, [setForceRender, setThemeError], diff --git a/packages/cli/src/ui/themes/theme-manager.ts b/packages/cli/src/ui/themes/theme-manager.ts index d1f8df9c..e17fa5b7 100644 --- a/packages/cli/src/ui/themes/theme-manager.ts +++ b/packages/cli/src/ui/themes/theme-manager.ts @@ -73,14 +73,23 @@ class ThemeManager { /** * Sets the active theme. * @param themeName The name of the theme to activate. + * @returns True if the theme was successfully set, false otherwise. */ - setActiveTheme(themeName: string | undefined): void { + setActiveTheme(themeName: string | undefined): boolean { const foundTheme = this.findThemeByName(themeName); if (foundTheme) { this.activeTheme = foundTheme; + return true; } else { - throw new Error(`Theme "${themeName}" not found.`); + // If themeName is undefined, it means we want to set the default theme. + // If findThemeByName returns undefined (e.g. default theme is also not found for some reason) + // then this will return false. + if (themeName === undefined) { + this.activeTheme = DEFAULT_THEME; + return true; + } + return false; } }