Merge branch 'master' into debug

azure-pipelines.yml

@@ -16,7 +16,7 @@ steps:
 
 - bash: |
     source activate jupyterlab-debugger
-    conda install --yes --quiet -c conda-forge nodejs xeus-python=0.6.5 ptvsd python=$PYTHON_VERSION
+    conda install --yes --quiet -c conda-forge nodejs xeus-python=0.6.6 ptvsd python=$PYTHON_VERSION
     python -m pip install -U --pre jupyterlab
   displayName: Install dependencies
 

binder/environment.yml

@@ -6,4 +6,4 @@ dependencies:
 - nodejs
 - notebook=6
 - ptvsd
-- xeus-python=0.6.5
+- xeus-python=0.6.6

examples/index.ipynb

@@ -13,6 +13,17 @@
     "result = add(1, 2)\n",
     "print(result)"
    ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "i = 0\n",
+    "i += 1\n",
+    "print(i)"
+   ]
   }
  ],
  "metadata": {

src/callstack/index.ts

@@ -96,6 +96,7 @@ export namespace Callstack {
     set frames(newFrames: IFrame[]) {
       this._state = newFrames;
       this._framesChanged.emit(newFrames);
+      this.frame = newFrames[0];
     }
 
     get frames(): IFrame[] {

src/handlers/cell.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Jupyter Development Team.
 // Distributed under the terms of the Modified BSD License.
 
-import { CodeCell, ICellModel } from '@jupyterlab/cells';
+import { Cell, CodeCell, ICellModel } from '@jupyterlab/cells';
 
 import { CodeMirrorEditor } from '@jupyterlab/codemirror';
 
@@ -15,6 +15,8 @@ import { Editor } from 'codemirror';
 
 import { Breakpoints, SessionTypes } from '../breakpoints';
 
+import { Callstack } from '../callstack';
+
 import { Debugger } from '../debugger';
 
 import { IDebugger } from '../tokens';
@@ -41,14 +43,14 @@ export class CellManager implements IDisposable {
     }
     this.breakpointsModel = this._debuggerModel.breakpointsModel;
 
-    this._debuggerModel.variablesModel.changed.connect(() => {
-      this.cleanupHighlight();
-      const firstFrame = this._debuggerModel.callstackModel.frames[0];
-      if (!firstFrame) {
-        return;
+    this._debuggerModel.callstackModel.currentFrameChanged.connect(
+      (_, frame) => {
+        CellManager.cleanupHighlight(this.activeCell);
+        if (!frame) {
+          return;
+        }
       }
-      this.showCurrentLine(firstFrame.line);
-    });
+    );
 
     this.breakpointsModel.changed.connect(async () => {
       if (
@@ -73,26 +75,6 @@ export class CellManager implements IDisposable {
     }
   }
 
-  private showCurrentLine(lineNumber: number) {
-    if (!this.activeCell || !this.activeCell.inputArea) {
-      return;
-    }
-    const editor = this.activeCell.editor as CodeMirrorEditor;
-    this.cleanupHighlight();
-    editor.editor.addLineClass(lineNumber - 1, 'wrap', LINE_HIGHLIGHT_CLASS);
-  }
-
-  // TODO: call when the debugger stops
-  private cleanupHighlight() {
-    if (!this.activeCell || this.activeCell.isDisposed) {
-      return;
-    }
-    const editor = this.activeCell.editor as CodeMirrorEditor;
-    editor.doc.eachLine(line => {
-      editor.editor.removeLineClass(line, 'wrap', LINE_HIGHLIGHT_CLASS);
-    });
-  }
-
   dispose(): void {
     if (this.isDisposed) {
       return;
@@ -104,7 +86,7 @@ export class CellManager implements IDisposable {
       this._cellMonitor.dispose();
     }
     this.removeListener(this.activeCell);
-    this.cleanupHighlight();
+    CellManager.cleanupHighlight(this.activeCell);
     Signal.clearData(this);
   }
 
@@ -303,6 +285,31 @@ export namespace CellManager {
     activeCell?: CodeCell;
     type: SessionTypes;
   }
+
+  /**
+   * Highlight the current line of the frame in the given cell.
+   * @param cell The cell to highlight.
+   * @param frame The frame with the current line number.
+   */
+  export function showCurrentLine(cell: Cell, frame: Callstack.IFrame) {
+    const editor = cell.editor as CodeMirrorEditor;
+    cleanupHighlight(cell);
+    editor.editor.addLineClass(frame.line - 1, 'wrap', LINE_HIGHLIGHT_CLASS);
+  }
+
+  /**
+   * Remove all line highlighting indicators for the given cell.
+   * @param cell The cell to cleanup.
+   */
+  export function cleanupHighlight(cell: Cell) {
+    if (!cell || cell.isDisposed) {
+      return;
+    }
+    const editor = cell.editor as CodeMirrorEditor;
+    editor.doc.eachLine(line => {
+      editor.editor.removeLineClass(line, 'wrap', LINE_HIGHLIGHT_CLASS);
+    });
+  }
 }
 
 export interface ILineInfo {

src/handlers/notebook.ts

@@ -15,6 +15,8 @@ import { Debugger } from '../debugger';
 
 import { IDebugger } from '../tokens';
 
+import { Callstack } from '../callstack';
+
 import { CellManager } from './cell';
 
 export class NotebookHandler implements IDisposable {
@@ -37,6 +39,11 @@ export class NotebookHandler implements IDisposable {
       this.onActiveCellChanged,
       this
     );
+
+    this.debuggerModel.callstackModel.currentFrameChanged.connect(
+      this.onCurrentFrameChanged,
+      this
+    );
   }
 
   isDisposed: boolean;
@@ -60,6 +67,34 @@ export class NotebookHandler implements IDisposable {
     this.cellManager.activeCell = codeCell;
   }
 
+  private onCurrentFrameChanged(
+    callstackModel: Callstack.Model,
+    frame: Callstack.IFrame
+  ) {
+    const notebook = this.notebookTracker.currentWidget;
+    if (!notebook) {
+      return;
+    }
+
+    const cells = notebook.content.widgets;
+    cells.forEach(cell => CellManager.cleanupHighlight(cell));
+
+    if (!frame) {
+      return;
+    }
+
+    cells.forEach((cell, i) => {
+      // check the event is for the correct cell
+      const code = cell.model.value.text;
+      const cellId = this.debuggerService.getCellId(code);
+      if (frame.source.path !== cellId) {
+        return;
+      }
+      notebook.content.activeCellIndex = i;
+      CellManager.showCurrentLine(cell, frame);
+    });
+  }
+
   private notebookTracker: INotebookTracker;
   private debuggerModel: Debugger.Model;
   private debuggerService: IDebugger;

src/index.ts

@@ -277,6 +277,10 @@ const main: JupyterFrontEndPlugin<IDebugger> = {
         sidebar.title.label = 'Environment';
         shell.add(sidebar, 'right', { activate: false });
 
+        if (restorer) {
+          restorer.add(sidebar, 'debugger-sidebar');
+        }
+
         await service.restoreState(true);
       }
     });

src/service.ts

@@ -88,6 +88,7 @@ export class DebugService implements IDebugger {
       } else if (event.event === 'continued') {
         this._stoppedThreads.delete(event.body.threadId);
         this.clearModel();
+        this.clearSignals();
       }
       this._eventMessage.emit(event);
     });
@@ -193,12 +194,10 @@ export class DebugService implements IDebugger {
     // No need to dump the cells again, we can simply
     // resend the breakpoints to the kernel and update
     // the model.
-    breakpoints.forEach(async (bp, path, _) => {
-      const sourceBreakpoints = Private.toSourceBreakpoints(bp);
-      await this.setBreakpoints(sourceBreakpoints, path);
-    });
-
-    this.model.breakpointsModel.restoreBreakpoints(breakpoints);
+    for (const [source, bps] of breakpoints) {
+      const sourceBreakpoints = Private.toSourceBreakpoints(bps);
+      await this.setBreakpoints(sourceBreakpoints, source);
+    }
   }
 
   /**
@@ -216,8 +215,8 @@ export class DebugService implements IDebugger {
 
     this.setHashParameters(reply.body.hashMethod, reply.body.hashSeed);
     this.setTmpFileParameters(
-      reply.body.tmp_file_prefix,
-      reply.body.tmp_file_suffix
+      reply.body.tmpFilePrefix,
+      reply.body.tmpFileSuffix
     );
 
     const breakpoints = reply.body.breakpoints;
@@ -350,34 +349,21 @@ export class DebugService implements IDebugger {
   }
 
   getAllFrames = async () => {
-    const stackFrames = await this.getFrames(this.currentThread());
-
-    stackFrames.forEach(async (frame, index) => {
-      const scopes = await this.getScopes(frame);
-      const variables = await this.getVariables(scopes);
-      const values = this.convertScope(scopes, variables);
-      this.frames.push({
-        id: frame.id,
-        scopes: values
-      });
-      if (index === 0) {
-        this._model.variablesModel.scopes = values;
-      }
-    });
-
-    if (stackFrames) {
-      this._model.callstackModel.frames = stackFrames;
-    }
-
     this._model.callstackModel.currentFrameChanged.connect(this.onChangeFrame);
     this._model.variablesModel.variableExpanded.connect(this.getVariable);
+
+    const stackFrames = await this.getFrames(this.currentThread());
+    this._model.callstackModel.frames = stackFrames;
   };
 
-  onChangeFrame = (_: Callstack.Model, update: Callstack.IFrame) => {
-    const frame = this.frames.find(ele => ele.id === update.id);
-    if (frame && frame.scopes) {
-      this._model.variablesModel.scopes = frame.scopes;
+  onChangeFrame = async (_: Callstack.Model, frame: Callstack.IFrame) => {
+    if (!frame) {
+      return;
     }
+    const scopes = await this.getScopes(frame);
+    const variables = await this.getVariables(scopes);
+    const variableScopes = this.convertScope(scopes, variables);
+    this._model.variablesModel.scopes = variableScopes;
   };
 
   dumpCell = async (code: string) => {
@@ -476,6 +462,13 @@ export class DebugService implements IDebugger {
     this._model.variablesModel.scopes = [];
   }
 
+  private clearSignals() {
+    this._model.callstackModel.currentFrameChanged.disconnect(
+      this.onChangeFrame
+    );
+    this._model.variablesModel.variableExpanded.disconnect(this.getVariable);
+  }
+
   private currentThread(): number {
     // TODO: ask the model for the current thread ID
     return 1;
@@ -507,18 +500,10 @@ export class DebugService implements IDebugger {
   private _tmpFilePrefix: string;
   private _tmpFileSuffix: string;
 
-  // TODO: remove frames from the service
-  private frames: Frame[] = [];
-
   // TODO: move this in model
   private _stoppedThreads = new Set();
 }
 
-export type Frame = {
-  id: number;
-  scopes: Variables.IScope[];
-};
-
 namespace Private {
   export function toSourceBreakpoints(breakpoints: Breakpoints.IBreakpoint[]) {
     return breakpoints.map(breakpoint => {

src/tokens.ts

@@ -226,8 +226,9 @@ export namespace IDebugger {
         hashMethod: string;
         hashSeed: number;
         breakpoints: IDebugInfoBreakpoints[];
-        tmp_file_prefix: string;
-        tmp_file_suffix: string;
+        tmpFilePrefix: string;
+        tmpFileSuffix: string;
+        stoppedThreads: number[];
       };
     }
 

tests/src/service.spec.ts

@@ -4,6 +4,8 @@ import { ClientSession, IClientSession } from '@jupyterlab/apputils';
 
 import { createClientSession } from '@jupyterlab/testutils';
 
+import { Breakpoints } from '../../lib/breakpoints';
+
 import { Debugger } from '../../lib/debugger';
 
 import { DebugService } from '../../lib/service';
@@ -91,4 +93,72 @@ describe('DebugService', () => {
       expect(modelChangedEvents[0]).to.eq(model);
     });
   });
+
+  describe('protocol', () => {
+    const code = [
+      'i = 0',
+      'i += 1',
+      'i += 1',
+      'j = i**2',
+      'j += 1',
+      'print(i, j)'
+    ].join('\n');
+
+    let breakpoints: Breakpoints.IBreakpoint[];
+    let sourceId: string;
+
+    beforeEach(async () => {
+      service.session = session;
+      service.model = model;
+      await service.restoreState(true);
+      const breakpointLines: number[] = [3, 5];
+      sourceId = service.getCellId(code);
+      breakpoints = breakpointLines.map((l: number, index: number) => {
+        return {
+          id: index,
+          line: l,
+          active: true,
+          verified: true,
+          source: {
+            path: sourceId
+          }
+        };
+      });
+      await service.updateBreakpoints(code, breakpoints);
+    });
+
+    describe('#updateBreakpoints', () => {
+      it('should update the breakpoints', () => {
+        const bpList = model.breakpointsModel.getBreakpoints(sourceId);
+        expect(bpList).to.deep.eq(breakpoints);
+      });
+    });
+
+    describe('#restoreState', () => {
+      it('should restore the breakpoints', async () => {
+        model.breakpointsModel.restoreBreakpoints(
+          new Map<string, Breakpoints.IBreakpoint[]>()
+        );
+        const bpList1 = model.breakpointsModel.getBreakpoints(sourceId);
+        expect(bpList1.length).to.equal(0);
+        await service.restoreState(true);
+        const bpList2 = model.breakpointsModel.getBreakpoints(sourceId);
+        expect(bpList2).to.deep.eq(breakpoints);
+      });
+    });
+
+    describe('#restart', () => {
+      it('should restart the debugger and send the breakpoints again', async () => {
+        await service.restart();
+        model.breakpointsModel.restoreBreakpoints(
+          new Map<string, Breakpoints.IBreakpoint[]>()
+        );
+        await service.restoreState(true);
+        const bpList = model.breakpointsModel.getBreakpoints(sourceId);
+        breakpoints[0].id = 2;
+        breakpoints[1].id = 3;
+        expect(bpList).to.deep.eq(breakpoints);
+      });
+    });
+  });
 });

tests/src/session.spec.ts

@@ -183,7 +183,7 @@ describe('protocol', () => {
       });
       expect(reply.success).to.be.true;
       const stackFrames = reply.body.stackFrames;
-      expect(stackFrames.length).to.equal(2);
+      expect(stackFrames.length).to.equal(1);
       const frame = stackFrames[0];
       // first breakpoint
       expect(frame.line).to.equal(3);
@@ -262,4 +262,28 @@ describe('protocol', () => {
       expect(j.value).to.equal('4');
     });
   });
+
+  describe('#loadedSources', () => {
+    it('should *not* retrieve the list of loaded sources', async () => {
+      // `loadedSources` is not supported at the moment "unknown command"
+      const reply = await debugSession.sendRequest('loadedSources', {});
+      expect(reply.success).to.be.false;
+    });
+  });
+
+  describe('#source', () => {
+    it('should retrieve the source of the dumped code cell', async () => {
+      const stackFramesReply = await debugSession.sendRequest('stackTrace', {
+        threadId
+      });
+      const frame = stackFramesReply.body.stackFrames[0];
+      const source = frame.source;
+      const reply = await debugSession.sendRequest('source', {
+        source: { path: source.path },
+        sourceReference: source.sourceReference
+      });
+      const sourceCode = reply.body.content;
+      expect(sourceCode).to.equal(code);
+    });
+  });
 });