Refactor theme selection action and improve error handling#306691
Refactor theme selection action and improve error handling#306691mrleemurray wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the “Try New Default Themes” action to simplify the theme-apply flow by removing direct configuration updates and relying on IWorkbenchThemeService.setColorTheme(...) with centralized error handling.
Changes:
- Removed
IConfigurationServiceusage from the “Try New Default Themes” command implementation. - Replaced an inline async IIFE + try/catch with a single
setColorTheme(...).catch(onUnexpectedError)call.
| if (!theme) { | ||
| return; | ||
| } | ||
|
|
||
| (async () => { | ||
| try { | ||
| await themeService.setColorTheme(theme, 'auto'); | ||
| await configurationService.updateValue(ThemeSettings.PREFERRED_LIGHT_THEME, ThemeSettingDefaults.COLOR_THEME_LIGHT); | ||
| await configurationService.updateValue(ThemeSettings.PREFERRED_DARK_THEME, ThemeSettingDefaults.COLOR_THEME_DARK); | ||
| } catch (error) { | ||
| if (!isCancellationError(error)) { | ||
| onUnexpectedError(error); | ||
| } | ||
| } | ||
| })(); | ||
| themeService.setColorTheme(theme, 'auto').catch(onUnexpectedError); | ||
| })); |
There was a problem hiding this comment.
Removing the updates to ThemeSettings.PREFERRED_LIGHT_THEME / ThemeSettings.PREFERRED_DARK_THEME changes the behavior of “Try New Default Themes”: accepting a pick now only sets the current theme, but no longer switches both preferred light/dark themes to the new defaults. This can prevent auto-detect from using the new defaults for the other color scheme and can also leave preferences inconsistent if the user selects the opposite-scheme theme. Consider restoring those setting updates (with appropriate error handling) or otherwise ensuring both preferred theme settings are updated to ThemeSettingDefaults.COLOR_THEME_LIGHT and ThemeSettingDefaults.COLOR_THEME_DARK when this action is accepted.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Simplify the theme selection action by removing unnecessary configuration service calls and enhance error handling when setting the color theme. This improves the overall efficiency and reliability of the theme selection process.