Fix colors behind errors in the Python Interactive window. (#4683)

For #3175 Also make sure empty markdown doesn't cause a split.  - [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. - [ ] 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.
microsoft · Mar 8, 2019 · 189119a · 189119a
1 parent ce6acf8
commit 189119a
Show file tree

Hide file tree

Showing 12 changed files with 41 additions and 12 deletions.
diff --git a/news/2 Fixes/3175.md b/news/2 Fixes/3175.md
@@ -0,0 +1 @@
+Add 'errorBackgroundColor' (defaulted to white/#FFFFFF) for errors in the Python Interactive window.
diff --git a/package.json b/package.json
@@ -1295,6 +1295,12 @@
                     "description": "Maximum size (in pixels) of text output in the Python Interactive window before a scrollbar appears. Set to -1 for infinity.",
                     "scope": "resource"
                 },
+                "python.dataScience.errorBackgroundColor": {
+                    "type": "string",
+                    "default": "#FFFFFF",
+                    "description": "Background color (in hex) for exception messages in the Python Interactive window.",
+                    "scope": "resource"
+                },
                 "python.dataScience.sendSelectionToInteractiveWindow": {
                     "type": "boolean",
                     "default": false,

diff --git a/src/client/common/types.ts b/src/client/common/types.ts
@@ -299,6 +299,7 @@ export interface IDataScienceSettings {
     markdownRegularExpression: string;
     codeRegularExpression: string;
     allowLiveShare? : boolean;
+    errorBackgroundColor: string;
 }
 
 export const IConfigurationService = Symbol('IConfigurationService');

diff --git a/src/client/datascience/cellFactory.ts b/src/client/datascience/cellFactory.ts
@@ -54,7 +54,12 @@ export function generateCells(settings: IDataScienceSettings | undefined, code:
         // We have at least one markdown. We might have to split it if there any lines that don't begin
         // with # or are inside a multiline comment
         let firstNonMarkdown = -1;
-        parseForComments(split, (s, i) => noop(), (s, i) => firstNonMarkdown = splitMarkdown ? i : -1);
+        parseForComments(split, (s, i) => noop(), (s, i) => {
+            // Make sure there's actually some code.
+            if (s && s.length > 0) {
+                firstNonMarkdown = splitMarkdown ? i : -1;
+            }
+        });
         if (firstNonMarkdown >= 0) {
             // Make sure if we split, the second cell has a new id. It's a new submission.
             return [

diff --git a/src/datascience-ui/history-react/MainPanel.tsx b/src/datascience-ui/history-react/MainPanel.tsx
@@ -222,6 +222,8 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
 
     private renderCells = () => {
         const maxOutputSize = getSettings().maxOutputSize;
+        const errorBackgroundColor = getSettings().errorBackgroundColor;
+        const actualErrorBackgroundColor = errorBackgroundColor ? errorBackgroundColor : '#FFFFFF';
         const maxTextSize = maxOutputSize && maxOutputSize < 10000 && maxOutputSize > 0 ? maxOutputSize : undefined;
         return this.state.cellVMs.map((cellVM: ICellViewModel, index: number) =>
             <ErrorBoundary key={index}>
@@ -235,6 +237,7 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
                     baseTheme={this.props.baseTheme}
                     codeTheme={this.props.codeTheme}
                     showWatermark={!this.state.submittedText}
+                    errorBackgroundColor={actualErrorBackgroundColor}
                     gotoCode={() => this.gotoCellCode(index)}
                     delete={() => this.deleteCell(index)}/>
             </ErrorBoundary>

diff --git a/src/datascience-ui/history-react/cell.tsx b/src/datascience-ui/history-react/cell.tsx
@@ -37,6 +37,7 @@ interface ICellProps {
     maxTextSize?: number;
     history: InputHistory | undefined;
     showWatermark: boolean;
+    errorBackgroundColor: string;
     gotoCode(): void;
     delete(): void;
     submitNewCode(code: string): void;
@@ -267,7 +268,7 @@ export class Cell extends React.Component<ICellProps> {
         return [<Transform data={source}/>];
     }
 
-    private renderWithTransform = (mimetype: string, output : nbformat.IOutput, index : number, renderWithScrollbars: boolean) => {
+    private renderWithTransform = (mimetype: string, output : nbformat.IOutput, index : number, renderWithScrollbars: boolean, forceLightTheme: boolean) => {
 
         // If we found a mimetype, use the transform
         if (mimetype) {
@@ -289,19 +290,23 @@ export class Cell extends React.Component<ICellProps> {
                         renderWithScrollbars = true;
                     }
 
+                    // Create a default set of properties
+                    const style: React.CSSProperties = {
+                    };
+
                     // Create a scrollbar style if necessary
                     if (renderWithScrollbars && this.props.maxTextSize) {
-                        const style: React.CSSProperties = {
-                            maxHeight : `${this.props.maxTextSize}px`,
-                            overflowY : 'auto',
-                            overflowX : 'auto'
-                        };
-
-                        return <div id='stylewrapper' key={index} style={style}><Transform data={data} /></div>;
-                    } else {
-                        return <Transform key={index} data={data} />;
+                        style.overflowX = 'auto';
+                        style.overflowY = 'auto';
+                        style.maxHeight = `${this.props.maxTextSize}px`;
+                    }
+
+                    // Change the background if necessary
+                    if (forceLightTheme) {
+                        style.backgroundColor = this.props.errorBackgroundColor;
                     }
 
+                    return <div id='stylewrapper' key={index} style={style}><Transform data={data} /></div>;
                 }
             } catch (ex) {
                 window.console.log('Error in rendering');
@@ -331,6 +336,7 @@ export class Cell extends React.Component<ICellProps> {
 
         // Only for text and error ouptut do we add scrollbars
         let addScrollbars = false;
+        let forceLightTheme = false;
 
         // Stream and error output need to be converted
         if (copy.output_type === 'stream') {
@@ -360,6 +366,7 @@ export class Cell extends React.Component<ICellProps> {
 
         } else if (copy.output_type === 'error') {
             addScrollbars = true;
+            forceLightTheme = true;
             const error = copy as nbformat.IError;
             try {
                 const converter = new ansiToHtml();
@@ -383,7 +390,7 @@ export class Cell extends React.Component<ICellProps> {
 
         // If that worked, use the transform
         if (mimetype) {
-            return this.renderWithTransform(mimetype, copy, index, addScrollbars);
+            return this.renderWithTransform(mimetype, copy, index, addScrollbars, forceLightTheme);
         }
 
         if (copy.data) {

diff --git a/src/datascience-ui/react-common/settingsReactSide.ts b/src/datascience-ui/react-common/settingsReactSide.ts
@@ -45,6 +45,7 @@ function load() {
             showCellInputCode: true,
             collapseCellInputCodeByDefault: true,
             maxOutputSize: 400,
+            errorBackgroundColor: '#FFFFFF',
             sendSelectionToInteractiveWindow: false,
             markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\<markdowncell\\>)',
             codeRegularExpression: '^(#\\s*%%|#\\s*\\<codecell\\>|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])',

diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts
@@ -272,6 +272,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
             collapseCellInputCodeByDefault: true,
             allowInput: true,
             maxOutputSize: 400,
+            errorBackgroundColor: '#FFFFFF',
             sendSelectionToInteractiveWindow: false,
             codeRegularExpression: '^(#\\s*%%|#\\s*\\<codecell\\>|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])',
             markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\<markdowncell\\>)'

diff --git a/src/test/datascience/editor-integration/codewatcher.unit.test.ts b/src/test/datascience/editor-integration/codewatcher.unit.test.ts
@@ -65,6 +65,7 @@ suite('DataScience Code Watcher Unit Tests', () => {
             collapseCellInputCodeByDefault: true,
             allowInput: true,
             maxOutputSize: 400,
+            errorBackgroundColor: '#FFFFFF',
             sendSelectionToInteractiveWindow: false,
             codeRegularExpression: '^(#\\s*%%|#\\s*\\<codecell\\>|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])',
             markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\<markdowncell\\>)'

diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts
@@ -528,6 +528,7 @@ suite('Jupyter Execution', async () => {
             collapseCellInputCodeByDefault: true,
             allowInput: true,
             maxOutputSize: 400,
+            errorBackgroundColor: '#FFFFFF',
             sendSelectionToInteractiveWindow: false,
             codeRegularExpression: '^(#\\s*%%|#\\s*\\<codecell\\>|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])',
             markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\<markdowncell\\>)',

diff --git a/src/test/datascience/historyCommandListener.unit.test.ts b/src/test/datascience/historyCommandListener.unit.test.ts
@@ -139,6 +139,7 @@ suite('History command listener', async () => {
             collapseCellInputCodeByDefault: true,
             allowInput: true,
             maxOutputSize: 400,
+            errorBackgroundColor: '#FFFFFF',
             sendSelectionToInteractiveWindow: false,
             codeRegularExpression: '^(#\\s*%%|#\\s*\\<codecell\\>|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])',
             markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\<markdowncell\\>)'

diff --git a/src/test/datascience/historyTestHelpers.ts b/src/test/datascience/historyTestHelpers.ts
@@ -261,6 +261,7 @@ export function defaultDataScienceSettings(): IDataScienceSettings {
         collapseCellInputCodeByDefault: true,
         allowInput: true,
         maxOutputSize: 400,
+        errorBackgroundColor: '#FFFFFF',
         sendSelectionToInteractiveWindow: false,
         codeRegularExpression: '^(#\\s*%%|#\\s*\\<codecell\\>|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])',
         markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\<markdowncell\\>)'