Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functional tests for native interactive window #6786

Merged
merged 22 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -370,4 +370,4 @@
}
}
]
}
}
6 changes: 3 additions & 3 deletions src/client/datascience/editor-integration/cellhashprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { IConfigurationService } from '../../common/types';
import { getCellResource } from '../cellFactory';
import { CellMatcher } from '../cellMatcher';
import { Identifiers } from '../constants';
import { getInteractiveCellMetadata } from '../interactive-window/nativeInteractiveWindow';
import { getInteractiveCellMetadata } from '../interactive-window/interactiveWindow';
import { ICellHash, ICellHashListener, ICellHashProvider, IFileHashes } from '../types';

// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires
Expand Down Expand Up @@ -134,9 +134,9 @@ export class CellHashProvider implements ICellHashProvider {
// Find the text document that matches. We need more information than
// the add code gives us
const { line: cellLine, file } = cell.metadata.interactive;
const id = getInteractiveCellMetadata(cell).id;
const id = getInteractiveCellMetadata(cell)?.id;
const doc = this.documentManager.textDocuments.find((d) => this.fs.areLocalPathsSame(d.fileName, file));
if (doc) {
if (doc && id) {
// Compute the code that will really be sent to jupyter
const { stripped, trueStartLine } = this.extractStrippedLines(cell);

Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/editor-integration/codeLensFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { IConfigurationService, IDisposableRegistry, Resource } from '../../comm
import * as localize from '../../common/utils/localize';
import { generateCellRangesFromDocument } from '../cellFactory';
import { CodeLensCommands, Commands } from '../constants';
import { getInteractiveCellMetadata } from '../interactive-window/nativeInteractiveWindow';
import { getInteractiveCellMetadata } from '../interactive-window/interactiveWindow';
import { IKernelProvider } from '../jupyter/kernels/types';
import { InteractiveWindowView } from '../notebook/constants';
import { ICellHashProvider, ICellRange, ICodeLensFactory, IFileHashes } from '../types';
Expand Down Expand Up @@ -184,7 +184,7 @@ export class CodeLensFactory implements ICodeLensFactory {
};
this.notebookData.set(e.cell.notebook.uri.toString(), data);
}
if (data) {
if (data !== undefined && metadata !== undefined) {
data.cellExecutionCounts.set(metadata.id, e.cell.executionSummary.executionOrder);
data.documentExecutionCounts.set(
metadata.interactive.file.toLowerCase(),
Expand Down
7 changes: 5 additions & 2 deletions src/client/datascience/editor-integration/hoverProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { sleep } from '../../common/utils/async';
import { StopWatch } from '../../common/utils/stopWatch';
import { sendTelemetryEvent } from '../../telemetry';
import { Identifiers, Telemetry } from '../constants';
import { getInteractiveCellMetadata } from '../interactive-window/nativeInteractiveWindow';
import { getInteractiveCellMetadata } from '../interactive-window/interactiveWindow';
import { IKernelProvider } from '../jupyter/kernels/types';
import { InteractiveWindowView } from '../notebook/constants';
import {
Expand Down Expand Up @@ -57,7 +57,10 @@ export class HoverProvider implements INotebookExecutionLogger, IHoverProvider {
return;
}
const size = this.runFiles.size;
this.runFiles.add(getInteractiveCellMetadata(e.cell).interactive.file.toLocaleLowerCase());
const metadata = getInteractiveCellMetadata(e.cell);
if (metadata !== undefined) {
this.runFiles.add(metadata.interactive.file.toLocaleLowerCase());
}
if (size !== this.runFiles.size) {
await this.initializeHoverProvider();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,12 @@ type InteractiveCellMetadata = {
};
id: string;
};
export function getInteractiveCellMetadata(cell: NotebookCell): InteractiveCellMetadata {
return cell.metadata as InteractiveCellMetadata;
export function getInteractiveCellMetadata(cell: NotebookCell): InteractiveCellMetadata | undefined {
if (cell.metadata.interactive !== undefined) {
return cell.metadata as InteractiveCellMetadata;
}
}
export class NativeInteractiveWindow implements IInteractiveWindowLoadable {
export class InteractiveWindow implements IInteractiveWindowLoadable {
public get onDidChangeViewState(): Event<void> {
return this._onDidChangeViewState.event;
}
Expand Down Expand Up @@ -500,7 +502,7 @@ export class NativeInteractiveWindow implements IInteractiveWindowLoadable {
const notebookEditor = await this._editorReadyPromise;
const matchingCell = notebookEditor.document
.getCells()
.find((cell) => getInteractiveCellMetadata(cell).id === id);
.find((cell) => getInteractiveCellMetadata(cell)?.id === id);
if (matchingCell) {
this.revealCell(matchingCell, notebookEditor);
}
Expand All @@ -514,13 +516,12 @@ export class NativeInteractiveWindow implements IInteractiveWindowLoadable {
}, 200); // Rendering output is async so the output is not guaranteed to immediately exist
}

// TODO this does not need to be async since we no longer need to roundtrip to the UI
public async hasCell(id: string): Promise<boolean> {
const notebookEditor = await this._editorReadyPromise;
if (!notebookEditor) {
return false;
}
return notebookEditor.document.getCells().some((cell) => getInteractiveCellMetadata(cell).id === id);
return notebookEditor.document.getCells().some((cell) => getInteractiveCellMetadata(cell)?.id === id);
}

public get owningResource(): Resource {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
TextEditor,
Uri,
ViewColumn,
window,
workspace,
WorkspaceEdit
} from 'vscode';
Expand Down Expand Up @@ -134,14 +135,14 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
this.disposableRegistry.push(
commandManager.registerCommand(
Commands.ExpandAllCells,
(context?: { notebookEditor: { notebookUri: Uri } }) =>
async (context?: { notebookEditor: { notebookUri: Uri } }) =>
this.expandAllCells(context?.notebookEditor.notebookUri)
)
);
this.disposableRegistry.push(
commandManager.registerCommand(
Commands.CollapseAllCells,
(context?: { notebookEditor: { notebookUri: Uri } }) =>
async (context?: { notebookEditor: { notebookUri: Uri } }) =>
this.collapseAllCells(context?.notebookEditor.notebookUri)
)
);
Expand Down Expand Up @@ -372,21 +373,19 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
return this.exportDialog.showDialog(ExportFormat.ipynb, file);
}

private expandAllCells(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
private async expandAllCells(uri?: Uri) {
const interactiveWindow = this.getTargetInteractiveWindow(uri);
traceInfo(`Expanding all cells in interactive window with uri ${interactiveWindow?.notebookUri}`);
if (interactiveWindow) {
interactiveWindow.expandAllCells();
await interactiveWindow.expandAllCells();
}
}

private collapseAllCells(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
private async collapseAllCells(uri?: Uri) {
const interactiveWindow = this.getTargetInteractiveWindow(uri);
traceInfo(`Collapsing all cells in interactive window with uri ${interactiveWindow?.notebookUri}`);
if (interactiveWindow) {
interactiveWindow.collapseAllCells();
await interactiveWindow.collapseAllCells();
}
}

Expand All @@ -398,18 +397,14 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
}

private exportAs(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
const interactiveWindow = this.getTargetInteractiveWindow(uri);
if (interactiveWindow) {
interactiveWindow.exportAs();
}
}

private export(uri?: Uri) {
const interactiveWindow = uri
? this.interactiveWindowProvider.windows.find((window) => window.notebookUri?.toString() === uri.toString())
: this.interactiveWindowProvider.activeWindow;
const interactiveWindow = this.getTargetInteractiveWindow(uri);
if (interactiveWindow) {
interactiveWindow.export();
}
Expand Down Expand Up @@ -488,9 +483,7 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
}

private async clearAllCellsInInteractiveWindow(context?: { notebookEditor: { notebookUri: Uri } }): Promise<void> {
// Use the context if invoked from interactive/toolbar
// Then fallback to the active interactive window
const uri = context?.notebookEditor.notebookUri ?? this.interactiveWindowProvider.activeWindow?.notebookUri;
const uri = this.getTargetInteractiveWindow(context?.notebookEditor.notebookUri)?.notebookUri;
if (!uri) {
return;
}
Expand Down Expand Up @@ -551,4 +544,22 @@ export class NativeInteractiveWindowCommandListener implements IDataScienceComma
await this.clipboard.writeText(source);
}
}

private getTargetInteractiveWindow(notebookUri: Uri | undefined) {
let targetInteractiveWindow;
if (notebookUri !== undefined) {
targetInteractiveWindow = this.interactiveWindowProvider.windows.find(
(w) => w.notebookUri?.toString() === notebookUri.toString()
);
} else if (notebookUri === undefined && this.interactiveWindowProvider.activeWindow !== undefined) {
targetInteractiveWindow = this.interactiveWindowProvider.activeWindow;
} else if (
notebookUri === undefined &&
this.interactiveWindowProvider.activeWindow === undefined &&
window.activeTextEditor?.document.uri !== undefined
) {
targetInteractiveWindow = this.interactiveWindowProvider.get(window.activeTextEditor.document.uri);
}
return targetInteractiveWindow;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ import { IExportDialog } from '../export/types';
import { IKernelProvider } from '../jupyter/kernels/types';
import { INotebookControllerManager } from '../notebook/types';
import { IInteractiveWindow, IInteractiveWindowProvider, IJupyterDebugger, INotebookExporter } from '../types';
import { NativeInteractiveWindow } from './nativeInteractiveWindow';
import { InteractiveWindow } from './interactiveWindow';

// Export for testing
export const AskedForPerFileSettingKey = 'ds_asked_per_file_interactive';

@injectable()
export class NativeInteractiveWindowProvider implements IInteractiveWindowProvider, IAsyncDisposable {
export class InteractiveWindowProvider implements IInteractiveWindowProvider, IAsyncDisposable {
public get onDidChangeActiveInteractiveWindow(): Event<IInteractiveWindow | undefined> {
return this._onDidChangeActiveInteractiveWindow.event;
}
Expand All @@ -57,7 +57,7 @@ export class NativeInteractiveWindowProvider implements IInteractiveWindowProvid
private readonly _onDidChangeActiveInteractiveWindow = new EventEmitter<IInteractiveWindow | undefined>();
private readonly _onDidCreateInteractiveWindow = new EventEmitter<IInteractiveWindow>();
private lastActiveInteractiveWindow: IInteractiveWindow | undefined;
private _windows: NativeInteractiveWindow[] = [];
private _windows: InteractiveWindow[] = [];

constructor(
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
Expand Down Expand Up @@ -114,7 +114,7 @@ export class NativeInteractiveWindowProvider implements IInteractiveWindowProvid
private create(resource: Resource, mode: InteractiveWindowMode) {
// Set it as soon as we create it. The .ctor for the interactive window
// may cause a subclass to talk to the IInteractiveWindowProvider to get the active interactive window.
const result = new NativeInteractiveWindow(
const result = new InteractiveWindow(
this.serviceContainer.get<IApplicationShell>(IApplicationShell),
this.serviceContainer.get<IDocumentManager>(IDocumentManager),
this.serviceContainer.get<IFileSystem>(IFileSystem),
Expand Down
6 changes: 3 additions & 3 deletions src/client/datascience/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ import { IApplicationEnvironment } from '../common/application/types';
import { NotebookIPyWidgetCoordinator } from './ipywidgets/notebookIPyWidgetCoordinator';
import { ExtensionRecommendationService } from './extensionRecommendation';
import { PythonVariablesRequester } from './jupyter/pythonVariableRequester';
import { NativeInteractiveWindowCommandListener } from './interactive-window/nativeInteractiveWindowCommandListener';
import { NativeInteractiveWindowProvider } from './interactive-window/nativeInteractiveWindowProvider';
import { NativeInteractiveWindowCommandListener } from './interactive-window/interactiveWindowCommandListener';
import { InteractiveWindowProvider } from './interactive-window/interactiveWindowProvider';
import { JupyterPaths } from './kernel-launcher/jupyterPaths';
import { LocalKnownPathKernelSpecFinder } from './kernel-launcher/localKnownPathKernelSpecFinder';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './kernel-launcher/localPythonAndRelatedNonPythonKernelSpecFinder';
Expand Down Expand Up @@ -238,7 +238,7 @@ export function registerTypes(serviceManager: IServiceManager, inNotebookApiExpe
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, NotebookUsageTracker);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, MigrateJupyterInterpreterStateService);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, VariableViewActivationService);
serviceManager.addSingleton<IInteractiveWindowProvider>(IInteractiveWindowProvider, NativeInteractiveWindowProvider);
serviceManager.addSingleton<IInteractiveWindowProvider>(IInteractiveWindowProvider, InteractiveWindowProvider);
serviceManager.addSingleton<IDataScienceCommandListener>(IDataScienceCommandListener, NativeInteractiveWindowCommandListener);
serviceManager.addSingleton<IDataScienceCommandListener>(IDataScienceCommandListener, KernelCommandListener);
serviceManager.addSingleton<IJupyterDebugger>(IJupyterDebugger, JupyterDebugger, undefined, [ICellHashListener]);
Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,8 @@ export interface IInteractiveWindow extends IInteractiveBase {
editor?: TextEditor,
runningStopWatch?: StopWatch
): Promise<boolean>;
expandAllCells(): void;
collapseAllCells(): void;
expandAllCells(): Promise<void>;
collapseAllCells(): Promise<void>;
exportCells(): void;
scrollToCell(id: string): void;
exportAs(cells?: ICell[]): void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ suite('CellHashProvider Unit Tests', () => {
line,
originalSource: code
},
executionId: 1
id: 1
},
outputs: [],
mime: undefined
Expand Down
Loading