Skip to content

Commit deb3fa7

Browse files
rchiodorchiodo
authored
Fix problems that can occur when we can't find the theme json (microsoft#5177)
For #5136 Modify our react code to have a 'StyleInjector' that pulls our css values from the extension instead of having the extension generate them on startup. This is necessary because in the non theme case, we can't tell if we're dark or light. However VS Code knows and will tell us. When no json is found we use the 'vscode-light' or 'vscode-dark' to decide on a base theme. This has the side effect of also making our window update when the user updates the theme. No more reopening the window. <!-- If an item below does not apply to you, then go ahead and check it off as "done" and strikethrough the text, e.g.: - [x] ~Has unit tests & system/integration tests~ --> - [x] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR) - [x] Title summarizes what is changing - [x] Has a [news entry](https://github.com/Microsoft/vscode-python/tree/master/news) file (remember to thank yourself!) - [ ] Has sufficient logging. - [ ] Has telemetry for enhancements. - [x] Unit tests & system/integration tests are added/updated - [ ] [Test plan](https://github.com/Microsoft/vscode-python/blob/master/.github/test_plan.md) is updated as appropriate - [ ] [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package-lock.json) has been regenerated by running `npm install` (if dependencies have changed) - [ ] The wiki is updated with any design decisions/details.
1 parent 0332c65 commit deb3fa7

33 files changed

Lines changed: 852 additions & 507 deletions

‎.github/test_plan.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,14 @@ def test_failure():
404404
1. The progress bar should be interrupted and you should see a KeyboardInterrupt error message in the output
405405
1. Test the `Restart iPython kernel` command. Kernel should be restarted and you should see a status output message for the kernel restart
406406
1. Use the expand all input and collapse all input commands to collapse all cell inputs
407+
- [ ] Verify theming works
408+
1. Start Python Interactive window
409+
1. Add a cell with some comments
410+
1. Switch VS Code theme to something else
411+
1. Check that the cell you just added updates the comment color
412+
1. Switch back and forth between a 'light' and a 'dark' theme
413+
1. Check that the cell switches colors
414+
1. Check that the buttons on the top change to their appropriate 'light' or 'dark' versions
407415
- [ ] Verify code lenses
408416
1. Check that `Run Cell` `Run Above` and `Run Below` all do the correct thing
409417
- [ ] Verify context menu navigation commands

‎news/2 Fixes/5136.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Default colors when theme.json cannot be found.
2+
Fix python interactive window to update when theme changes.

‎src/client/common/logger.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// tslint:disable:no-console no-any
2-
32
import { injectable } from 'inversify';
3+
44
import { sendTelemetryEvent } from '../telemetry';
5-
import { ILogger, LogLevel } from './types';
65
import { isTestExecution } from './constants';
6+
import { ILogger, LogLevel } from './types';
77

88
const PREFIX = 'Python Extension: ';
99

‎src/client/datascience/cellFactory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export function generateCells(settings: IDataScienceSettings | undefined, code:
5656
let firstNonMarkdown = -1;
5757
parseForComments(split, (_s, _i) => noop(), (s, i) => {
5858
// Make sure there's actually some code.
59-
if (s && s.length > 0) {
59+
if (s && s.length > 0 && firstNonMarkdown === -1) {
6060
firstNonMarkdown = splitMarkdown ? i : -1;
6161
}
6262
});

‎src/client/datascience/codeCssGenerator.ts‎

Lines changed: 85 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,18 @@ import * as stripJsonComments from 'strip-json-comments';
99

1010
import { IWorkspaceService } from '../common/application/types';
1111
import { IConfigurationService, ILogger } from '../common/types';
12-
import { EXTENSION_ROOT_DIR } from '../constants';
1312
import { DefaultTheme, Identifiers } from './constants';
1413
import { ICodeCssGenerator, IThemeFinder } from './types';
1514

1615
// tslint:disable:no-any
16+
const DarkTheme = 'dark';
17+
const LightTheme = 'light';
1718

1819
// These are based on the colors generated by 'Default Light+' and are only set when we
1920
// are ignoring themes.
20-
//tslint:disable-next-line:no-multiline-string
21-
const DefaultStyle = `
21+
//tslint:disable:no-multiline-string object-literal-key-quotes
22+
const DefaultCssVars: { [key: string] : string } = {
23+
LightTheme : `
2224
:root {
2325
--override-widget-background: #f3f3f3;
2426
--override-foreground: #000000;
@@ -28,7 +30,42 @@ const DefaultStyle = `
2830
--override-tabs-background: #f3f3f3;
2931
--override-progress-background: #0066bf;
3032
}
31-
`;
33+
`,
34+
DarkTheme : `
35+
:root {
36+
--override-widget-background: #1e1e1e;
37+
--override-foreground: #d4d4d4;
38+
--override-background: #1e1e1e;
39+
--override-selection-background: #264f78;
40+
--override-watermark-color: #3f3f46;
41+
--override-tabs-background: #252526;
42+
--override-progress-background: #0066bf;
43+
}
44+
`
45+
};
46+
47+
// These colors below should match colors that come from either the Default Light+ theme or the Default Dark+ theme.
48+
// They are used when we can't find a theme json file.
49+
const DefaultColors: { [key: string] : string } = {
50+
'light.comment' : '#008000',
51+
'light.constant.numeric': '#09885a',
52+
'light.string' : '#a31515',
53+
'light.keyword.control' : '#AF00DB',
54+
'light.keyword.operator': '#000000',
55+
'light.variable' : '#001080',
56+
'light.entity.name.type': '#267f99',
57+
'light.support.function': '#795E26',
58+
'light.punctuation' : '#000000',
59+
'dark.comment' : '#6A9955',
60+
'dark.constant.numeric' : '#b5cea8',
61+
'dark.string' : '#ce9178',
62+
'dark.keyword.control' : '#C586C0',
63+
'dark.keyword.operator' : '#d4d4d4',
64+
'dark.variable' : '#9CDCFE',
65+
'dark.entity.name.type' : '#4EC9B0',
66+
'dark.support.function' : '#DCDCAA',
67+
'dark.punctuation' : '#1e1e1e'
68+
};
3269

3370
// This class generates css using the current theme in order to colorize code.
3471
//
@@ -45,17 +82,17 @@ export class CodeCssGenerator implements ICodeCssGenerator {
4582
@inject(ILogger) private logger: ILogger) {
4683
}
4784

48-
public generateThemeCss = async (): Promise<string> => {
85+
public async generateThemeCss(isDark: boolean, theme: string): Promise<string> {
4986
let css : string = '';
5087
try {
5188
// First compute our current theme.
5289
const workbench = this.workspaceService.getConfiguration('workbench');
5390
const ignoreTheme = this.configService.getSettings().datascience.ignoreVscodeTheme ? true : false;
54-
const theme = ignoreTheme ? DefaultTheme : workbench.get<string>('colorTheme');
55-
const terminalCursor = workbench.get<string>('terminal.integrated.cursorStyle', 'block');
91+
theme = ignoreTheme ? DefaultTheme : theme;
92+
const terminalCursor = workbench ? workbench.get<string>('terminal.integrated.cursorStyle', 'block') : 'block';
5693
const editor = this.workspaceService.getConfiguration('editor', undefined);
57-
const font = editor.get<string>('fontFamily');
58-
const fontSize = editor.get<number>('fontSize');
94+
const font = editor ? editor.get<string>('fontFamily', 'Consolas, \'Courier New\', monospace') : 'Consolas, \'Courier New\', monospace';
95+
const fontSize = editor ? editor.get<number>('fontSize', 14) : 14;
5996

6097
// Then we have to find where the theme resources are loaded from
6198
if (theme) {
@@ -65,7 +102,11 @@ export class CodeCssGenerator implements ICodeCssGenerator {
65102
// The tokens object then contains the necessary data to generate our css
66103
if (tokenColors && font && fontSize) {
67104
this.logger.logInformation('Using colors to generate CSS ...');
68-
css = this.generateCss(theme, tokenColors, font, fontSize, terminalCursor, ignoreTheme);
105+
css = this.generateCss(theme, tokenColors, font, fontSize, terminalCursor, ignoreTheme ? LightTheme : undefined);
106+
} else if (tokenColors === null && font && fontSize) {
107+
// No colors found. See if we can figure out what type of theme we have
108+
const style = isDark ? DarkTheme : LightTheme ;
109+
css = this.generateCss(theme, null, font, fontSize, terminalCursor, style);
69110
}
70111
}
71112
} catch (err) {
@@ -92,43 +133,48 @@ export class CodeCssGenerator implements ICodeCssGenerator {
92133
});
93134
}
94135

95-
private getScopeStyle = (tokenColors: JSONArray, scope: string, secondary?: string): { color: string; fontStyle: string } => {
136+
private getScopeStyle = (tokenColors: JSONArray | null, scope: string, secondary: string, defaultStyle: string | undefined): { color: string; fontStyle: string } => {
96137
// Search through the scopes on the json object
97-
let match = this.matchTokenColor(tokenColors, scope);
98-
if (match < 0 && secondary) {
99-
match = this.matchTokenColor(tokenColors, secondary);
100-
}
101-
const found = match >= 0 ? tokenColors[match] as any : null;
102-
if (found !== null) {
103-
const settings = found.settings;
104-
if (settings && settings !== null) {
105-
const fontStyle = settings.fontStyle ? settings.fontStyle : 'normal';
106-
const foreground = settings.foreground ? settings.foreground : 'var(--vscode-editor-foreground)';
107-
108-
return { fontStyle, color: foreground };
138+
if (tokenColors) {
139+
let match = this.matchTokenColor(tokenColors, scope);
140+
if (match < 0 && secondary) {
141+
match = this.matchTokenColor(tokenColors, secondary);
142+
}
143+
const found = match >= 0 ? tokenColors[match] as any : null;
144+
if (found !== null) {
145+
const settings = found.settings;
146+
if (settings && settings !== null) {
147+
const fontStyle = settings.fontStyle ? settings.fontStyle : 'normal';
148+
const foreground = settings.foreground ? settings.foreground : 'var(--vscode-editor-foreground)';
149+
150+
return { fontStyle, color: foreground };
151+
}
109152
}
110153
}
111154

112155
// Default to editor foreground
113-
return { color: 'var(--vscode-editor-foreground)', fontStyle: 'normal' };
156+
return { color: this.getDefaultColor(defaultStyle, scope), fontStyle: 'normal' };
157+
}
158+
159+
private getDefaultColor(style: string | undefined, scope: string) : string {
160+
return style ? DefaultColors[`${style}.${scope}`] : 'var(--override-foreground, var(--vscode-editor-foreground))';
114161
}
115162

116163
// tslint:disable-next-line:max-func-body-length
117-
private generateCss(theme: string, tokenColors: JSONArray, fontFamily: string, fontSize: number, cursorType: string, generateDefaults: boolean): string {
164+
private generateCss(theme: string, tokenColors: JSONArray | null, fontFamily: string, fontSize: number, cursorType: string, defaultStyle: string | undefined): string {
118165
const escapedThemeName = Identifiers.GeneratedThemeName;
119166

120167
// There's a set of values that need to be found
121-
const commentStyle = this.getScopeStyle(tokenColors, 'comment');
122-
const numericStyle = this.getScopeStyle(tokenColors, 'constant.numeric');
123-
const stringStyle = this.getScopeStyle(tokenColors, 'string');
124-
const keywordStyle = this.getScopeStyle(tokenColors, 'keyword.control', 'keyword');
125-
const operatorStyle = this.getScopeStyle(tokenColors, 'keyword.operator', 'keyword');
126-
const variableStyle = this.getScopeStyle(tokenColors, 'variable');
127-
const entityTypeStyle = this.getScopeStyle(tokenColors, 'entity.name.type');
168+
const commentStyle = this.getScopeStyle(tokenColors, 'comment', 'comment', defaultStyle);
169+
const numericStyle = this.getScopeStyle(tokenColors, 'constant.numeric', 'constant', defaultStyle);
170+
const stringStyle = this.getScopeStyle(tokenColors, 'string', 'string', defaultStyle);
171+
const keywordStyle = this.getScopeStyle(tokenColors, 'keyword.control', 'keyword', defaultStyle);
172+
const operatorStyle = this.getScopeStyle(tokenColors, 'keyword.operator', 'keyword', defaultStyle);
173+
const variableStyle = this.getScopeStyle(tokenColors, 'variable', 'variable', defaultStyle);
174+
const entityTypeStyle = this.getScopeStyle(tokenColors, 'entity.name.type', 'entity.name.type', defaultStyle);
128175
// const atomic = this.getScopeColor(tokenColors, 'atomic');
129-
const builtinStyle = this.getScopeStyle(tokenColors, 'support.function');
130-
const punctuationStyle = this.getScopeStyle(tokenColors, 'punctuation');
131-
const overrides = generateDefaults ? DefaultStyle : '';
176+
const builtinStyle = this.getScopeStyle(tokenColors, 'support.function', 'support.function', defaultStyle);
177+
const punctuationStyle = this.getScopeStyle(tokenColors, 'punctuation', 'punctuation', defaultStyle);
132178

133179
const def = 'var(--vscode-editor-foreground)';
134180

@@ -150,7 +196,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
150196
--code-font-size: ${fontSize}px;
151197
}
152198
153-
${overrides}
199+
${defaultStyle ? DefaultCssVars[defaultStyle] : undefined }
154200
155201
.cm-header, .cm-strong {font-weight: bold;}
156202
.cm-em {font-style: italic;}
@@ -206,7 +252,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
206252
return [];
207253
}
208254

209-
private findTokenColors = async (theme: string): Promise<JSONArray> => {
255+
private findTokenColors = async (theme: string): Promise<JSONArray | null> => {
210256

211257
try {
212258
this.logger.logInformation('Attempting search for colors ...');
@@ -256,8 +302,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
256302
this.logger.logError(err);
257303
}
258304

259-
// We should return a default. The vscode-light theme
260-
const defaultThemeFile = path.join(EXTENSION_ROOT_DIR, 'resources', 'defaultTheme.json');
261-
return this.readTokenColors(defaultThemeFile);
305+
// Force the colors to the defaults
306+
return null;
262307
}
263308
}

‎src/client/datascience/constants.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,23 @@ export namespace LiveShareCommands {
162162
export const historyCreateSync = 'historyCreateSync';
163163
export const disposeServer = 'disposeServer';
164164
}
165+
166+
export namespace CssMessages {
167+
export const GetCssRequest = 'get_css_request';
168+
export const GetCssResponse = 'get_css_response';
169+
}
170+
171+
export namespace SharedMessages {
172+
export const UpdateSettings = 'update_settings';
173+
export const Started = 'started';
174+
}
175+
176+
export interface IGetCssRequest {
177+
isDark: boolean;
178+
}
179+
180+
export interface IGetCssResponse {
181+
css: string;
182+
theme: string;
183+
knownDark?: boolean;
184+
}

0 commit comments

Comments
 (0)