Fix a bunch of debugger issues (microsoft#10572)

* Fix notebook taking ownership of hash provider * Fix last line removal and startup of interactive window during debugging * Add news entries

Author: rchiodo

news/2 Fixes/10206.md

@@ -0,0 +1 @@
+Fix interactive window debugging after running cells in a notebook.

news/2 Fixes/10395.md

@@ -0,0 +1 @@
+Fix interactive window debugging when debugging the first cell to be run.

news/2 Fixes/10396.md

@@ -0,0 +1 @@
+Fix interactive window debugging for extra lines in a function.

src/client/datascience/editor-integration/cellhashprovider.ts

@@ -136,6 +136,7 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi
  return lines;
  }
 
+ // tslint:disable-next-line: cyclomatic-complexity
  public async addCellHash(cell: ICell, expectedCount: number): Promise<void> {
  // Find the text document that matches. We need more information than
  // the add code gives us
@@ -163,14 +164,25 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi
  const endOffset = doc.offsetAt(endLine.rangeIncludingLineBreak.end);
 
  // Jupyter also removes blank lines at the end. Make sure only one
- let lastLine = stripped[stripped.length - 1];
- while (lastLine !== undefined && (lastLine.length === 0 || lastLine === '\n')) {
- stripped.splice(stripped.length - 1, 1);
- lastLine = stripped[stripped.length - 1];
+ let lastLinePos = stripped.length - 1;
+ let nextToLastLinePos = stripped.length - 2;
+ while (nextToLastLinePos > 0) {
+ const lastLine = stripped[lastLinePos];
+ const nextToLastLine = stripped[nextToLastLinePos];
+ if (
+ (lastLine.length === 0 || lastLine === '\n') &&
+ (nextToLastLine.length === 0 || nextToLastLine === '\n')
+ ) {
+ stripped.splice(lastLinePos, 1);
+ lastLinePos -= 1;
+ nextToLastLinePos -= 1;
+ } else {
+ break;
+ }
  }
  // Make sure the last line with actual content ends with a linefeed
- if (!lastLine.endsWith('\n') && lastLine.length > 0) {
- stripped[stripped.length - 1] = `${lastLine}\n`;
+ if (!stripped[lastLinePos].endsWith('\n') && stripped[lastLinePos].length > 0) {
+ stripped[lastLinePos] = `${stripped[lastLinePos]}\n`;
  }
 
  // Compute the runtime line and adjust our cell/stripped source for debugging

src/client/datascience/interactive-common/interactiveBase.ts

@@ -1159,11 +1159,11 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
  // Create a new notebook if we need to.
  if (!this._notebook) {
  // While waiting make the notebook look busy
- await this.postMessage(InteractiveWindowMessages.UpdateKernel, {
+ this.postMessage(InteractiveWindowMessages.UpdateKernel, {
  jupyterServerStatus: ServerStatus.Busy,
  localizedUri: this.getServerUri(server),
  displayName: ''
- });
+ }).ignoreErrors();
 
  this._notebook = await this.createNotebook(server);
 

src/client/datascience/interactive-window/interactiveWindow.ts

@@ -181,20 +181,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
  }
 
  public async addCode(code: string, file: string, line: number): Promise<boolean> {
- if (this.lastFile && !this.fileSystem.arePathsSame(file, this.lastFile)) {
- sendTelemetryEvent(Telemetry.NewFileForInteractiveWindow);
- }
- // Save the last file we ran with.
- this.lastFile = file;
-
- // Make sure our web panel opens.
- await this.show();
-
- // Tell the webpanel about the new directory.
- this.updateCwd(path.dirname(file));
-
- // Call the internal method.
- return this.submitCode(code, file, line);
+ return this.addOrDebugCode(code, file, line, false);
  }
 
  public exportCells() {
@@ -252,7 +239,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
 
  // Call the internal method if we were able to save
  if (saved) {
- return this.submitCode(code, file, line, undefined, undefined, true);
+ return this.addOrDebugCode(code, file, line, true);
  }
 
  return false;
@@ -365,6 +352,23 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
  return super.ensureServerAndNotebook();
  }
 
+ private async addOrDebugCode(code: string, file: string, line: number, debug: boolean): Promise<boolean> {
+ if (this.lastFile && !this.fileSystem.arePathsSame(file, this.lastFile)) {
+ sendTelemetryEvent(Telemetry.NewFileForInteractiveWindow);
+ }
+ // Save the last file we ran with.
+ this.lastFile = file;
+
+ // Make sure our web panel opens.
+ await this.show();
+
+ // Tell the webpanel about the new directory.
+ this.updateCwd(path.dirname(file));
+
+ // Call the internal method.
+ return this.submitCode(code, file, line, undefined, undefined, debug);
+ }
+
  @captureTelemetry(Telemetry.ExportNotebook, undefined, false)
  // tslint:disable-next-line: no-any no-empty
  private async export(cells: ICell[]) {

src/client/datascience/jupyter/jupyterNotebook.ts

@@ -154,7 +154,6 @@ export class JupyterNotebookBase implements INotebook {
  private _identity: Uri;
  private _disposed: boolean = false;
  private _workingDirectory: string | undefined;
- private _loggers: INotebookExecutionLogger[] = [];
  private onStatusChangedEvent: EventEmitter<ServerStatus> | undefined;
  public get onKernelChanged(): Event<IJupyterKernelSpec | LiveKernelModel> {
  return this.kernelChanged.event;
@@ -170,7 +169,7 @@ export class JupyterNotebookBase implements INotebook {
  private disposableRegistry: IDisposableRegistry,
  private owner: INotebookServer,
  private launchInfo: INotebookServerLaunchInfo,
- loggers: INotebookExecutionLogger[],
+ private loggers: INotebookExecutionLogger[],
  resource: Resource,
  identity: Uri,
  private getDisposedError: () => Error,
@@ -188,7 +187,6 @@ export class JupyterNotebookBase implements INotebook {
  this.sessionStatusChanged = this.session.onSessionStatusChanged(statusChangeHandler);
  this._identity = identity;
  this._resource = resource;
- this._loggers = [...loggers];
  // Save our interpreter and don't change it. Later on when kernel changes
  // are possible, recompute it.
  }
@@ -617,7 +615,7 @@ export class JupyterNotebookBase implements INotebook {
  }
 
  public getLoggers(): INotebookExecutionLogger[] {
- return this._loggers;
+ return this.loggers;
  }
 
  private async initializeMatplotlib(cancelToken?: CancellationToken): Promise<void> {
@@ -1051,11 +1049,11 @@ export class JupyterNotebookBase implements INotebook {
  }
 
  private async logPreCode(cell: ICell, silent: boolean): Promise<void> {
- await Promise.all(this._loggers.map(l => l.preExecute(cell, silent)));
+ await Promise.all(this.loggers.map(l => l.preExecute(cell, silent)));
  }
 
  private async logPostCode(cell: ICell, silent: boolean): Promise<void> {
- await Promise.all(this._loggers.map(l => l.postExecute(cloneDeep(cell), silent)));
+ await Promise.all(this.loggers.map(l => l.postExecute(cloneDeep(cell), silent)));
  }
 
  private addToCellData = (

src/client/datascience/jupyter/jupyterServer.ts

@@ -18,13 +18,13 @@ import {
 import { createDeferred, Deferred } from '../../common/utils/async';
 import * as localize from '../../common/utils/localize';
 import { noop } from '../../common/utils/misc';
+import { IServiceContainer } from '../../ioc/types';
 import {
  IConnection,
  IJupyterSession,
  IJupyterSessionManager,
  IJupyterSessionManagerFactory,
  INotebook,
- INotebookExecutionLogger,
  INotebookServer,
  INotebookServerLaunchInfo
 } from '../types';
@@ -48,7 +48,7 @@ export class JupyterServerBase implements INotebookServer {
  private disposableRegistry: IDisposableRegistry,
  protected readonly configService: IConfigurationService,
  private sessionManagerFactory: IJupyterSessionManagerFactory,
- private loggers: INotebookExecutionLogger[],
+ private serviceContainer: IServiceContainer,
  private jupyterOutputChannel: IOutputChannel
  ) {
  this.asyncRegistry.push(this);
@@ -115,7 +115,7 @@ export class JupyterServerBase implements INotebookServer {
  savedSession,
  this.disposableRegistry,
  this.configService,
- this.loggers,
+ this.serviceContainer,
  notebookMetadata,
  cancelToken
  ).then(r => {
@@ -223,7 +223,7 @@ export class JupyterServerBase implements INotebookServer {
  _savedSession: IJupyterSession | undefined,
  _disposableRegistry: IDisposableRegistry,
  _configService: IConfigurationService,
- _loggers: INotebookExecutionLogger[],
+ _serviceContainer: IServiceContainer,
  _notebookMetadata?: nbformat.INotebookMetadata,
  _cancelToken?: CancellationToken
  ): Promise<INotebook> {

src/client/datascience/jupyter/jupyterServerWrapper.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 'use strict';
 import { nbformat } from '@jupyterlab/coreutils';
-import { inject, injectable, multiInject, named, optional } from 'inversify';
+import { inject, injectable, named } from 'inversify';
 import * as uuid from 'uuid/v4';
 import { Uri } from 'vscode';
 import { CancellationToken } from 'vscode-jsonrpc';
@@ -18,13 +18,13 @@ import {
  Resource
 } from '../../common/types';
 import { IInterpreterService } from '../../interpreter/contracts';
+import { IServiceContainer } from '../../ioc/types';
 import { JUPYTER_OUTPUT_CHANNEL } from '../constants';
 import {
  IConnection,
  IDataScience,
  IJupyterSessionManagerFactory,
  INotebook,
- INotebookExecutionLogger,
  INotebookServer,
  INotebookServerLaunchInfo
 } from '../types';
@@ -46,7 +46,7 @@ type JupyterServerClassType = {
  configService: IConfigurationService,
  sessionManager: IJupyterSessionManagerFactory,
  workspaceService: IWorkspaceService,
- loggers: INotebookExecutionLogger[],
+ serviceContainer: IServiceContainer,
  appShell: IApplicationShell,
  fs: IFileSystem,
  kernelSelector: KernelSelector,
@@ -73,12 +73,12 @@ export class JupyterServerWrapper implements INotebookServer, ILiveShareHasRole
  @inject(IConfigurationService) configService: IConfigurationService,
  @inject(IJupyterSessionManagerFactory) sessionManager: IJupyterSessionManagerFactory,
  @inject(IWorkspaceService) workspaceService: IWorkspaceService,
- @multiInject(INotebookExecutionLogger) @optional() loggers: INotebookExecutionLogger[] | undefined,
  @inject(IApplicationShell) appShell: IApplicationShell,
  @inject(IFileSystem) fs: IFileSystem,
  @inject(IInterpreterService) interpreterService: IInterpreterService,
  @inject(KernelSelector) kernelSelector: KernelSelector,
- @inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) jupyterOutput: IOutputChannel
+ @inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) jupyterOutput: IOutputChannel,
+ @inject(IServiceContainer) serviceContainer: IServiceContainer
  ) {
  // The server factory will create the appropriate HostJupyterServer or GuestJupyterServer based on
  // the liveshare state.
@@ -93,7 +93,7 @@ export class JupyterServerWrapper implements INotebookServer, ILiveShareHasRole
  configService,
  sessionManager,
  workspaceService,
- loggers ? loggers : [],
+ serviceContainer,
  appShell,
  fs,
  kernelSelector,

src/client/datascience/jupyter/liveshare/guestJupyterServer.ts

@@ -9,13 +9,13 @@ import { ILiveShareApi, IWorkspaceService } from '../../../common/application/ty
 import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types';
 import { createDeferred, Deferred } from '../../../common/utils/async';
 import * as localize from '../../../common/utils/localize';
+import { IServiceContainer } from '../../../ioc/types';
 import { LiveShare, LiveShareCommands } from '../../constants';
 import {
  IConnection,
  IDataScience,
  IJupyterSessionManagerFactory,
  INotebook,
- INotebookExecutionLogger,
  INotebookServer,
  INotebookServerLaunchInfo
 } from '../../types';
@@ -39,7 +39,7 @@ export class GuestJupyterServer
  private configService: IConfigurationService,
  _sessionManager: IJupyterSessionManagerFactory,
  _workspaceService: IWorkspaceService,
- _loggers: INotebookExecutionLogger[]
+ _serviceContainer: IServiceContainer
  ) {
  super(liveShare);
  }