microsoft · Rich Chiodo (rchiodo) · Apr 10, 2019 · Apr 6, 2019 · Apr 9, 2019 · Apr 9, 2019
diff --git a/.github/test_plan.md b/.github/test_plan.md
@@ -404,6 +404,14 @@ def test_failure():
         1. The progress bar should be interrupted and you should see a KeyboardInterrupt error message in the output
         1. Test the `Restart iPython kernel` command. Kernel should be restarted and you should see a status output message for the kernel restart
         1. Use the expand all input and collapse all input commands to collapse all cell inputs
+- [ ] Verify theming works
+    1. Start Python Interactive window
+    1. Add a cell with some comments
+    1. Switch VS Code theme to something else
+    1. Check that the cell you just added updates the comment color
+    1. Switch back and forth between a 'light' and a 'dark' theme
+    1. Check that the cell switches colors
+    1. Check that the buttons on the top change to their appropriate 'light' or 'dark' versions
 - [ ] Verify code lenses
     1. Check that `Run Cell` `Run Above` and `Run Below` all do the correct thing
 - [ ] Verify context menu navigation commands

diff --git a/news/2 Fixes/5136.md b/news/2 Fixes/5136.md
@@ -0,0 +1,2 @@
+Default colors when theme.json cannot be found.
+Fix python interactive window to update when theme changes.
diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts
@@ -1,9 +1,9 @@
 // tslint:disable:no-console no-any
-
 import { injectable } from 'inversify';
+
 import { sendTelemetryEvent } from '../telemetry';
-import { ILogger, LogLevel } from './types';
 import { isTestExecution } from './constants';
+import { ILogger, LogLevel } from './types';
 
 const PREFIX = 'Python Extension: ';
 

diff --git a/src/client/datascience/cellFactory.ts b/src/client/datascience/cellFactory.ts
@@ -56,7 +56,7 @@ export function generateCells(settings: IDataScienceSettings | undefined, code:
         let firstNonMarkdown = -1;
         parseForComments(split, (_s, _i) => noop(), (s, i) => {
             // Make sure there's actually some code.
-            if (s && s.length > 0) {
+            if (s && s.length > 0 && firstNonMarkdown === -1) {
                 firstNonMarkdown = splitMarkdown ? i : -1;
             }
         });

diff --git a/src/client/datascience/codeCssGenerator.ts b/src/client/datascience/codeCssGenerator.ts
@@ -9,16 +9,18 @@ import * as stripJsonComments from 'strip-json-comments';
 
 import { IWorkspaceService } from '../common/application/types';
 import { IConfigurationService, ILogger } from '../common/types';
-import { EXTENSION_ROOT_DIR } from '../constants';
 import { DefaultTheme, Identifiers } from './constants';
 import { ICodeCssGenerator, IThemeFinder } from './types';
 
 // tslint:disable:no-any
+const DarkTheme = 'dark';
+const LightTheme = 'light';
 
 // These are based on the colors generated by 'Default Light+' and are only set when we
 // are ignoring themes.
-//tslint:disable-next-line:no-multiline-string
-const DefaultStyle = `
+//tslint:disable:no-multiline-string object-literal-key-quotes
+const DefaultCssVars: { [key: string] : string } = {
+    LightTheme : `
     :root {
         --override-widget-background: #f3f3f3;
         --override-foreground: #000000;
@@ -28,7 +30,42 @@ const DefaultStyle = `
         --override-tabs-background: #f3f3f3;
         --override-progress-background: #0066bf;
     }
-`;
+`,
+    DarkTheme : `
+    :root {
+        --override-widget-background: #1e1e1e;
+        --override-foreground: #d4d4d4;
+        --override-background: #1e1e1e;
+        --override-selection-background: #264f78;
+        --override-watermark-color: #3f3f46;
+        --override-tabs-background: #252526;
+        --override-progress-background: #0066bf;
+    }
+`
+};
+
+// These colors below should match colors that come from either the Default Light+ theme or the Default Dark+ theme.
+// They are used when we can't find a theme json file.
+const DefaultColors: { [key: string] : string } = {
+    'light.comment'         : '#008000',
+    'light.constant.numeric': '#09885a',
+    'light.string'          : '#a31515',
+    'light.keyword.control' : '#AF00DB',
+    'light.keyword.operator': '#000000',
+    'light.variable'        : '#001080',
+    'light.entity.name.type': '#267f99',
+    'light.support.function': '#795E26',
+    'light.punctuation'     : '#000000',
+    'dark.comment'          : '#6A9955',
+    'dark.constant.numeric' : '#b5cea8',
+    'dark.string'           : '#ce9178',
+    'dark.keyword.control'  : '#C586C0',
+    'dark.keyword.operator' : '#d4d4d4',
+    'dark.variable'         : '#9CDCFE',
+    'dark.entity.name.type' : '#4EC9B0',
+    'dark.support.function' : '#DCDCAA',
+    'dark.punctuation'      : '#1e1e1e'
+};
 
 // This class generates css using the current theme in order to colorize code.
 //
@@ -45,17 +82,17 @@ export class CodeCssGenerator implements ICodeCssGenerator {
         @inject(ILogger) private logger: ILogger) {
     }
 
-    public generateThemeCss = async (): Promise<string> => {
+    public async generateThemeCss(isDark: boolean, theme: string): Promise<string> {
         let css : string = '';
         try {
             // First compute our current theme.
             const workbench = this.workspaceService.getConfiguration('workbench');
             const ignoreTheme = this.configService.getSettings().datascience.ignoreVscodeTheme ? true : false;
-            const theme = ignoreTheme ? DefaultTheme : workbench.get<string>('colorTheme');
-            const terminalCursor = workbench.get<string>('terminal.integrated.cursorStyle', 'block');
+            theme = ignoreTheme ? DefaultTheme : theme;
+            const terminalCursor = workbench ? workbench.get<string>('terminal.integrated.cursorStyle', 'block') : 'block';
             const editor = this.workspaceService.getConfiguration('editor', undefined);
-            const font = editor.get<string>('fontFamily');
-            const fontSize = editor.get<number>('fontSize');
+            const font = editor ? editor.get<string>('fontFamily', 'Consolas, \'Courier New\', monospace') : 'Consolas, \'Courier New\', monospace';
+            const fontSize = editor ? editor.get<number>('fontSize', 14) : 14;
 
             // Then we have to find where the theme resources are loaded from
             if (theme) {
@@ -65,7 +102,11 @@ export class CodeCssGenerator implements ICodeCssGenerator {
                 // The tokens object then contains the necessary data to generate our css
                 if (tokenColors && font && fontSize) {
                     this.logger.logInformation('Using colors to generate CSS ...');
-                    css = this.generateCss(theme, tokenColors, font, fontSize, terminalCursor, ignoreTheme);
+                    css = this.generateCss(theme, tokenColors, font, fontSize, terminalCursor, ignoreTheme ? LightTheme : undefined);
+                } else if (tokenColors === null && font && fontSize) {
+                    // No colors found. See if we can figure out what type of theme we have
+                    const style = isDark ? DarkTheme : LightTheme ;
+                    css = this.generateCss(theme, null, font, fontSize, terminalCursor, style);
                 }
             }
         } catch (err) {
@@ -92,43 +133,48 @@ export class CodeCssGenerator implements ICodeCssGenerator {
         });
     }
 
-    private getScopeStyle = (tokenColors: JSONArray, scope: string, secondary?: string): { color: string; fontStyle: string } => {
+    private getScopeStyle = (tokenColors: JSONArray | null, scope: string, secondary: string, defaultStyle: string | undefined): { color: string; fontStyle: string } => {
         // Search through the scopes on the json object
-        let match = this.matchTokenColor(tokenColors, scope);
-        if (match < 0 && secondary) {
-            match = this.matchTokenColor(tokenColors, secondary);
-        }
-        const found = match >= 0 ? tokenColors[match] as any : null;
-        if (found !== null) {
-            const settings = found.settings;
-            if (settings && settings !== null) {
-                const fontStyle = settings.fontStyle ? settings.fontStyle : 'normal';
-                const foreground = settings.foreground ? settings.foreground : 'var(--vscode-editor-foreground)';
-
-                return { fontStyle, color: foreground };
+        if (tokenColors) {
+            let match = this.matchTokenColor(tokenColors, scope);
+            if (match < 0 && secondary) {
+                match = this.matchTokenColor(tokenColors, secondary);
+            }
+            const found = match >= 0 ? tokenColors[match] as any : null;
+            if (found !== null) {
+                const settings = found.settings;
+                if (settings && settings !== null) {
+                    const fontStyle = settings.fontStyle ? settings.fontStyle : 'normal';
+                    const foreground = settings.foreground ? settings.foreground : 'var(--vscode-editor-foreground)';
+
+                    return { fontStyle, color: foreground };
+                }
             }
         }
 
         // Default to editor foreground
-        return { color: 'var(--vscode-editor-foreground)', fontStyle: 'normal' };
+        return { color: this.getDefaultColor(defaultStyle, scope), fontStyle: 'normal' };
+    }
+
+    private getDefaultColor(style: string | undefined, scope: string) : string {
+        return style ? DefaultColors[`${style}.${scope}`] : 'var(--override-foreground, var(--vscode-editor-foreground))';
     }
 
     // tslint:disable-next-line:max-func-body-length
-    private generateCss(theme: string, tokenColors: JSONArray, fontFamily: string, fontSize: number, cursorType: string, generateDefaults: boolean): string {
+    private generateCss(theme: string, tokenColors: JSONArray | null, fontFamily: string, fontSize: number, cursorType: string, defaultStyle: string | undefined): string {
         const escapedThemeName = Identifiers.GeneratedThemeName;
 
         // There's a set of values that need to be found
-        const commentStyle = this.getScopeStyle(tokenColors, 'comment');
-        const numericStyle = this.getScopeStyle(tokenColors, 'constant.numeric');
-        const stringStyle = this.getScopeStyle(tokenColors, 'string');
-        const keywordStyle = this.getScopeStyle(tokenColors, 'keyword.control', 'keyword');
-        const operatorStyle = this.getScopeStyle(tokenColors, 'keyword.operator', 'keyword');
-        const variableStyle = this.getScopeStyle(tokenColors, 'variable');
-        const entityTypeStyle = this.getScopeStyle(tokenColors, 'entity.name.type');
+        const commentStyle = this.getScopeStyle(tokenColors, 'comment', 'comment', defaultStyle);
+        const numericStyle = this.getScopeStyle(tokenColors, 'constant.numeric', 'constant', defaultStyle);
+        const stringStyle = this.getScopeStyle(tokenColors, 'string', 'string', defaultStyle);
+        const keywordStyle = this.getScopeStyle(tokenColors, 'keyword.control', 'keyword', defaultStyle);
+        const operatorStyle = this.getScopeStyle(tokenColors, 'keyword.operator', 'keyword', defaultStyle);
+        const variableStyle = this.getScopeStyle(tokenColors, 'variable', 'variable', defaultStyle);
+        const entityTypeStyle = this.getScopeStyle(tokenColors, 'entity.name.type', 'entity.name.type', defaultStyle);
         // const atomic = this.getScopeColor(tokenColors, 'atomic');
-        const builtinStyle = this.getScopeStyle(tokenColors, 'support.function');
-        const punctuationStyle = this.getScopeStyle(tokenColors, 'punctuation');
-        const overrides = generateDefaults ? DefaultStyle : '';
+        const builtinStyle = this.getScopeStyle(tokenColors, 'support.function', 'support.function', defaultStyle);
+        const punctuationStyle = this.getScopeStyle(tokenColors, 'punctuation', 'punctuation', defaultStyle);
 
         const def = 'var(--vscode-editor-foreground)';
 
@@ -150,7 +196,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
             --code-font-size: ${fontSize}px;
         }
 
-        ${overrides}
+        ${defaultStyle ? DefaultCssVars[defaultStyle] : undefined }
 
         .cm-header, .cm-strong {font-weight: bold;}
         .cm-em {font-style: italic;}
@@ -206,7 +252,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
         return [];
     }
 
-    private findTokenColors = async (theme: string): Promise<JSONArray> => {
+    private findTokenColors = async (theme: string): Promise<JSONArray | null> => {
 
         try {
             this.logger.logInformation('Attempting search for colors ...');
@@ -256,8 +302,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
             this.logger.logError(err);
         }
 
-        // We should return a default. The vscode-light theme
-        const defaultThemeFile = path.join(EXTENSION_ROOT_DIR, 'resources', 'defaultTheme.json');
-        return this.readTokenColors(defaultThemeFile);
+        // Force the colors to the defaults
+        return null;
     }
 }
diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts
@@ -162,3 +162,23 @@ export namespace LiveShareCommands {
     export const historyCreateSync = 'historyCreateSync';
     export const disposeServer = 'disposeServer';
 }
+
+export namespace CssMessages {
+    export const GetCssRequest = 'get_css_request';
+    export const GetCssResponse = 'get_css_response';
+}
+
+export namespace SharedMessages {
+    export const UpdateSettings = 'update_settings';
+    export const Started = 'started';
+}
+
+export interface IGetCssRequest {
+    isDark: boolean;
+}
+
+export interface IGetCssResponse {
+    css: string;
+    theme: string;
+    knownDark?: boolean;
+}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Default colors when theme.json cannot be found.
		Fix python interactive window to update when theme changes.
Ian Huff (IanMatthewHuff) Apr 10, 2019 • edited by rchiodo Loading Copy link Copy Markdown Member Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. This seems like a good candidate for a manual test case in our CTI test plan file. #Resolved Rich Chiodo (rchiodo) Apr 10, 2019 Copy link Copy Markdown Author Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I agree. Is that in our source somewhere? I can update it here. In reply to: 274056601 [](ancestors = 274056601) Ian Huff (IanMatthewHuff) Apr 10, 2019 Copy link Copy Markdown Member Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Yup. .github/test_plan.md In reply to: 274056754 [](ancestors = 274056754,274056601)