Merge pull request #190 from KsavinN/debug

Resolve-switching between notebooks removes the breakpoints from the UI

src/debugger.ts

@@ -128,6 +128,22 @@ export namespace Debugger {
     readonly variablesModel: Variables.Model;
     readonly connector: IDataConnector<ReadonlyJSONValue> | null;
 
+    dispose(): void {
+      this._isDisposed = true;
+      this._disposed.emit();
+    }
+
+    /**
+     * A signal emitted when the debugger widget is disposed.
+     */
+    get disposed(): ISignal<this, void> {
+      return this._disposed;
+    }
+
+    get isDisposed(): boolean {
+      return this._isDisposed;
+    }
+
     get mode(): IDebugger.Mode {
       return this._mode;
     }
@@ -144,10 +160,6 @@ export namespace Debugger {
       return this._modeChanged;
     }
 
-    get isDisposed(): boolean {
-      return this._isDisposed;
-    }
-
     get codeValue() {
       return this._codeValue;
     }
@@ -156,10 +168,6 @@ export namespace Debugger {
       this._codeValue = observableString;
     }
 
-    dispose(): void {
-      this._isDisposed = true;
-    }
-
     private async _populate(): Promise<void> {
       const { connector } = this;
 
@@ -172,6 +180,7 @@ export namespace Debugger {
     private _isDisposed = false;
     private _mode: IDebugger.Mode;
     private _modeChanged = new Signal<this, IDebugger.Mode>(this);
+    private _disposed = new Signal<this, void>(this);
   }
 
   export namespace Sidebar {

src/handlers/cell.ts

@@ -29,11 +29,12 @@ const CELL_CHANGED_TIMEOUT = 1000;
 
 export class CellManager implements IDisposable {
   constructor(options: CellManager.IOptions) {
+    // TODO: should we use the client name or a debug session id?
+    this._id = options.debuggerService.session.client.name;
     this._debuggerService = options.debuggerService;
     this.onModelChanged();
     this._debuggerService.modelChanged.connect(() => this.onModelChanged());
     this.activeCell = options.activeCell;
-    this.onActiveCellChanged();
   }
 
   isDisposed: boolean;
@@ -47,7 +48,7 @@ export class CellManager implements IDisposable {
 
     this._debuggerModel.callstackModel.currentFrameChanged.connect(
       (_, frame) => {
-        CellManager.cleanupHighlight(this.activeCell);
+        CellManager.clearHighlight(this.activeCell);
         if (!frame) {
           return;
         }
@@ -72,7 +73,7 @@ export class CellManager implements IDisposable {
       this.addBreakpointsToEditor(this.activeCell);
     });
 
-    if (this.activeCell) {
+    if (this.activeCell && !this.activeCell.isDisposed) {
       this._debuggerModel.codeValue = this.activeCell.model.value;
     }
   }
@@ -81,15 +82,14 @@ export class CellManager implements IDisposable {
     if (this.isDisposed) {
       return;
     }
-    if (this.previousCell) {
-      this.removeListener(this.previousCell);
-    }
     if (this._cellMonitor) {
       this._cellMonitor.dispose();
     }
-    this.removeListener(this.activeCell);
-    CellManager.cleanupHighlight(this.activeCell);
+    this.removeGutterClick(this.activeCell);
+    CellManager.clearHighlight(this.activeCell);
+    CellManager.clearGutter(this.activeCell);
     Signal.clearData(this);
+    this.isDisposed = true;
   }
 
   set previousCell(cell: CodeCell) {
@@ -112,15 +112,6 @@ export class CellManager implements IDisposable {
     return this._activeCell;
   }
 
-  protected clearGutter(cell: CodeCell) {
-    const editor = cell.editor as CodeMirrorEditor;
-    editor.doc.eachLine(line => {
-      if ((line as ILineInfo).gutterMarkers) {
-        editor.editor.setGutterMarker(line, 'breakpoints', null);
-      }
-    });
-  }
-
   onActiveCellChanged() {
     if (
       this.activeCell &&
@@ -133,7 +124,7 @@ export class CellManager implements IDisposable {
         if (this._cellMonitor) {
           this._cellMonitor.dispose();
         }
-        this.removeListener(this.previousCell);
+        this.removeGutterClick(this.previousCell);
       }
 
       this._cellMonitor = new ActivityMonitor({
@@ -145,8 +136,11 @@ export class CellManager implements IDisposable {
         this.sendEditorBreakpoints();
       }, this);
 
+      requestAnimationFrame(() => {
+        this.setEditor(this.activeCell);
+      });
+
       this.previousCell = this.activeCell;
-      this.setEditor(this.activeCell);
     }
   }
 
@@ -187,7 +181,7 @@ export class CellManager implements IDisposable {
     editor.editor.on('gutterClick', this.onGutterClick);
   }
 
-  protected removeListener(cell: CodeCell) {
+  protected removeGutterClick(cell: CodeCell) {
     if (cell.isDisposed) {
       return;
     }
@@ -201,10 +195,14 @@ export class CellManager implements IDisposable {
 
   protected onGutterClick = (editor: Editor, lineNumber: number) => {
     const info = editor.lineInfo(lineNumber);
-    if (!info) {
+
+    if (!info || this._id !== this._debuggerService.session.client.name) {
       return;
     }
 
+    editor.focus();
+    CellManager.clearGutter(this.activeCell);
+
     const isRemoveGutter = !!info.gutterMarkers;
     let breakpoints: Breakpoints.IBreakpoint[] = this.getBreakpoints(
       this._activeCell
@@ -228,16 +226,22 @@ export class CellManager implements IDisposable {
   };
 
   private addBreakpointsToEditor(cell: CodeCell) {
-    this.clearGutter(cell);
     const editor = cell.editor as CodeMirrorEditor;
     const breakpoints = this.getBreakpoints(cell);
-    breakpoints.forEach(breakpoint => {
-      editor.editor.setGutterMarker(
-        breakpoint.line - 1,
-        'breakpoints',
-        Private.createMarkerNode()
-      );
-    });
+    if (
+      breakpoints.length === 0 &&
+      this._id === this._debuggerService.session.client.name
+    ) {
+      CellManager.clearGutter(cell);
+    } else {
+      breakpoints.forEach(breakpoint => {
+        editor.editor.setGutterMarker(
+          breakpoint.line - 1,
+          'breakpoints',
+          Private.createMarkerNode()
+        );
+      });
+    }
   }
 
   private getBreakpointsFromEditor(cell: CodeCell): ILineInfo[] {
@@ -263,6 +267,7 @@ export class CellManager implements IDisposable {
   private breakpointsModel: Breakpoints.Model;
   private _activeCell: CodeCell;
   private _debuggerService: IDebugger;
+  private _id: string;
   private _cellMonitor: ActivityMonitor<
     IObservableString,
     IObservableString.IChangedArgs
@@ -285,23 +290,40 @@ export namespace CellManager {
    */
   export function showCurrentLine(cell: Cell, frame: Callstack.IFrame) {
     const editor = cell.editor as CodeMirrorEditor;
-    cleanupHighlight(cell);
+    clearHighlight(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) {
+  export function clearHighlight(cell: Cell) {
+    if (!cell || cell.isDisposed || !cell.inputArea) {
       return;
     }
     const editor = cell.editor as CodeMirrorEditor;
     editor.doc.eachLine(line => {
       editor.editor.removeLineClass(line, 'wrap', LINE_HIGHLIGHT_CLASS);
     });
   }
+
+  /**
+   * Remove line numbers and all gutters from cell.
+   * @param cell The cell to cleanup.
+   */
+
+  export function clearGutter(cell: Cell) {
+    if (!cell || !cell.inputArea) {
+      return;
+    }
+    const editor = cell.editor as CodeMirrorEditor;
+    editor.doc.eachLine(line => {
+      if ((line as ILineInfo).gutterMarkers) {
+        editor.editor.setGutterMarker(line, 'breakpoints', null);
+      }
+    });
+  }
 }
 
 export interface ILineInfo {

src/handlers/console.ts

@@ -56,14 +56,6 @@ export class ConsoleHandler implements IDisposable {
     if (this.cellManager) {
       this.cellManager.previousCell = this.cellManager.activeCell;
       this.cellManager.activeCell = update;
-    } else if (!this.cellManager) {
-      this.cellManager = new CellManager({
-        activeCell: update,
-        breakpointsModel: this.breakpoints,
-        debuggerModel: this.debuggerModel,
-        debuggerService: this.debuggerService,
-        type: 'console'
-      });
     }
   }
 }

src/handlers/notebook.ts

@@ -1,7 +1,11 @@
 // Copyright (c) Jupyter Development Team.
 // Distributed under the terms of the Modified BSD License.
 
-import { INotebookTracker, NotebookTracker } from '@jupyterlab/notebook';
+import {
+  INotebookTracker,
+  NotebookPanel,
+  NotebookTracker
+} from '@jupyterlab/notebook';
 
 import { CodeCell } from '@jupyterlab/cells';
 
@@ -24,6 +28,8 @@ export class NotebookHandler implements IDisposable {
     this.debuggerModel = options.debuggerService.model;
     this.debuggerService = options.debuggerService;
     this.notebookTracker = options.tracker;
+    this.notebookPanel = this.notebookTracker.currentWidget;
+
     this.id = options.id;
     this.breakpoints = this.debuggerModel.breakpointsModel;
 
@@ -53,21 +59,27 @@ export class NotebookHandler implements IDisposable {
       return;
     }
     this.isDisposed = true;
+    this.cleanAllCells();
     this.cellManager.dispose();
     Signal.clearData(this);
   }
 
+  protected cleanAllCells() {
+    const cells = this.notebookPanel.content.widgets;
+    cells.forEach(cell => {
+      CellManager.clearHighlight(cell);
+      CellManager.clearGutter(cell);
+    });
+  }
+
   protected onActiveCellChanged(
     notebookTracker: NotebookTracker,
     codeCell: CodeCell
   ) {
     if (notebookTracker.currentWidget.id !== this.id) {
       return;
     }
-    // TODO: do we need this requestAnimationFrame?
-    requestAnimationFrame(() => {
-      this.cellManager.activeCell = codeCell;
-    });
+    this.cellManager.activeCell = codeCell;
   }
 
   private onCurrentFrameChanged(
@@ -80,7 +92,7 @@ export class NotebookHandler implements IDisposable {
     }
 
     const cells = notebook.content.widgets;
-    cells.forEach(cell => CellManager.cleanupHighlight(cell));
+    cells.forEach(cell => CellManager.clearHighlight(cell));
 
     if (!frame) {
       return;
@@ -103,6 +115,7 @@ export class NotebookHandler implements IDisposable {
   private debuggerService: IDebugger;
   private breakpoints: Breakpoints.Model;
   private cellManager: CellManager;
+  private notebookPanel: NotebookPanel;
   private id: string;
 }