Disable run-by-line and continue buttons when actively running by line (#12170)

IanMatthewHuff · web-flow · commit e03f44723940 · 2020-06-05T17:02:17.000-07:00
diff --git a/news/2 Fixes/12169.md b/news/2 Fixes/12169.md
@@ -0,0 +1 @@
+Disable run-by-line and continue buttons in run by line mode when running.
diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts
@@ -3,7 +3,7 @@
 'use strict';
 import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
 import { Uri } from 'vscode';
-import { IServerState } from '../../../datascience-ui/interactive-common/mainState';
+import { DebugState, IServerState } from '../../../datascience-ui/interactive-common/mainState';
 
 import type { KernelMessage } from '@jupyterlab/services';
 import { DebugProtocol } from 'vscode-debugprotocol';
@@ -135,6 +135,7 @@ export enum InteractiveWindowMessages {
     ShowContinue = 'show_continue',
     ShowBreak = 'show_break',
     ShowingIp = 'showing_ip',
+    DebugStateChange = 'debug_state_change',
     KernelIdle = 'kernel_idle'
 }
 
@@ -341,6 +342,11 @@ export interface IRenderComplete {
     ids: string[];
 }
 
+export interface IDebugStateChange {
+    oldState: DebugState;
+    newState: DebugState;
+}
+
 export interface IFocusedCellEditor {
     cellId: string;
 }
@@ -645,4 +651,5 @@ export class IInteractiveWindowMapping {
     public [InteractiveWindowMessages.Step]: never | undefined;
     public [InteractiveWindowMessages.ShowingIp]: never | undefined;
     public [InteractiveWindowMessages.KernelIdle]: never | undefined;
+    public [InteractiveWindowMessages.DebugStateChange]: IDebugStateChange;
 }
diff --git a/src/client/datascience/interactive-common/synchronization.ts b/src/client/datascience/interactive-common/synchronization.ts
@@ -106,6 +106,7 @@ const messageWithMessageTypes: MessageMapping<IInteractiveWindowMapping> & Messa
     [InteractiveWindowMessages.CollapseAll]: MessageType.syncWithLiveShare,
     [InteractiveWindowMessages.Continue]: MessageType.other,
     [InteractiveWindowMessages.CopyCodeCell]: MessageType.other,
+    [InteractiveWindowMessages.DebugStateChange]: MessageType.other,
     [InteractiveWindowMessages.DeleteAllCells]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare,
     [InteractiveWindowMessages.DoSave]: MessageType.other,
     [InteractiveWindowMessages.ExecutionRendered]: MessageType.other,
diff --git a/src/client/datascience/interactive-ipynb/nativeEditorRunByLineListener.ts b/src/client/datascience/interactive-ipynb/nativeEditorRunByLineListener.ts
@@ -105,11 +105,13 @@ export class NativeEditorRunByLineListener
 
     private async handleStep() {
         // User issued a step command.
+        this.postEmitter.fire({ message: InteractiveWindowMessages.ShowContinue, payload: undefined });
         return this.debugService.step();
     }
 
     private async handleContinue() {
         // User issued a continue command
+        this.postEmitter.fire({ message: InteractiveWindowMessages.ShowContinue, payload: undefined });
         return this.debugService.continue();
     }
 
diff --git a/src/datascience-ui/interactive-common/mainState.ts b/src/datascience-ui/interactive-common/mainState.ts
@@ -24,6 +24,17 @@ export enum CursorPos {
     Current
 }
 
+// The state we are in for run by line debugging
+export enum DebugState {
+    Break,
+    Design,
+    Run
+}
+
+export function activeDebugState(state: DebugState): boolean {
+    return state === DebugState.Break || state === DebugState.Run;
+}
+
 export interface ICellViewModel {
     cell: ICell;
     inputBlockShow: boolean;
@@ -43,7 +54,7 @@ export interface ICellViewModel {
     runDuringDebug?: boolean;
     codeVersion?: number;
     uiSideError?: string;
-    runningByLine: boolean;
+    runningByLine: DebugState;
     currentStack?: DebugProtocol.StackFrame[];
 }
 
@@ -212,7 +223,7 @@ export function createEditableCellVM(executionCount: number): ICellViewModel {
         cursorPos: CursorPos.Current,
         hasBeenRun: false,
         scrollCount: 0,
-        runningByLine: false
+        runningByLine: DebugState.Design
     };
 }
 
@@ -266,7 +277,7 @@ export function createCellVM(
         hasBeenRun: false,
         scrollCount: 0,
         runDuringDebug,
-        runningByLine: false
+        runningByLine: DebugState.Design
     };
 
     // Update the input text
diff --git a/src/datascience-ui/interactive-common/redux/reducers/helpers.ts b/src/datascience-ui/interactive-common/redux/reducers/helpers.ts
@@ -8,7 +8,7 @@ const cloneDeep = require('lodash/cloneDeep');
 import { CellState, ICell, IDataScienceExtraSettings } from '../../../../client/datascience/types';
 import { arePathsSame } from '../../../react-common/arePathsSame';
 import { detectBaseTheme } from '../../../react-common/themeDetector';
-import { ICellViewModel, IMainState } from '../../mainState';
+import { DebugState, ICellViewModel, IMainState } from '../../mainState';
 import { CommonActionType, CommonReducerArg } from './types';
 
 const StackLimit = 10;
@@ -96,7 +96,7 @@ export namespace Helpers {
                         source: newVMs[index].cell.data.source
                     }
                 },
-                runningByLine: finished ? false : newVMs[index].runningByLine
+                runningByLine: finished ? DebugState.Design : newVMs[index].runningByLine
             };
             newVMs[index] = newVM;
 
diff --git a/src/datascience-ui/interactive-common/redux/store.ts b/src/datascience-ui/interactive-common/redux/store.ts
@@ -14,7 +14,14 @@ import { MessageType } from '../../../client/datascience/interactive-common/sync
 import { BaseReduxActionPayload } from '../../../client/datascience/interactive-common/types';
 import { CssMessages } from '../../../client/datascience/messages';
 import { CellState } from '../../../client/datascience/types';
-import { getSelectedAndFocusedInfo, IMainState, ServerStatus } from '../../interactive-common/mainState';
+import {
+    activeDebugState,
+    DebugState,
+    getSelectedAndFocusedInfo,
+    ICellViewModel,
+    IMainState,
+    ServerStatus
+} from '../../interactive-common/mainState';
 import { getLocString } from '../../react-common/locReactSide';
 import { PostOffice } from '../../react-common/postOffice';
 import { combineReducers, createQueueableActionMiddleware, QueuableAction } from '../../react-common/reduxUtils';
@@ -240,13 +247,27 @@ function createTestMiddleware(): Redux.Middleware<{}, IStore> {
             sendMessage(InteractiveWindowMessages.KernelIdle);
         }
 
+        // Debug state changing
+        const oldState = getDebugState(prevState.main.cellVMs);
+        const newState = getDebugState(afterState.main.cellVMs);
+        if (oldState !== newState) {
+            sendMessage(InteractiveWindowMessages.DebugStateChange, { oldState, newState });
+        }
+
         if (action.type !== 'action.postOutgoingMessage') {
             sendMessage(`DISPATCHED_ACTION_${action.type}`, {});
         }
         return res;
     };
 }
 
+// Find the debug state for cell view models
+function getDebugState(vms: ICellViewModel[]): DebugState {
+    const firstNonDesign = vms.find((cvm) => activeDebugState(cvm.runningByLine));
+
+    return firstNonDesign ? firstNonDesign.runningByLine : DebugState.Design;
+}
+
 function createMiddleWare(testMode: boolean): Redux.Middleware<{}, IStore>[] {
     // Create the middleware that modifies actions to queue new actions
     const queueableActions = createQueueableActionMiddleware();
diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx
@@ -21,7 +21,7 @@ import { CellInput } from '../interactive-common/cellInput';
 import { CellOutput } from '../interactive-common/cellOutput';
 import { ExecutionCount } from '../interactive-common/executionCount';
 import { InformationMessages } from '../interactive-common/informationMessages';
-import { CursorPos, ICellViewModel, IFont } from '../interactive-common/mainState';
+import { activeDebugState, CursorPos, DebugState, ICellViewModel, IFont } from '../interactive-common/mainState';
 import { getOSType } from '../react-common/constants';
 import { IKeyboardEvent } from '../react-common/event';
 import { Image, ImageName } from '../react-common/image';
@@ -61,7 +61,7 @@ interface INativeCellBaseProps {
     focusPending: number;
     busy: boolean;
     useCustomEditorApi: boolean;
-    runningByLine: boolean;
+    runningByLine: DebugState;
     supportsRunByLine: boolean;
 }
 
@@ -611,7 +611,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
         };
         const toolbarClassName = this.props.cellVM.cell.data.cell_type === 'code' ? '' : 'markdown-toolbar';
 
-        if (this.props.runningByLine && !this.isMarkdownCell()) {
+        if (activeDebugState(this.props.runningByLine) && !this.isMarkdownCell()) {
             return (
                 <div className={toolbarClassName}>
                     <div className="native-editor-celltoolbar-middle">
@@ -620,7 +620,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
                             onClick={cont}
                             tooltip={getLocString('DataScience.continueRunByLine', 'Stop')}
                             hidden={this.isMarkdownCell()}
-                            disabled={this.props.busy}
+                            disabled={this.props.busy || this.props.runningByLine === DebugState.Run}
                         >
                             <div className="codicon codicon-button">{CodIcon.Stop}</div>
                         </ImageButton>
@@ -629,7 +629,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
                             onClick={step}
                             tooltip={getLocString('DataScience.step', 'Run next line')}
                             hidden={this.isMarkdownCell()}
-                            disabled={this.props.busy}
+                            disabled={this.props.busy || this.props.runningByLine === DebugState.Run}
                         >
                             <Image
                                 baseTheme={this.props.baseTheme}
diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx
@@ -8,7 +8,13 @@ import { NativeKeyboardCommandTelemetry, NativeMouseCommandTelemetry } from '../
 import { buildSettingsCss } from '../interactive-common/buildSettingsCss';
 import { ContentPanel, IContentPanelProps } from '../interactive-common/contentPanel';
 import { handleLinkClick } from '../interactive-common/handlers';
-import { getSelectedAndFocusedInfo, ICellViewModel, IMainState } from '../interactive-common/mainState';
+import {
+    activeDebugState,
+    DebugState,
+    getSelectedAndFocusedInfo,
+    ICellViewModel,
+    IMainState
+} from '../interactive-common/mainState';
 import { IMainWithVariables, IStore } from '../interactive-common/redux/store';
 import { IVariablePanelProps, VariablePanel } from '../interactive-common/variablePanel';
 import { getOSType } from '../react-common/constants';
@@ -225,7 +231,8 @@ ${buildSettingsCss(this.props.settings)}`}</style>
 
             case 'F10':
                 if (this.props.debugging) {
-                    const debuggingCell = this.props.cellVMs.find((cvm) => cvm.runningByLine);
+                    // Only allow step if debugging in break mode
+                    const debuggingCell = this.props.cellVMs.find((cvm) => cvm.runningByLine === DebugState.Break);
                     if (debuggingCell) {
                         this.props.step(debuggingCell.cell.id);
                     }
@@ -234,7 +241,8 @@ ${buildSettingsCss(this.props.settings)}`}</style>
                 break;
             case 'F5':
                 if (this.props.debugging) {
-                    const debuggingCell = this.props.cellVMs.find((cvm) => cvm.runningByLine);
+                    // Only allow continue if debugging in break mode
+                    const debuggingCell = this.props.cellVMs.find((cvm) => cvm.runningByLine === DebugState.Break);
                     if (debuggingCell) {
                         this.props.continue(debuggingCell.cell.id);
                     }
@@ -299,7 +307,7 @@ ${buildSettingsCss(this.props.settings)}`}</style>
             ) : null;
 
         const otherCellRunningByLine = this.props.cellVMs.find(
-            (cvm) => cvm.runningByLine && cvm.cell.id !== cellVM.cell.id
+            (cvm) => activeDebugState(cvm.runningByLine) && cvm.cell.id !== cellVM.cell.id
         );
         const maxOutputSize = this.props.settings.maxOutputSize;
         const outputSizeLimit = 10000;
diff --git a/src/datascience-ui/native-editor/redux/reducers/creation.ts b/src/datascience-ui/native-editor/redux/reducers/creation.ts
@@ -14,6 +14,7 @@ import {
     createCellVM,
     createEmptyCell,
     CursorPos,
+    DebugState,
     extractInputText,
     getSelectedAndFocusedInfo,
     ICellViewModel,
@@ -191,7 +192,7 @@ export namespace Creation {
             cursorPos: CursorPos.Current,
             hasBeenRun: false,
             scrollCount: 0,
-            runningByLine: false
+            runningByLine: DebugState.Design
         };
 
         Transfer.postModelRemoveAll(arg, newVM.cell.id);
@@ -247,7 +248,7 @@ export namespace Creation {
                 cursorPos: CursorPos.Current,
                 hasBeenRun: false,
                 scrollCount: 0,
-                runningByLine: false
+                runningByLine: DebugState.Design
             };
 
             // Send messages to other side to indicate the new add
diff --git a/src/datascience-ui/native-editor/redux/reducers/execution.ts b/src/datascience-ui/native-editor/redux/reducers/execution.ts
@@ -11,6 +11,7 @@ import { concatMultilineStringInput } from '../../../common';
 import { createCellFrom } from '../../../common/cellFactory';
 import {
     CursorPos,
+    DebugState,
     getSelectedAndFocusedInfo,
     ICellViewModel,
     IMainState
@@ -279,7 +280,7 @@ export namespace Execution {
             });
             const newVM = {
                 ...arg.prevState.cellVMs[index],
-                runningByLine: true
+                runningByLine: DebugState.Run
             };
             const newVMs = [...arg.prevState.cellVMs];
             newVMs[index] = newVM;
@@ -298,7 +299,7 @@ export namespace Execution {
         if (index >= 0) {
             const newVM = {
                 ...arg.prevState.cellVMs[index],
-                runningByLine: true,
+                runningByLine: DebugState.Break,
                 currentStack: arg.payload.data.frames
             };
             const newVMs = [...arg.prevState.cellVMs];
@@ -316,7 +317,7 @@ export namespace Execution {
         if (index >= 0) {
             const newVM = {
                 ...arg.prevState.cellVMs[index],
-                runningByLine: true,
+                runningByLine: DebugState.Run,
                 currentStack: undefined
             };
             const newVMs = [...arg.prevState.cellVMs];
diff --git a/src/test/datascience/debugger.functional.test.tsx b/src/test/datascience/debugger.functional.test.tsx
@@ -23,6 +23,7 @@ import {
     IJupyterDebugService,
     IJupyterExecution
 } from '../../client/datascience/types';
+import { DebugState } from '../../datascience-ui/interactive-common/mainState';
 import { ImageButton } from '../../datascience-ui/react-common/imageButton';
 import { DataScienceIocContainer } from './dataScienceIocContainer';
 import { takeSnapshot, writeDiffSnapshot } from './helpers';
@@ -389,4 +390,56 @@ suite('DataScience Debugger tests', () => {
             return createIOC();
         }
     );
+    runNativeTest(
+        'Run by line state check',
+        async () => {
+            // Create an editor so something is listening to messages
+            await createNewEditor(ioc);
+            const wrapper = ioc.wrapper!;
+
+            // Add a cell into the UI and wait for it to render and submit it.
+            await addCell(wrapper, ioc, 'a=1\na=2\na=3', true);
+
+            // Step into this cell using the button
+            let cell = getLastOutputCell(wrapper, 'NativeCell');
+            let ImageButtons = cell.find(ImageButton);
+            assert.equal(ImageButtons.length, 7, 'Cell buttons not found');
+            const runByLineButton = ImageButtons.at(3);
+            // tslint:disable-next-line: no-any
+            assert.equal((runByLineButton.instance().props as any).tooltip, 'Run by line');
+
+            const promise = waitForMessage(ioc, InteractiveWindowMessages.DebugStateChange, {
+                withPayload: (p) => {
+                    return p.oldState === DebugState.Design && p.newState === DebugState.Run;
+                }
+            });
+            runByLineButton.simulate('click');
+            await promise;
+
+            // We should be running, is the run by line button disabled?
+            cell = getLastOutputCell(wrapper, 'NativeCell');
+            ImageButtons = cell.find(ImageButton);
+            // tslint:disable-next-line: no-any
+            let runByLineButtonProps = ImageButtons.at(3).instance().props as any;
+            expect(runByLineButtonProps.disabled).to.equal(true, 'Run by line button not disabled when running');
+
+            // Now wait for break mode
+            const breakPromise = waitForMessage(ioc, InteractiveWindowMessages.DebugStateChange, {
+                withPayload: (p) => {
+                    return p.oldState === DebugState.Run && p.newState === DebugState.Break;
+                }
+            });
+            await breakPromise;
+
+            cell = getLastOutputCell(wrapper, 'NativeCell');
+            ImageButtons = cell.find(ImageButton);
+            // tslint:disable-next-line: no-any
+            runByLineButtonProps = ImageButtons.at(3).instance().props as any;
+            expect(runByLineButtonProps.disabled).to.equal(false, 'Run by line button not active in break mode');
+        },
+        () => {
+            ioc.setExperimentState(RunByLine.experiment, true);
+            return createIOC();
+        }
+    );
 });
diff --git a/src/test/datascience/interactivePanel.functional.test.tsx b/src/test/datascience/interactivePanel.functional.test.tsx
@@ -7,7 +7,7 @@ import { PYTHON_LANGUAGE } from '../../client/common/constants';
 import { CellState } from '../../client/datascience/types';
 import { InteractiveCellComponent } from '../../datascience-ui/history-react/interactiveCell';
 import { IInteractivePanelProps, InteractivePanel } from '../../datascience-ui/history-react/interactivePanel';
-import { CursorPos, ServerStatus } from '../../datascience-ui/interactive-common/mainState';
+import { CursorPos, DebugState, ServerStatus } from '../../datascience-ui/interactive-common/mainState';
 import { noop } from '../core';
 import { mountComponent } from './testHelpers';
 
@@ -57,7 +57,7 @@ suite('DataScience Interactive Panel', () => {
                 inputBlockText: '',
                 scrollCount: 0,
                 selected: false,
-                runningByLine: false
+                runningByLine: DebugState.Design
             },
             editorLoaded: noopAny,
             editorUnmounted: noopAny,
diff --git a/src/test/datascience/mainState.unit.test.ts b/src/test/datascience/mainState.unit.test.ts
diff --git a/src/test/datascience/testHelpers.tsx b/src/test/datascience/testHelpers.tsx

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Disable run-by-line and continue buttons in run by line mode when running.`
Original file line number	Diff line number	Diff line change
`@@ -105,11 +105,13 @@ export class NativeEditorRunByLineListener`
`105`	`105`
`106`	`106`	`private async handleStep() {`
`107`	`107`	`// User issued a step command.`
	`108`	`+ this.postEmitter.fire({ message: InteractiveWindowMessages.ShowContinue, payload: undefined });`
`108`	`109`	`return this.debugService.step();`
`109`	`110`	`}`
`110`	`111`
`111`	`112`	`private async handleContinue() {`
`112`	`113`	`// User issued a continue command`
	`114`	`+ this.postEmitter.fire({ message: InteractiveWindowMessages.ShowContinue, payload: undefined });`
`113`	`115`	`return this.debugService.continue();`
`114`	`116`	`}`
`115`	`117`