Fix so legacy custom themes still load. (#4757)

Co-authored-by: matt korwel <matt.korwel@gmail.com>
This commit is contained in:
Jacob Richman 2025-07-25 17:36:42 -07:00 committed by GitHub
parent b089845f1c
commit ad2ef080aa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 183 additions and 6 deletions

View File

@ -75,8 +75,18 @@ class ThemeManager {
)) {
const validation = validateCustomTheme(customThemeConfig);
if (validation.isValid) {
if (validation.warning) {
console.warn(`Theme "${name}": ${validation.warning}`);
}
const themeWithDefaults: CustomTheme = {
...DEFAULT_THEME.colors,
...customThemeConfig,
name: customThemeConfig.name || name,
type: 'custom',
};
try {
const theme = createCustomTheme(customThemeConfig);
const theme = createCustomTheme(themeWithDefaults);
this.customThemes.set(name, theme);
} catch (error) {
console.warn(`Failed to load custom theme "${name}":`, error);

View File

@ -0,0 +1,147 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import * as themeModule from './theme.js';
import { themeManager } from './theme-manager.js';
const { validateCustomTheme } = themeModule;
type CustomTheme = themeModule.CustomTheme;
describe('validateCustomTheme', () => {
const validTheme: CustomTheme = {
type: 'custom',
name: 'My Custom Theme',
Background: '#FFFFFF',
Foreground: '#000000',
LightBlue: '#ADD8E6',
AccentBlue: '#0000FF',
AccentPurple: '#800080',
AccentCyan: '#00FFFF',
AccentGreen: '#008000',
AccentYellow: '#FFFF00',
AccentRed: '#FF0000',
DiffAdded: '#00FF00',
DiffRemoved: '#FF0000',
Comment: '#808080',
Gray: '#808080',
};
it('should return isValid: true for a valid theme', () => {
const result = validateCustomTheme(validTheme);
expect(result.isValid).toBe(true);
expect(result.error).toBeUndefined();
});
it('should return isValid: false for a theme with a missing required field', () => {
const invalidTheme = {
...validTheme,
name: undefined as unknown as string,
};
const result = validateCustomTheme(invalidTheme);
expect(result.isValid).toBe(false);
expect(result.error).toBe('Missing required field: name');
});
it('should return isValid: false for a theme with an invalid color format', () => {
const invalidTheme = { ...validTheme, Background: 'not-a-color' };
const result = validateCustomTheme(invalidTheme);
expect(result.isValid).toBe(false);
expect(result.error).toBe(
'Invalid color format for Background: not-a-color',
);
});
it('should return isValid: false for a theme with an invalid name', () => {
const invalidTheme = { ...validTheme, name: ' ' };
const result = validateCustomTheme(invalidTheme);
expect(result.isValid).toBe(false);
expect(result.error).toBe('Invalid theme name: ');
});
it('should return isValid: true for a theme missing optional DiffAdded and DiffRemoved colors', () => {
const legacyTheme: Partial<CustomTheme> = { ...validTheme };
delete legacyTheme.DiffAdded;
delete legacyTheme.DiffRemoved;
const result = validateCustomTheme(legacyTheme);
expect(result.isValid).toBe(true);
expect(result.error).toBeUndefined();
});
it('should return a warning if DiffAdded and DiffRemoved are missing', () => {
const legacyTheme: Partial<CustomTheme> = { ...validTheme };
delete legacyTheme.DiffAdded;
delete legacyTheme.DiffRemoved;
const result = validateCustomTheme(legacyTheme);
expect(result.isValid).toBe(true);
expect(result.warning).toBe('Missing field(s) DiffAdded, DiffRemoved');
});
it('should return a warning if only DiffRemoved is missing', () => {
const legacyTheme: Partial<CustomTheme> = { ...validTheme };
delete legacyTheme.DiffRemoved;
const result = validateCustomTheme(legacyTheme);
expect(result.isValid).toBe(true);
expect(result.warning).toBe('Missing field(s) DiffRemoved');
});
it('should return isValid: false for a theme with an invalid DiffAdded color', () => {
const invalidTheme = { ...validTheme, DiffAdded: 'invalid' };
const result = validateCustomTheme(invalidTheme);
expect(result.isValid).toBe(false);
expect(result.error).toBe('Invalid color format for DiffAdded: invalid');
});
it('should return isValid: false for a theme with an invalid DiffRemoved color', () => {
const invalidTheme = { ...validTheme, DiffRemoved: 'invalid' };
const result = validateCustomTheme(invalidTheme);
expect(result.isValid).toBe(false);
expect(result.error).toBe('Invalid color format for DiffRemoved: invalid');
});
it('should return isValid: false for a theme with a very long name', () => {
const invalidTheme = { ...validTheme, name: 'a'.repeat(51) };
const result = validateCustomTheme(invalidTheme);
expect(result.isValid).toBe(false);
expect(result.error).toBe(`Invalid theme name: ${'a'.repeat(51)}`);
});
});
describe('themeManager.loadCustomThemes', () => {
const baseTheme: Omit<CustomTheme, 'DiffAdded' | 'DiffRemoved'> & {
DiffAdded?: string;
DiffRemoved?: string;
} = {
type: 'custom',
name: 'Test Theme',
Background: '#FFF',
Foreground: '#000',
LightBlue: '#ADD8E6',
AccentBlue: '#00F',
AccentPurple: '#808',
AccentCyan: '#0FF',
AccentGreen: '#080',
AccentYellow: '#FF0',
AccentRed: '#F00',
Comment: '#888',
Gray: '#888',
};
it('should use values from DEFAULT_THEME when DiffAdded and DiffRemoved are not provided', () => {
const { darkTheme } = themeModule;
const legacyTheme: Partial<CustomTheme> = { ...baseTheme };
delete legacyTheme.DiffAdded;
delete legacyTheme.DiffRemoved;
themeManager.loadCustomThemes({ 'Legacy Custom Theme': legacyTheme });
const result = themeManager.getTheme('Legacy Custom Theme')!;
expect(result.colors.DiffAdded).toBe(darkTheme.DiffAdded);
expect(result.colors.DiffRemoved).toBe(darkTheme.DiffRemoved);
expect(result.colors.AccentBlue).toBe(legacyTheme.AccentBlue);
expect(result.name).toBe(legacyTheme.name);
});
});

View File

@ -323,6 +323,7 @@ export function createCustomTheme(customTheme: CustomTheme): Theme {
export function validateCustomTheme(customTheme: Partial<CustomTheme>): {
isValid: boolean;
error?: string;
warning?: string;
} {
// Check required fields
const requiredFields: Array<keyof CustomTheme> = [
@ -336,12 +337,17 @@ export function validateCustomTheme(customTheme: Partial<CustomTheme>): {
'AccentGreen',
'AccentYellow',
'AccentRed',
'DiffAdded',
'DiffRemoved',
// 'DiffAdded' and 'DiffRemoved' are not required as they were added after
// the theme format was defined.
'Comment',
'Gray',
];
const recommendedFields: Array<keyof CustomTheme> = [
'DiffAdded',
'DiffRemoved',
];
for (const field of requiredFields) {
if (!customTheme[field]) {
return {
@ -351,6 +357,14 @@ export function validateCustomTheme(customTheme: Partial<CustomTheme>): {
}
}
const missingFields: string[] = [];
for (const field of recommendedFields) {
if (!customTheme[field]) {
missingFields.push(field);
}
}
// Validate color format (basic hex validation)
const colorFields: Array<keyof CustomTheme> = [
'Background',
@ -369,8 +383,8 @@ export function validateCustomTheme(customTheme: Partial<CustomTheme>): {
];
for (const field of colorFields) {
const color = customTheme[field] as string;
if (!isValidColor(color)) {
const color = customTheme[field] as string | undefined;
if (color !== undefined && !isValidColor(color)) {
return {
isValid: false,
error: `Invalid color format for ${field}: ${color}`,
@ -386,7 +400,13 @@ export function validateCustomTheme(customTheme: Partial<CustomTheme>): {
};
}
return { isValid: true };
return {
isValid: true,
warning:
missingFields.length > 0
? `Missing field(s) ${missingFields.join(', ')}`
: undefined,
};
}
/**