From 2c05a8bc334cc96242f7ed80506504c7437205fc Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 16:39:56 -0700 Subject: [PATCH 01/23] Initial commit --- .../common/application/applicationShell.ts | 6 +- src/client/common/application/types.ts | 7 ++ .../interactiveWindowTypes.ts | 2 + .../interactive-ipynb/autoSaveService.ts | 53 +++++++++++++++ src/client/datascience/types.ts | 4 ++ src/client/datascience/webViewHost.ts | 7 ++ .../interactive-common/mainStateController.ts | 2 +- .../native-editor/autoSaveService.ts | 68 +++++++++++++++++++ .../nativeEditorStateController.ts | 15 +++- src/datascience-ui/react-common/postOffice.ts | 9 +-- .../react-common/settingsReactSide.ts | 4 ++ 11 files changed, 169 insertions(+), 8 deletions(-) create mode 100644 src/client/datascience/interactive-ipynb/autoSaveService.ts create mode 100644 src/datascience-ui/native-editor/autoSaveService.ts diff --git a/src/client/common/application/applicationShell.ts b/src/client/common/application/applicationShell.ts index 2ab4066f67b4..ea902b332fda 100644 --- a/src/client/common/application/applicationShell.ts +++ b/src/client/common/application/applicationShell.ts @@ -5,11 +5,14 @@ // tslint:disable:no-var-requires no-any unified-signatures import { injectable } from 'inversify'; -import { CancellationToken, Disposable, env, InputBox, InputBoxOptions, MessageItem, MessageOptions, OpenDialogOptions, OutputChannel, Progress, ProgressOptions, QuickPick, QuickPickItem, QuickPickOptions, SaveDialogOptions, StatusBarAlignment, StatusBarItem, TreeView, TreeViewOptions, Uri, window, WorkspaceFolder, WorkspaceFolderPickOptions } from 'vscode'; +import { CancellationToken, Disposable, env, Event, InputBox, InputBoxOptions, MessageItem, MessageOptions, OpenDialogOptions, OutputChannel, Progress, ProgressOptions, QuickPick, QuickPickItem, QuickPickOptions, SaveDialogOptions, StatusBarAlignment, StatusBarItem, TreeView, TreeViewOptions, Uri, window, WindowState, WorkspaceFolder, WorkspaceFolderPickOptions } from 'vscode'; import { IApplicationShell } from './types'; @injectable() export class ApplicationShell implements IApplicationShell { + public get onDidChangeWindowState(): Event { + return window.onDidChangeWindowState; + } public showInformationMessage(message: string, ...items: string[]): Thenable; public showInformationMessage(message: string, options: MessageOptions, ...items: string[]): Thenable; public showInformationMessage(message: string, ...items: T[]): Thenable; @@ -81,5 +84,4 @@ export class ApplicationShell implements IApplicationShell { public createOutputChannel(name: string): OutputChannel { return window.createOutputChannel(name); } - } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index e3681214ed9a..2c0f6a56be71 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -49,6 +49,7 @@ import { TreeViewOptions, Uri, ViewColumn, + WindowState, WorkspaceConfiguration, WorkspaceEdit, WorkspaceFolder, @@ -64,6 +65,12 @@ import { ICommandNameArgumentTypeMapping } from './commands'; export const IApplicationShell = Symbol('IApplicationShell'); export interface IApplicationShell { + /** + * An [event](#Event) which fires when the focus s tate of the current window + * changes. The value of the event represents whether the window is focused. + */ + readonly onDidChangeWindowState: Event; + showInformationMessage(message: string, ...items: string[]): Thenable; /** diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts index 203dbdd470bc..2e03977bc7a1 100644 --- a/src/client/datascience/interactive-common/interactiveWindowTypes.ts +++ b/src/client/datascience/interactive-common/interactiveWindowTypes.ts @@ -27,6 +27,8 @@ export namespace InteractiveWindowMessages { export const Interrupt = 'interrupt'; export const SubmitNewCell = 'submit_new_cell'; export const UpdateSettings = SharedMessages.UpdateSettings; + export const WindowStateChanged = 'WindowStateChanged'; + export const ActiveTextEditorChanged = 'ActiveTextEditorChanged'; export const SendInfo = 'send_info'; export const Started = SharedMessages.Started; export const AddedSysInfo = 'added_sys_info'; diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts new file mode 100644 index 000000000000..a17eea3cc18b --- /dev/null +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { EventEmitter, TextEditor, WindowState } from 'vscode'; +import { IApplicationShell, IDocumentManager } from '../../common/application/types'; +import '../../common/extensions'; +import { IDisposable } from '../../common/types'; +import { noop } from '../../common/utils/misc'; +import { IInteractiveWindowListener } from '../types'; +import { InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes'; + +// tslint:disable: no-any + +/** + * Sends notifications to Notebooks related to auto saving of files. + * If window state changes, then notify notebooks. + * If active editor changes, then notify notebooks. + * This information is necessary for the implementation of automatically saving notebooks. + * + * @export + * @class AutoSaveService + * @implements {IInteractiveWindowListener} + */ +@injectable() +export class AutoSaveService implements IInteractiveWindowListener { + private postEmitter: EventEmitter<{ message: string; payload: any }> = new EventEmitter<{ message: string; payload: any }>(); + private disposables: IDisposable[] = []; + constructor(@inject(IApplicationShell) appShell: IApplicationShell, @inject(IDocumentManager) documentManager: IDocumentManager) { + this.disposables.push(appShell.onDidChangeWindowState(this.onDidChangeWindowState.bind(this))); + this.disposables.push(documentManager.onDidChangeActiveTextEditor(this.onDidChangeActiveTextEditor.bind(this))); + } + + public get postMessage(): Event<{ message: string; payload: any }> { + return this.postEmitter.event; + } + + public onMessage(_message: string, _payload?: any): void { + noop(); + } + public dispose(): void | undefined { + this.disposables.forEach(item => item.dispose()); + } + + private onDidChangeWindowState(e: WindowState) { + this.postEmitter.fire({ message: InteractiveWindowMessages.WindowStateChanged, payload: e }); + } + private onDidChangeActiveTextEditor(e: TextEditor) { + this.postEmitter.fire({ message: InteractiveWindowMessages.ActiveTextEditorChanged, payload: undefined }); + } +} diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index f0edda8be83b..556cd011aca9 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -395,6 +395,10 @@ export interface IJupyterCommandFactory { // Config settings we pass to our react code export interface IDataScienceExtraSettings extends IDataScienceSettings { + files: { + autoSaveDelay: number; + autoSave: 'afterDelay' | 'off' | 'onFocusChange' | 'onWindowChange'; + }; extraSettings: { editorCursor: string; editorCursorBlink: string; diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index 114e8fece340..4315c804ecf9 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -163,10 +163,15 @@ export class WebViewHost implements IDisposable { protected generateDataScienceExtraSettings(): IDataScienceExtraSettings { const editor = this.workspaceService.getConfiguration('editor'); + const files = this.workspaceService.getConfiguration('files'); const workbench = this.workspaceService.getConfiguration('workbench'); const theme = !workbench ? DefaultTheme : workbench.get('colorTheme', DefaultTheme); return { ...this.configService.getSettings().datascience, + files: { + autoSaveDelay: this.getValue(files, 'autoSaveDelay', 1000), + autoSave: this.getValue(files, 'autoSave', 'afterDelay') + }, extraSettings: { editorCursor: this.getValue(editor, 'cursorStyle', 'line'), editorCursorBlink: this.getValue(editor, 'cursorBlinking', 'blink'), @@ -246,6 +251,8 @@ export class WebViewHost implements IDisposable { event.affectsConfiguration('editor.fontFamily') || event.affectsConfiguration('editor.cursorStyle') || event.affectsConfiguration('editor.cursorBlinking') || + event.affectsConfiguration('files.autoSave') || + event.affectsConfiguration('files.autoSaveDelay') || event.affectsConfiguration('python.dataScience.enableGather')) { // See if the theme changed const newSettings = this.generateDataScienceExtraSettings(); diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index d40a34e3eb1a..5d5049ec8a9a 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -56,10 +56,10 @@ export interface IMainStateControllerProps { // tslint:disable-next-line: max-func-body-length export class MainStateController implements IMessageHandler { + protected readonly postOffice: PostOffice = new PostOffice(); private stackLimit = 10; private pendingState: IMainState; private renderedState: IMainState; - private postOffice: PostOffice = new PostOffice(); private intellisenseProvider: IntellisenseProvider; private onigasmPromise: Deferred | undefined; private tmlangugePromise: Deferred | undefined; diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts new file mode 100644 index 000000000000..edb878d70773 --- /dev/null +++ b/src/datascience-ui/native-editor/autoSaveService.ts @@ -0,0 +1,68 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { IDisposable } from '../../client/common/types'; +import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; +import { IMessageHandler } from '../react-common/postOffice'; +import { getSettings } from '../react-common/settingsReactSide'; +import { NativeEditorStateController } from './nativeEditorStateController'; + +export class AutoSaveService implements IDisposable, IMessageHandler { + private timeout?: NodeJS.Timer | number; + constructor(private readonly controller: NativeEditorStateController) { + this.initialize(); + } + public dispose() { + this.clearTimeout(); + } + // tslint:disable-next-line: no-any + public handleMessage(type: string, _payload?: any): boolean { + switch (type) { + case InteractiveWindowMessages.UpdateSettings: { + // Settings have changed. + this.initialize(); + return true; + } + case InteractiveWindowMessages.ActiveTextEditorChanged: + case InteractiveWindowMessages.WindowStateChanged: { + this.save(); + return true; + } + default: { + return false; + } + } + } + public initialize() { + this.clearTimeout(); + const settings = getSettings().files; + if (settings.autoSave === 'afterDelay') { + // Add a timeout to save after n milli seconds. + this.timeout = setTimeout(() => this.save(), settings.autoSaveDelay); + } + } + private clearTimeout() { + if (this.timeout) { + // tslint:disable-next-line: no-any + clearTimeout(this.timeout as any); + this.timeout = undefined; + } + } + /** + * Save the notebook if it is dirty. + * If auto save is turned off or the notebook is not dirty, then this method is a noop. + * + * @private + * @returns + * @memberof AutoSaveService + */ + private save() { + const settings = getSettings().files; + if (settings.autoSave === 'off' || this.controller.getState().dirty !== true) { + return; + } + this.controller.save(); + } +} diff --git a/src/datascience-ui/native-editor/nativeEditorStateController.ts b/src/datascience-ui/native-editor/nativeEditorStateController.ts index 371939f22781..b43773cfc496 100644 --- a/src/datascience-ui/native-editor/nativeEditorStateController.ts +++ b/src/datascience-ui/native-editor/nativeEditorStateController.ts @@ -14,16 +14,29 @@ import { } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { createEmptyCell, extractInputText, ICellViewModel } from '../interactive-common/mainState'; import { IMainStateControllerProps, MainStateController } from '../interactive-common/mainStateController'; +import { IMessageHandler } from '../react-common/postOffice'; import { getSettings } from '../react-common/settingsReactSide'; +import { AutoSaveService } from './autoSaveService'; export class NativeEditorStateController extends MainStateController { private waitingForLoadRender: boolean = false; + private listeners: IMessageHandler[]; // tslint:disable-next-line:max-func-body-length constructor(props: IMainStateControllerProps) { super(props); + this.listeners = [new AutoSaveService(this)]; + this.listeners.forEach(listener => this.postOffice.addHandler(listener)); + } + public dispose(){ + this.listeners.forEach(listener => { + this.postOffice.removeHandler(listener); + if (listener.dispose){ + listener.dispose(); + } + }); + super.dispose(); } - // tslint:disable-next-line: no-any public handleMessage(msg: string, payload?: any) { // Handle message before base class so we will diff --git a/src/datascience-ui/react-common/postOffice.ts b/src/datascience-ui/react-common/postOffice.ts index 017c7def8c0b..e71953ed68e9 100644 --- a/src/datascience-ui/react-common/postOffice.ts +++ b/src/datascience-ui/react-common/postOffice.ts @@ -14,10 +14,11 @@ export interface IVsCodeApi { getState() : any; } -export interface IMessageHandler { - // tslint:disable-next-line:no-any - handleMessage(type: string, payload?: any) : boolean; -} + export interface IMessageHandler { + // tslint:disable-next-line:no-any + handleMessage(type: string, payload?: any) : boolean; + dispose?(): void; + } // This special function talks to vscode from a web panel export declare function acquireVsCodeApi(): IVsCodeApi; diff --git a/src/datascience-ui/react-common/settingsReactSide.ts b/src/datascience-ui/react-common/settingsReactSide.ts index 4bd071381afa..6fd517a7a569 100644 --- a/src/datascience-ui/react-common/settingsReactSide.ts +++ b/src/datascience-ui/react-common/settingsReactSide.ts @@ -53,6 +53,10 @@ function load() { showJupyterVariableExplorer: true, variableExplorerExclude: 'module;function;builtin_function_or_method', enablePlotViewer: true, + files : { + autoSave: 'off', + autoSaveDelay: 1000 + }, extraSettings: { editorCursor: 'line', editorCursorBlink: 'blink', From 76c7d4c99b7b491548f0e9eeb22e0fd22aa83195 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 16:48:58 -0700 Subject: [PATCH 02/23] Oops --- src/client/datascience/interactive-ipynb/autoSaveService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index a17eea3cc18b..61b60d14b5c9 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -4,13 +4,13 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { EventEmitter, TextEditor, WindowState } from 'vscode'; +import { Event, EventEmitter, TextEditor, WindowState } from 'vscode'; import { IApplicationShell, IDocumentManager } from '../../common/application/types'; import '../../common/extensions'; import { IDisposable } from '../../common/types'; import { noop } from '../../common/utils/misc'; -import { IInteractiveWindowListener } from '../types'; import { InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes'; +import { IInteractiveWindowListener } from '../types'; // tslint:disable: no-any From edef45f7659595425079ea199e89d6d412d0dd53 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 16:49:40 -0700 Subject: [PATCH 03/23] Oops again --- src/client/datascience/interactive-ipynb/autoSaveService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index 61b60d14b5c9..2cccaf6000f1 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -47,7 +47,7 @@ export class AutoSaveService implements IInteractiveWindowListener { private onDidChangeWindowState(e: WindowState) { this.postEmitter.fire({ message: InteractiveWindowMessages.WindowStateChanged, payload: e }); } - private onDidChangeActiveTextEditor(e: TextEditor) { + private onDidChangeActiveTextEditor(e?: TextEditor) { this.postEmitter.fire({ message: InteractiveWindowMessages.ActiveTextEditorChanged, payload: undefined }); } } From dda9986a47d1c8eea79dbc702f122293554c4072 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 17:12:53 -0700 Subject: [PATCH 04/23] Updates --- src/client/datascience/types.ts | 9 +++--- .../native-editor/autoSaveService.ts | 30 +++++++++++++++++-- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 556cd011aca9..30be457f786e 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -394,11 +394,12 @@ export interface IJupyterCommandFactory { } // Config settings we pass to our react code +export type FileSettings = { + autoSaveDelay: number; + autoSave: 'afterDelay' | 'off' | 'onFocusChange' | 'onWindowChange'; +}; export interface IDataScienceExtraSettings extends IDataScienceSettings { - files: { - autoSaveDelay: number; - autoSave: 'afterDelay' | 'off' | 'onFocusChange' | 'onWindowChange'; - }; + files: FileSettings; extraSettings: { editorCursor: string; editorCursorBlink: string; diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts index edb878d70773..6375f452627e 100644 --- a/src/datascience-ui/native-editor/autoSaveService.ts +++ b/src/datascience-ui/native-editor/autoSaveService.ts @@ -5,12 +5,15 @@ import { IDisposable } from '../../client/common/types'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; +import { FileSettings } from '../../client/datascience/types'; import { IMessageHandler } from '../react-common/postOffice'; import { getSettings } from '../react-common/settingsReactSide'; import { NativeEditorStateController } from './nativeEditorStateController'; export class AutoSaveService implements IDisposable, IMessageHandler { private timeout?: NodeJS.Timer | number; + private settings?: FileSettings; + // private fileSettings: typeof constructor(private readonly controller: NativeEditorStateController) { this.initialize(); } @@ -20,14 +23,35 @@ export class AutoSaveService implements IDisposable, IMessageHandler { // tslint:disable-next-line: no-any public handleMessage(type: string, _payload?: any): boolean { switch (type) { + // When clean message is sent, this means notebook was saved. + // We need to reset the timer to start again (timer starts from last save). + case InteractiveWindowMessages.NotebookClean: { + this.initialize(); + return true; + } + // When settings have been updated, its possible the timer has changed. + // We need to reset the timer to start again. case InteractiveWindowMessages.UpdateSettings: { - // Settings have changed. + const settings = getSettings().files; + if (this.settings && this.settings.autoSave === settings.autoSave && this.settings.autoSaveDelay === settings.autoSaveDelay) { + return true; + } this.initialize(); + this.settings = settings; + return true; + } + case InteractiveWindowMessages.ActiveTextEditorChanged: { + // Save if active text editor changes. + if (this.settings && this.settings.autoSave === 'onFocusChange') { + this.save(); + } return true; } - case InteractiveWindowMessages.ActiveTextEditorChanged: case InteractiveWindowMessages.WindowStateChanged: { - this.save(); + // Save if window state changes. + if (this.settings && this.settings.autoSave === 'onWindowChange') { + this.save(); + } return true; } default: { From ea443f5cb9ebd65130cdb192ac1238fc74a0e0f2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 17:14:19 -0700 Subject: [PATCH 05/23] Misc changes --- src/datascience-ui/native-editor/autoSaveService.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts index 6375f452627e..42bcb6e124c0 100644 --- a/src/datascience-ui/native-editor/autoSaveService.ts +++ b/src/datascience-ui/native-editor/autoSaveService.ts @@ -37,7 +37,6 @@ export class AutoSaveService implements IDisposable, IMessageHandler { return true; } this.initialize(); - this.settings = settings; return true; } case InteractiveWindowMessages.ActiveTextEditorChanged: { @@ -61,7 +60,7 @@ export class AutoSaveService implements IDisposable, IMessageHandler { } public initialize() { this.clearTimeout(); - const settings = getSettings().files; + const settings = this.settings = getSettings().files; if (settings.autoSave === 'afterDelay') { // Add a timeout to save after n milli seconds. this.timeout = setTimeout(() => this.save(), settings.autoSaveDelay); From c96f818db44c034e9424b146b9cc58621185f5ee Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 17:15:19 -0700 Subject: [PATCH 06/23] Oops --- src/client/datascience/interactive-ipynb/autoSaveService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index 2cccaf6000f1..10b5f39bdec6 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -47,7 +47,7 @@ export class AutoSaveService implements IInteractiveWindowListener { private onDidChangeWindowState(e: WindowState) { this.postEmitter.fire({ message: InteractiveWindowMessages.WindowStateChanged, payload: e }); } - private onDidChangeActiveTextEditor(e?: TextEditor) { + private onDidChangeActiveTextEditor(_e?: TextEditor) { this.postEmitter.fire({ message: InteractiveWindowMessages.ActiveTextEditorChanged, payload: undefined }); } } From 52be8295befcf16c5efd7c77339d03899c877279 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 17:43:01 -0700 Subject: [PATCH 07/23] Bug fixes --- src/client/datascience/serviceRegistry.ts | 2 ++ .../native-editor/autoSaveService.ts | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/client/datascience/serviceRegistry.ts b/src/client/datascience/serviceRegistry.ts index ffba92db2bc1..435b6285d99b 100644 --- a/src/client/datascience/serviceRegistry.ts +++ b/src/client/datascience/serviceRegistry.ts @@ -25,6 +25,7 @@ import { DotNetIntellisenseProvider } from './interactive-common/intellisense/do import { JediIntellisenseProvider } from './interactive-common/intellisense/jediIntellisenseProvider'; import { LinkProvider } from './interactive-common/linkProvider'; import { ShowPlotListener } from './interactive-common/showPlotListener'; +import { AutoSaveService } from './interactive-ipynb/autoSaveService'; import { NativeEditor } from './interactive-ipynb/nativeEditor'; import { NativeEditorCommandListener } from './interactive-ipynb/nativeEditorCommandListener'; import { NativeEditorProvider } from './interactive-ipynb/nativeEditorProvider'; @@ -123,6 +124,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.add(IInteractiveWindowListener, wrapType(ShowPlotListener)); serviceManager.add(IInteractiveWindowListener, wrapType(DebugListener)); serviceManager.add(IInteractiveWindowListener, wrapType(GatherListener)); + serviceManager.add(IInteractiveWindowListener, wrapType(AutoSaveService)); serviceManager.addSingleton(IPlotViewerProvider, wrapType(PlotViewerProvider)); serviceManager.add(IPlotViewer, wrapType(PlotViewer)); serviceManager.addSingleton(IJupyterDebugger, wrapType(JupyterDebugger)); diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts index 42bcb6e124c0..b1b63e697afb 100644 --- a/src/datascience-ui/native-editor/autoSaveService.ts +++ b/src/datascience-ui/native-editor/autoSaveService.ts @@ -15,7 +15,7 @@ export class AutoSaveService implements IDisposable, IMessageHandler { private settings?: FileSettings; // private fileSettings: typeof constructor(private readonly controller: NativeEditorStateController) { - this.initialize(); + this.setTimer(); } public dispose() { this.clearTimeout(); @@ -26,7 +26,7 @@ export class AutoSaveService implements IDisposable, IMessageHandler { // When clean message is sent, this means notebook was saved. // We need to reset the timer to start again (timer starts from last save). case InteractiveWindowMessages.NotebookClean: { - this.initialize(); + this.setTimer(); return true; } // When settings have been updated, its possible the timer has changed. @@ -36,7 +36,7 @@ export class AutoSaveService implements IDisposable, IMessageHandler { if (this.settings && this.settings.autoSave === settings.autoSave && this.settings.autoSaveDelay === settings.autoSaveDelay) { return true; } - this.initialize(); + this.setTimer(); return true; } case InteractiveWindowMessages.ActiveTextEditorChanged: { @@ -58,11 +58,12 @@ export class AutoSaveService implements IDisposable, IMessageHandler { } } } - public initialize() { + public setTimer() { this.clearTimeout(); - const settings = this.settings = getSettings().files; + const settings = (this.settings = getSettings().files); if (settings.autoSave === 'afterDelay') { // Add a timeout to save after n milli seconds. + // Do not use setInterval, as that will cause all handlers to queue up. this.timeout = setTimeout(() => this.save(), settings.autoSaveDelay); } } @@ -83,9 +84,13 @@ export class AutoSaveService implements IDisposable, IMessageHandler { */ private save() { const settings = getSettings().files; - if (settings.autoSave === 'off' || this.controller.getState().dirty !== true) { + if (settings.autoSave === 'off') { return; } - this.controller.save(); + if (this.controller.getState().dirty === true) { + this.controller.save(); + } else { + this.setTimer(); + } } } From b3f5b50f7c7a331a79db42d9b1a91cac6e6869d2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 8 Oct 2019 19:54:27 -0700 Subject: [PATCH 08/23] Fixes --- src/datascience-ui/native-editor/autoSaveService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts index b1b63e697afb..76996edab282 100644 --- a/src/datascience-ui/native-editor/autoSaveService.ts +++ b/src/datascience-ui/native-editor/autoSaveService.ts @@ -60,11 +60,11 @@ export class AutoSaveService implements IDisposable, IMessageHandler { } public setTimer() { this.clearTimeout(); - const settings = (this.settings = getSettings().files); - if (settings.autoSave === 'afterDelay') { + this.settings = getSettings().files; + if (this.settings && this.settings.autoSave === 'afterDelay') { // Add a timeout to save after n milli seconds. // Do not use setInterval, as that will cause all handlers to queue up. - this.timeout = setTimeout(() => this.save(), settings.autoSaveDelay); + this.timeout = setTimeout(() => this.save(), this.settings.autoSaveDelay); } } private clearTimeout() { From b2df34e7f9b819576f10aab92d9db1db319ceb1e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 09:09:50 -0700 Subject: [PATCH 09/23] Added tests --- .../interactive-ipynb/autoSaveService.ts | 2 +- src/client/datascience/webViewHost.ts | 2 +- src/test/common.ts | 36 +- .../datascience/dataScienceIocContainer.ts | 47 +- src/test/datascience/mockDocumentManager.ts | 2 +- src/test/datascience/mockWorkspaceConfig.ts | 28 + .../nativeEditor.functional.test.tsx | 1237 +++++++++++------ .../datascience/nativeEditorTestHelpers.tsx | 4 +- src/test/datascience/reactHelpers.ts | 6 +- src/test/datascience/testHelpers.tsx | 29 + 10 files changed, 901 insertions(+), 492 deletions(-) create mode 100644 src/test/datascience/mockWorkspaceConfig.ts diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index 10b5f39bdec6..106aceb16877 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -41,7 +41,7 @@ export class AutoSaveService implements IInteractiveWindowListener { noop(); } public dispose(): void | undefined { - this.disposables.forEach(item => item.dispose()); + this.disposables.filter(item => !!item).forEach(item => item.dispose()); } private onDidChangeWindowState(e: WindowState) { diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index 4315c804ecf9..ae4c46d11767 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -170,7 +170,7 @@ export class WebViewHost implements IDisposable { ...this.configService.getSettings().datascience, files: { autoSaveDelay: this.getValue(files, 'autoSaveDelay', 1000), - autoSave: this.getValue(files, 'autoSave', 'afterDelay') + autoSave: this.getValue(files, 'autoSave', 'off') }, extraSettings: { editorCursor: this.getValue(editor, 'cursorStyle', 'line'), diff --git a/src/test/common.ts b/src/test/common.ts index a5172832b799..8cf22d833fde 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -410,27 +410,35 @@ export async function unzip(zipFile: string, targetFolder: string): Promise Promise} condition + * @param {number} timeoutMs + * @param {string} errorMessage + * @returns {Promise} + */ export async function waitForCondition(condition: () => Promise, timeoutMs: number, errorMessage: string): Promise { return new Promise(async (resolve, reject) => { - let completed = false; const timeout = setTimeout(() => { - if (!completed) { - reject(new Error(errorMessage)); - } - completed = true; + clearTimeout(timeout); + // tslint:disable-next-line: no-use-before-declare + clearTimeout(timer); + reject(new Error(errorMessage)); }, timeoutMs); - for (let i = 0; i < timeoutMs / 1000; i += 1) { - if (await condition()) { + const timer = setInterval(async () => { + try { + if (!await condition()){ + throw new Error('failed'); + } clearTimeout(timeout); + clearTimeout(timer); resolve(); - return; + } catch { + noop(); } - await sleep(500); - if (completed) { - return; - } - } + }, 10); }); } diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 9cd482d3023b..1ff5fdb9cf2b 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -7,6 +7,7 @@ import { ReactWrapper } from 'enzyme'; import { interfaces } from 'inversify'; import * as path from 'path'; import { SemVer } from 'semver'; +import { anything, instance, mock, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { ConfigurationChangeEvent, @@ -20,7 +21,6 @@ import { WorkspaceFolder } from 'vscode'; import * as vsls from 'vsls/vscode'; - import { ILanguageServer, ILanguageServerAnalysisOptions } from '../../client/activation/types'; import { TerminalManager } from '../../client/common/application/terminalManager'; import { @@ -37,6 +37,7 @@ import { IWorkspaceService, WebPanelMessage } from '../../client/common/application/types'; +import { WorkspaceService } from '../../client/common/application/workspace'; import { AsyncDisposableRegistry } from '../../client/common/asyncDisposableRegistry'; import { PythonSettings } from '../../client/common/configSettings'; import { EXTENSION_ROOT_DIR } from '../../client/common/constants'; @@ -108,6 +109,7 @@ import { GatherListener } from '../../client/datascience/gather/gatherListener'; import { DotNetIntellisenseProvider } from '../../client/datascience/interactive-common/intellisense/dotNetIntellisenseProvider'; +import { AutoSaveService } from '../../client/datascience/interactive-ipynb/autoSaveService'; import { NativeEditor } from '../../client/datascience/interactive-ipynb/nativeEditor'; import { NativeEditorCommandListener } from '../../client/datascience/interactive-ipynb/nativeEditorCommandListener'; import { InteractiveWindow } from '../../client/datascience/interactive-window/interactiveWindow'; @@ -249,6 +251,7 @@ import { MockJupyterManagerFactory } from './mockJupyterManagerFactory'; import { MockLanguageServer } from './mockLanguageServer'; import { MockLanguageServerAnalysisOptions } from './mockLanguageServerAnalysisOptions'; import { MockLiveShareApi } from './mockLiveShare'; +import { MockWorkspaceConfiguration } from './mockWorkspaceConfig'; import { blurWindow, createMessageEvent } from './reactHelpers'; import { TestInteractiveWindowProvider } from './testInteractiveWindowProvider'; import { TestNativeEditorProvider } from './testNativeEditorProvider'; @@ -259,6 +262,8 @@ export class DataScienceIocContainer extends UnitTestIocContainer { public wrapper: ReactWrapper, React.Component> | undefined; public wrapperCreatedPromise: Deferred | undefined; public postMessage: ((ev: MessageEvent) => void) | undefined; + public mockedWorkspaceConfig!: WorkspaceConfiguration; + public applicationShell!: TypeMoq.IMock; // tslint:disable-next-line:no-any private missedMessages: any[] = []; private pythonSettings = new class extends PythonSettings { @@ -391,6 +396,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingleton(ILanguageServer, MockLanguageServer); this.serviceManager.addSingleton(ILanguageServerAnalysisOptions, MockLanguageServerAnalysisOptions); this.serviceManager.add(IInteractiveWindowListener, DotNetIntellisenseProvider); + this.serviceManager.add(IInteractiveWindowListener, AutoSaveService); this.serviceManager.add(IProtocolParser, ProtocolParser); this.serviceManager.addSingleton(IDebugService, MockDebuggerService); this.serviceManager.addSingleton(ICellHashProvider, CellHashProvider); @@ -416,8 +422,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { // Also setup a mock execution service and interpreter service const condaService = TypeMoq.Mock.ofType(); - const appShell = TypeMoq.Mock.ofType(); - const workspaceService = TypeMoq.Mock.ofType(); + const appShell = this.applicationShell = TypeMoq.Mock.ofType(); + // const workspaceService = TypeMoq.Mock.ofType(); + const workspaceService = mock(WorkspaceService); const configurationService = TypeMoq.Mock.ofType(); const interpreterDisplay = TypeMoq.Mock.ofType(); const datascience = TypeMoq.Mock.ofType(); @@ -454,18 +461,14 @@ export class DataScienceIocContainer extends UnitTestIocContainer { debugJustMyCode: true }; - const workspaceConfig: TypeMoq.IMock = TypeMoq.Mock.ofType(); - workspaceConfig.setup(ws => ws.has(TypeMoq.It.isAnyString())) - .returns(() => false); - workspaceConfig.setup(ws => ws.get(TypeMoq.It.isAnyString())) - .returns(() => undefined); - workspaceConfig.setup(ws => ws.get(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())) - .returns((_s, d) => d); - + const workspaceConfig = this.mockedWorkspaceConfig = mock(MockWorkspaceConfiguration); configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => this.pythonSettings); - workspaceService.setup(c => c.getConfiguration(TypeMoq.It.isAny())).returns(() => workspaceConfig.object); - workspaceService.setup(c => c.getConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => workspaceConfig.object); - workspaceService.setup(w => w.onDidChangeConfiguration).returns(() => this.configChangeEvent.event); + when(workspaceConfig.get(anything(), anything())).thenCall((_, defaultValue) => defaultValue); + when(workspaceConfig.has(anything())).thenReturn(false); + when((workspaceConfig as any).then).thenReturn(undefined); + when(workspaceService.getConfiguration(anything())).thenReturn(instance(workspaceConfig)); + when(workspaceService.getConfiguration(anything(), anything())).thenReturn(instance(workspaceConfig)); + when(workspaceService.onDidChangeConfiguration).thenReturn(this.configChangeEvent.event); interpreterDisplay.setup(i => i.refresh(TypeMoq.It.isAny())).returns(() => Promise.resolve()); const startTime = Date.now(); datascience.setup(d => d.activationStartTime).returns(() => startTime); @@ -490,18 +493,12 @@ export class DataScienceIocContainer extends UnitTestIocContainer { noop(); } } - workspaceService.setup(w => w.createFileSystemWatcher(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { - return new MockFileSystemWatcher(); - }); - workspaceService - .setup(w => w.hasWorkspaceFolders) - .returns(() => true); + when(workspaceService.createFileSystemWatcher(anything(), anything(), anything(), anything())).thenReturn(new MockFileSystemWatcher()); + when(workspaceService.hasWorkspaceFolders).thenReturn(true); const testWorkspaceFolder = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience'); const workspaceFolder = this.createMoqWorkspaceFolder(testWorkspaceFolder); - workspaceService - .setup(w => w.workspaceFolders) - .returns(() => [workspaceFolder]); - workspaceService.setup(w => w.rootPath).returns(() => '~'); + when(workspaceService.workspaceFolders).thenReturn([workspaceFolder]); + when(workspaceService.rootPath).thenReturn('~'); // Look on the path for python const pythonPath = this.findPythonPath(); @@ -528,7 +525,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingletonInstance(ICondaService, condaService.object); this.serviceManager.addSingletonInstance(IApplicationShell, appShell.object); this.serviceManager.addSingletonInstance(IDocumentManager, this.documentManager); - this.serviceManager.addSingletonInstance(IWorkspaceService, workspaceService.object); + this.serviceManager.addSingletonInstance(IWorkspaceService, instance(workspaceService)); this.serviceManager.addSingletonInstance(IConfigurationService, configurationService.object); this.serviceManager.addSingletonInstance(IDataScience, datascience.object); this.serviceManager.addSingleton(IBufferDecoder, BufferDecoder); diff --git a/src/test/datascience/mockDocumentManager.ts b/src/test/datascience/mockDocumentManager.ts index 3ef780303f82..3c8052fc2c08 100644 --- a/src/test/datascience/mockDocumentManager.ts +++ b/src/test/datascience/mockDocumentManager.ts @@ -31,7 +31,7 @@ export class MockDocumentManager implements IDocumentManager { public textDocuments: TextDocument[] = []; public activeTextEditor: TextEditor | undefined; public visibleTextEditors: TextEditor[] = []; - private didChangeEmitter = new EventEmitter(); + public didChangeEmitter = new EventEmitter(); private didOpenEmitter = new EventEmitter(); private didChangeVisibleEmitter = new EventEmitter(); private didChangeTextEditorSelectionEmitter = new EventEmitter(); diff --git a/src/test/datascience/mockWorkspaceConfig.ts b/src/test/datascience/mockWorkspaceConfig.ts new file mode 100644 index 000000000000..5a6b6e1fe48e --- /dev/null +++ b/src/test/datascience/mockWorkspaceConfig.ts @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { ConfigurationTarget, WorkspaceConfiguration } from 'vscode'; + +export class MockWorkspaceConfiguration implements WorkspaceConfiguration { + // tslint:disable: no-any + public get(key: string): any; + public get(section: string): T | undefined; + public get(section: string, defaultValue: T): T; + public get(section: any, defaultValue?: any): any; + public get(_: string, defaultValue?: any): any { + return arguments.length > 1 ? defaultValue : (undefined as any); + } + public has(_section: string): boolean { + return false; + } + public inspect( + _section: string + ): { key: string; defaultValue?: T | undefined; globalValue?: T | undefined; workspaceValue?: T | undefined; workspaceFolderValue?: T | undefined } | undefined { + return; + } + public update(_section: string, _value: any, _configurationTarget?: boolean | ConfigurationTarget | undefined): Promise { + return Promise.resolve(); + } +} diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index dc0704d81c01..549a7405e002 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -3,16 +3,22 @@ 'use strict'; -import * as assert from 'assert'; +import { assert, expect, use } from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; import { ReactWrapper } from 'enzyme'; -import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; +import { EventEmitter } from 'events'; +import * as fs from 'fs-extra'; import * as path from 'path'; +import * as sinon from 'sinon'; +import { anything, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; -import { Disposable, TextDocument, TextEditor, Uri } from 'vscode'; +import { Disposable, TextDocument, TextEditor, Uri, WindowState } from 'vscode'; import { IApplicationShell, IDocumentManager } from '../../client/common/application/types'; import { createDeferred } from '../../client/common/utils/async'; +import { createTemporaryFile } from '../../client/common/utils/fs'; import { noop } from '../../client/common/utils/misc'; import { Identifiers } from '../../client/datascience/constants'; +import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { JupyterExecutionFactory } from '../../client/datascience/jupyter/jupyterExecutionFactory'; import { ICell, IJupyterExecution, INotebookEditorProvider, INotebookExporter } from '../../client/datascience/types'; import { PythonInterpreter } from '../../client/interpreter/contracts'; @@ -21,16 +27,14 @@ import { CellOutput } from '../../datascience-ui/interactive-common/cellOutput'; import { Editor } from '../../datascience-ui/interactive-common/editor'; import { NativeCell } from '../../datascience-ui/native-editor/nativeCell'; import { NativeEditor } from '../../datascience-ui/native-editor/nativeEditor'; +import { NativeEditorStateController } from '../../datascience-ui/native-editor/nativeEditorStateController'; import { IKeyboardEvent } from '../../datascience-ui/react-common/event'; import { ImageButton } from '../../datascience-ui/react-common/imageButton'; import { IMonacoEditorState, MonacoEditor } from '../../datascience-ui/react-common/monacoEditor'; +import { waitForCondition } from '../common'; import { DataScienceIocContainer } from './dataScienceIocContainer'; -import { - addCell, - closeNotebook, - createNewEditor, - getNativeCellResults, - mountNativeWebView, openEditor, runMountedTest, setupWebview } from './nativeEditorTestHelpers'; +import { MockDocumentManager } from './mockDocumentManager'; +import { addCell, closeNotebook, createNewEditor, getNativeCellResults, mountNativeWebView, openEditor, runMountedTest, setupWebview } from './nativeEditorTestHelpers'; import { waitForUpdate } from './reactHelpers'; import { addContinuousMockData, @@ -47,13 +51,13 @@ import { verifyHtmlOnCell, waitForMessageResponse } from './testHelpers'; +use(chaiAsPromised); //import { asyncDump } from '../common/asyncDump'; // tslint:disable:max-func-body-length trailing-comma no-any no-multiline-string suite('DataScience Native Editor', () => { - function createFileCell(cell: any, data: any): ICell { - const newCell = { type: 'preview', id: 'FakeID', file: Identifiers.EmptyFileName, line: 0, state: 2, ...cell}; + const newCell = { type: 'preview', id: 'FakeID', file: Identifiers.EmptyFileName, line: 0, state: 2, ...cell }; newCell.data = { cell_type: 'code', execution_count: null, metadata: {}, outputs: [], source: '', ...data }; return newCell; @@ -67,13 +71,16 @@ suite('DataScience Native Editor', () => { ioc.registerDataScienceTypes(); const appShell = TypeMoq.Mock.ofType(); - appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns((_e) => Promise.resolve('')); + appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns(_e => Promise.resolve('')); appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve('')); - appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((_a1: string, a2: string, _a3: string) => Promise.resolve(a2)); - appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((_a1: string, _a2: any, _a3: string, a4: string) => Promise.resolve(a4)); + appShell + .setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns((_a1: string, a2: string, _a3: string) => Promise.resolve(a2)); + appShell + .setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns((_a1: string, _a2: any, _a3: string, a4: string) => Promise.resolve(a4)); appShell.setup(a => a.showSaveDialog(TypeMoq.It.isAny())).returns(() => Promise.resolve(Uri.file('foo.ipynb'))); ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); - }); teardown(async () => { @@ -95,29 +102,37 @@ suite('DataScience Native Editor', () => { // asyncDump(); // }); - runMountedTest('Simple text', async (wrapper) => { - // Create an editor so something is listening to messages - await createNewEditor(ioc); + runMountedTest( + 'Simple text', + async wrapper => { + // Create an editor so something is listening to messages + await createNewEditor(ioc); - // Add a cell into the UI and wait for it to render - await addCell(wrapper, 'a=1\na'); + // Add a cell into the UI and wait for it to render + await addCell(wrapper, 'a=1\na'); - verifyHtmlOnCell(wrapper, 'NativeCell', '1', CellPosition.Last); - }, () => { return ioc; }); + verifyHtmlOnCell(wrapper, 'NativeCell', '1', CellPosition.Last); + }, + () => { + return ioc; + } + ); - runMountedTest('Mime Types', async (wrapper) => { - // Create an editor so something is listening to messages - await createNewEditor(ioc); + runMountedTest( + 'Mime Types', + async wrapper => { + // Create an editor so something is listening to messages + await createNewEditor(ioc); - const badPanda = `import pandas as pd + const badPanda = `import pandas as pd df = pd.read("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") df.head()`; - const goodPanda = `import pandas as pd + const goodPanda = `import pandas as pd df = pd.read_csv("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") df.head()`; - const matPlotLib = 'import matplotlib.pyplot as plt\r\nimport numpy as np\r\nx = np.linspace(0,20,100)\r\nplt.plot(x, np.sin(x))\r\nplt.show()'; - const matPlotLibResults = 'img'; - const spinningCursor = `import sys + const matPlotLib = 'import matplotlib.pyplot as plt\r\nimport numpy as np\r\nx = np.linspace(0,20,100)\r\nplt.plot(x, np.sin(x))\r\nplt.show()'; + const matPlotLibResults = 'img'; + const spinningCursor = `import sys import time def spinning_cursor(): while True: @@ -130,164 +145,206 @@ suite('DataScience Native Editor', () => { time.sleep(0.1) sys.stdout.write('\\r')`; - addMockData(ioc, badPanda, `pandas has no attribute 'read'`, 'text/html', 'error'); - addMockData(ioc, goodPanda, `A table`, 'text/html'); - addMockData(ioc, matPlotLib, matPlotLibResults, 'text/html'); - const cursors = ['|', '/', '-', '\\']; - let cursorPos = 0; - let loops = 3; - addContinuousMockData(ioc, spinningCursor, async (_c) => { - const result = `${cursors[cursorPos]}\r`; - cursorPos += 1; - if (cursorPos >= cursors.length) { - cursorPos = 0; - loops -= 1; - } - return Promise.resolve({ result: result, haveMore: loops > 0 }); - }); + addMockData(ioc, badPanda, `pandas has no attribute 'read'`, 'text/html', 'error'); + addMockData(ioc, goodPanda, `A table`, 'text/html'); + addMockData(ioc, matPlotLib, matPlotLibResults, 'text/html'); + const cursors = ['|', '/', '-', '\\']; + let cursorPos = 0; + let loops = 3; + addContinuousMockData(ioc, spinningCursor, async _c => { + const result = `${cursors[cursorPos]}\r`; + cursorPos += 1; + if (cursorPos >= cursors.length) { + cursorPos = 0; + loops -= 1; + } + return Promise.resolve({ result: result, haveMore: loops > 0 }); + }); - await addCell(wrapper, badPanda, true); - verifyHtmlOnCell(wrapper, 'NativeCell', `has no attribute 'read'`, CellPosition.Last); - - await addCell(wrapper, goodPanda, true); - verifyHtmlOnCell(wrapper, 'NativeCell', ``, CellPosition.Last); - - await addCell(wrapper, matPlotLib, true, 6); - verifyHtmlOnCell(wrapper, 'NativeCell', matPlotLibResults, CellPosition.Last); - - await addCell(wrapper, spinningCursor, true, 4 + (ioc.mockJupyter ? (cursors.length * 3) : 50)); - verifyHtmlOnCell(wrapper, 'NativeCell', '
', CellPosition.Last); - }, () => { return ioc; }); - - runMountedTest('Click buttons', async (wrapper) => { - // Goto source should cause the visible editor to be picked as long as its filename matches - const showedEditor = createDeferred(); - const textEditors: TextEditor[] = []; - const docManager = TypeMoq.Mock.ofType(); - const visibleEditor = TypeMoq.Mock.ofType(); - const dummyDocument = TypeMoq.Mock.ofType(); - dummyDocument.setup(d => d.fileName).returns(() => 'foo.py'); - visibleEditor.setup(v => v.show()).returns(() => showedEditor.resolve()); - visibleEditor.setup(v => v.revealRange(TypeMoq.It.isAny())).returns(noop); - visibleEditor.setup(v => v.document).returns(() => dummyDocument.object); - textEditors.push(visibleEditor.object); - docManager.setup(a => a.visibleTextEditors).returns(() => textEditors); - ioc.serviceManager.rebindInstance(IDocumentManager, docManager.object); - // Create an editor so something is listening to messages - await createNewEditor(ioc); + await addCell(wrapper, badPanda, true); + verifyHtmlOnCell(wrapper, 'NativeCell', `has no attribute 'read'`, CellPosition.Last); - // Get a cell into the list - await addCell(wrapper, 'a=1\na'); + await addCell(wrapper, goodPanda, true); + verifyHtmlOnCell(wrapper, 'NativeCell', ``, CellPosition.Last); - // find the buttons on the cell itself - let cell = getLastOutputCell(wrapper, 'NativeCell'); - let ImageButtons = cell.find(ImageButton); - assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); - let deleteButton = ImageButtons.at(6); + await addCell(wrapper, matPlotLib, true, 6); + verifyHtmlOnCell(wrapper, 'NativeCell', matPlotLibResults, CellPosition.Last); - // Make sure delete works - let afterDelete = await getNativeCellResults(wrapper, 1, async () => { - deleteButton.simulate('click'); - return Promise.resolve(); - }); - assert.equal(afterDelete.length, 1, `Delete should remove a cell`); - - // Secondary delete should NOT delete the cell as there should ALWAYS be at - // least one cell in the file. - cell = getLastOutputCell(wrapper, 'NativeCell'); - ImageButtons = cell.find(ImageButton); - assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); - deleteButton = ImageButtons.at(6); - - afterDelete = await getNativeCellResults(wrapper, 1, async () => { - deleteButton.simulate('click'); - return Promise.resolve(); - }); - assert.equal(afterDelete.length, 1, `Delete should NOT remove the last cell`); - }, () => { return ioc; }); - - runMountedTest('Export', async (wrapper) => { - // Export should cause the export dialog to come up. Remap appshell so we can check - const dummyDisposable = { - dispose: () => { return; } - }; - let exportCalled = false; - const appShell = TypeMoq.Mock.ofType(); - appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns((e) => { throw e; }); - appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve('')); - appShell.setup(a => a.showSaveDialog(TypeMoq.It.isAny())).returns(() => { - exportCalled = true; - return Promise.resolve(undefined); - }); - appShell.setup(a => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable); - ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); + await addCell(wrapper, spinningCursor, true, 4 + (ioc.mockJupyter ? cursors.length * 3 : 50)); + verifyHtmlOnCell(wrapper, 'NativeCell', '
', CellPosition.Last); + }, + () => { + return ioc; + } + ); + + runMountedTest( + 'Click buttons', + async wrapper => { + // Goto source should cause the visible editor to be picked as long as its filename matches + const showedEditor = createDeferred(); + const textEditors: TextEditor[] = []; + const docManager = TypeMoq.Mock.ofType(); + const visibleEditor = TypeMoq.Mock.ofType(); + const dummyDocument = TypeMoq.Mock.ofType(); + dummyDocument.setup(d => d.fileName).returns(() => 'foo.py'); + visibleEditor.setup(v => v.show()).returns(() => showedEditor.resolve()); + visibleEditor.setup(v => v.revealRange(TypeMoq.It.isAny())).returns(noop); + visibleEditor.setup(v => v.document).returns(() => dummyDocument.object); + textEditors.push(visibleEditor.object); + docManager.setup(a => a.visibleTextEditors).returns(() => textEditors); + ioc.serviceManager.rebindInstance(IDocumentManager, docManager.object); + // Create an editor so something is listening to messages + await createNewEditor(ioc); + + // Get a cell into the list + await addCell(wrapper, 'a=1\na'); + + // find the buttons on the cell itself + let cell = getLastOutputCell(wrapper, 'NativeCell'); + let ImageButtons = cell.find(ImageButton); + assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); + let deleteButton = ImageButtons.at(6); + + // Make sure delete works + let afterDelete = await getNativeCellResults(wrapper, 1, async () => { + deleteButton.simulate('click'); + return Promise.resolve(); + }); + assert.equal(afterDelete.length, 1, `Delete should remove a cell`); + + // Secondary delete should NOT delete the cell as there should ALWAYS be at + // least one cell in the file. + cell = getLastOutputCell(wrapper, 'NativeCell'); + ImageButtons = cell.find(ImageButton); + assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); + deleteButton = ImageButtons.at(6); + + afterDelete = await getNativeCellResults(wrapper, 1, async () => { + deleteButton.simulate('click'); + return Promise.resolve(); + }); + assert.equal(afterDelete.length, 1, `Delete should NOT remove the last cell`); + }, + () => { + return ioc; + } + ); + + runMountedTest( + 'Export', + async wrapper => { + // Export should cause the export dialog to come up. Remap appshell so we can check + const dummyDisposable = { + dispose: () => { + return; + } + }; + let exportCalled = false; + const appShell = TypeMoq.Mock.ofType(); + appShell + .setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())) + .returns(e => { + throw e; + }); + appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve('')); + appShell + .setup(a => a.showSaveDialog(TypeMoq.It.isAny())) + .returns(() => { + exportCalled = true; + return Promise.resolve(undefined); + }); + appShell.setup(a => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable); + ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); + + // Make sure to create the interactive window after the rebind or it gets the wrong application shell. + await createNewEditor(ioc); + await addCell(wrapper, 'a=1\na'); + + // Export should cause exportCalled to change to true + const exportButton = findButton(wrapper, NativeEditor, 6); + await waitForMessageResponse(ioc, () => exportButton!.simulate('click')); + assert.equal(exportCalled, true, 'Export should have been called'); + }, + () => { + return ioc; + } + ); - // Make sure to create the interactive window after the rebind or it gets the wrong application shell. - await createNewEditor(ioc); - await addCell(wrapper, 'a=1\na'); - - // Export should cause exportCalled to change to true - const exportButton = findButton(wrapper, NativeEditor, 6); - await waitForMessageResponse(ioc, () => exportButton!.simulate('click')); - assert.equal(exportCalled, true, 'Export should have been called'); - }, () => { return ioc; }); - - runMountedTest('RunAllCells', async (wrapper) => { - addMockData(ioc, 'b=2\nb', 2); - addMockData(ioc, 'c=3\nc', 3); - - const baseFile = [ {id: 'NotebookImport#0', data: {source: 'a=1\na'}}, - {id: 'NotebookImport#1', data: {source: 'b=2\nb'}}, - {id: 'NotebookImport#2', data: {source: 'c=3\nc'}} ]; - const runAllCells = baseFile.map(cell => { - return createFileCell(cell, cell.data); - }); - const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); - await openEditor(ioc, JSON.stringify(notebook)); + runMountedTest( + 'RunAllCells', + async wrapper => { + addMockData(ioc, 'b=2\nb', 2); + addMockData(ioc, 'c=3\nc', 3); - // Export should cause exportCalled to change to true - const runAllButton = findButton(wrapper, NativeEditor, 3); - await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); + const baseFile = [ + { id: 'NotebookImport#0', data: { source: 'a=1\na' } }, + { id: 'NotebookImport#1', data: { source: 'b=2\nb' } }, + { id: 'NotebookImport#2', data: { source: 'c=3\nc' } } + ]; + const runAllCells = baseFile.map(cell => { + return createFileCell(cell, cell.data); + }); + const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); + await openEditor(ioc, JSON.stringify(notebook)); - await waitForUpdate(wrapper, NativeEditor, 16); + // Export should cause exportCalled to change to true + const runAllButton = findButton(wrapper, NativeEditor, 3); + await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); - verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); - verifyHtmlOnCell(wrapper, 'NativeCell', `2`, 1); - verifyHtmlOnCell(wrapper, 'NativeCell', `3`, 2); - }, () => { return ioc; }); + await waitForUpdate(wrapper, NativeEditor, 16); - runMountedTest('Startup and shutdown', async (wrapper) => { - addMockData(ioc, 'b=2\nb', 2); - addMockData(ioc, 'c=3\nc', 3); + verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); + verifyHtmlOnCell(wrapper, 'NativeCell', `2`, 1); + verifyHtmlOnCell(wrapper, 'NativeCell', `3`, 2); + }, + () => { + return ioc; + } + ); - const baseFile = [ {id: 'NotebookImport#0', data: {source: 'a=1\na'}}, - {id: 'NotebookImport#1', data: {source: 'b=2\nb'}}, - {id: 'NotebookImport#2', data: {source: 'c=3\nc'}} ]; - const runAllCells = baseFile.map(cell => { - return createFileCell(cell, cell.data); - }); - const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); - let editor = await openEditor(ioc, JSON.stringify(notebook)); - - // Run everything - let runAllButton = findButton(wrapper, NativeEditor, 3); - await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); - await waitForUpdate(wrapper, NativeEditor, 16); - - // Close editor. Should still have the server up - await closeNotebook(editor, wrapper); - const jupyterExecution = ioc.serviceManager.get(IJupyterExecution); - const editorProvider = ioc.serviceManager.get(INotebookEditorProvider); - const server = await jupyterExecution.getServer(await editorProvider.getNotebookOptions()); - assert.ok(server, 'Server was destroyed on notebook shutdown'); - - // Reopen, and rerun - editor = await openEditor(ioc, JSON.stringify(notebook)); - runAllButton = findButton(wrapper, NativeEditor, 3); - await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); - await waitForUpdate(wrapper, NativeEditor, 15); - verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); - }, () => { return ioc; }); + runMountedTest( + 'Startup and shutdown', + async wrapper => { + addMockData(ioc, 'b=2\nb', 2); + addMockData(ioc, 'c=3\nc', 3); + + const baseFile = [ + { id: 'NotebookImport#0', data: { source: 'a=1\na' } }, + { id: 'NotebookImport#1', data: { source: 'b=2\nb' } }, + { id: 'NotebookImport#2', data: { source: 'c=3\nc' } } + ]; + const runAllCells = baseFile.map(cell => { + return createFileCell(cell, cell.data); + }); + const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); + let editor = await openEditor(ioc, JSON.stringify(notebook)); + + // Run everything + let runAllButton = findButton(wrapper, NativeEditor, 3); + await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); + await waitForUpdate(wrapper, NativeEditor, 16); + + // Close editor. Should still have the server up + await closeNotebook(editor, wrapper); + const jupyterExecution = ioc.serviceManager.get(IJupyterExecution); + const editorProvider = ioc.serviceManager.get(INotebookEditorProvider); + const server = await jupyterExecution.getServer(await editorProvider.getNotebookOptions()); + assert.ok(server, 'Server was destroyed on notebook shutdown'); + + // Reopen, and rerun + editor = await openEditor(ioc, JSON.stringify(notebook)); + runAllButton = findButton(wrapper, NativeEditor, 3); + await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); + await waitForUpdate(wrapper, NativeEditor, 15); + verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); + }, + () => { + // Disable the warning displayed by nodejs when there are too many listeners. + EventEmitter.defaultMaxListeners = 15; + return ioc; + } + ); test('Failure', async () => { // Make a dummy class that will fail during launch @@ -308,7 +365,7 @@ suite('DataScience Native Editor', () => { }); }); - suite('Editor Keyboard tests', () => { + suite('Editor tests', () => { let wrapper: ReactWrapper, React.Component>; const disposables: Disposable[] = []; let ioc: DataScienceIocContainer; @@ -317,9 +374,15 @@ suite('DataScience Native Editor', () => { { id: 'NotebookImport#1', data: { source: 'b=2\nb' } }, { id: 'NotebookImport#2', data: { source: 'c=3\nc' } } ]; - setup(async function() { + let notebookFile: { + filePath: string; + cleanupCallback: Function; + }; + function initIoc() { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); + } + async function setupFunction(this: Mocha.Context) { const wrapperPossiblyUndefined = await setupWebview(ioc); if (wrapperPossiblyUndefined) { wrapper = wrapperPossiblyUndefined; @@ -330,13 +393,16 @@ suite('DataScience Native Editor', () => { const runAllCells = baseFile.map(cell => { return createFileCell(cell, cell.data); }); + // Use a real file so we can save notebook to a file. + // This is used in some tests (saving). + notebookFile = await createTemporaryFile('.ipynb'); const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); - await Promise.all([waitForUpdate(wrapper, NativeEditor, 1), openEditor(ioc, JSON.stringify(notebook))]); + await Promise.all([waitForUpdate(wrapper, NativeEditor, 1), openEditor(ioc, JSON.stringify(notebook), notebookFile.filePath)]); } else { // tslint:disable-next-line: no-invalid-this this.skip(); } - }); + } teardown(async () => { for (const disposable of disposables) { @@ -350,6 +416,11 @@ suite('DataScience Native Editor', () => { } } await ioc.dispose(); + try { + notebookFile.cleanupCallback(); + } catch { + noop(); + } }); function clickCell(cellIndex: number) { @@ -359,6 +430,7 @@ suite('DataScience Native Editor', () => { .simulate('click'); wrapper.update(); } + function simulateKeyPressOnCell(cellIndex: number, keyboardEvent: Partial & { code: string }) { const event = { ...createKeyboardEventForCell(keyboardEvent), ...keyboardEvent }; wrapper @@ -369,324 +441,595 @@ suite('DataScience Native Editor', () => { wrapper.update(); } - test('None of the cells are selected by default', async () => { - assert.ok(!isCellSelected(wrapper, 'NativeCell', 0)); - assert.ok(!isCellSelected(wrapper, 'NativeCell', 1)); - assert.ok(!isCellSelected(wrapper, 'NativeCell', 2)); - }); + suite('Selection/Focus', () => { + setup(async function() { + initIoc(); + // tslint:disable-next-line: no-invalid-this + await setupFunction.call(this); + }); + test('None of the cells are selected by default', async () => { + assert.ok(!isCellSelected(wrapper, 'NativeCell', 0)); + assert.ok(!isCellSelected(wrapper, 'NativeCell', 1)); + assert.ok(!isCellSelected(wrapper, 'NativeCell', 2)); + }); - test('None of the cells are not focused by default', async () => { - assert.ok(!isCellFocused(wrapper, 'NativeCell', 0)); - assert.ok(!isCellFocused(wrapper, 'NativeCell', 1)); - assert.ok(!isCellFocused(wrapper, 'NativeCell', 2)); - }); + test('None of the cells are not focused by default', async () => { + assert.ok(!isCellFocused(wrapper, 'NativeCell', 0)); + assert.ok(!isCellFocused(wrapper, 'NativeCell', 1)); + assert.ok(!isCellFocused(wrapper, 'NativeCell', 2)); + }); - test('Select cells by clicking them', async () => { - // Click first cell, then second, then third. - clickCell(0); - assert.ok(isCellSelected(wrapper, 'NativeCell', 0)); - assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); - assert.equal(isCellSelected(wrapper, 'NativeCell', 2), false); - - clickCell(1); - assert.ok(isCellSelected(wrapper, 'NativeCell', 1)); - assert.equal(isCellSelected(wrapper, 'NativeCell', 0), false); - assert.equal(isCellSelected(wrapper, 'NativeCell', 2), false); - - clickCell(2); - assert.ok(isCellSelected(wrapper, 'NativeCell', 2)); - assert.equal(isCellSelected(wrapper, 'NativeCell', 0), false); - assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); - }); + test('Select cells by clicking them', async () => { + // Click first cell, then second, then third. + clickCell(0); + assert.ok(isCellSelected(wrapper, 'NativeCell', 0)); + assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); + assert.equal(isCellSelected(wrapper, 'NativeCell', 2), false); - test('Traverse cells by using ArrowUp and ArrowDown, k and j', async () => { - const keyCodesAndPositions = [ - // When we press arrow down in the first cell, then second cell gets selected. - { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 0, expectedSelectedCell: 1 }, - { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 1, expectedSelectedCell: 2 }, - // Arrow down on last cell is a noop. - { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 2, expectedSelectedCell: 2 }, - // When we press arrow up in the last cell, then second cell (from bottom) gets selected. - { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 2, expectedSelectedCell: 1 }, - { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 1, expectedSelectedCell: 0 }, - // Arrow up on last cell is a noop. - { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 0, expectedSelectedCell: 0 }, - - // Same tests as above with k and j. - { keyCode: 'j', cellIndexToPressKeysOn: 0, expectedSelectedCell: 1 }, - { keyCode: 'j', cellIndexToPressKeysOn: 1, expectedSelectedCell: 2 }, - // Arrow down on last cell is a noop. - { keyCode: 'j', cellIndexToPressKeysOn: 2, expectedSelectedCell: 2 }, - { keyCode: 'k', cellIndexToPressKeysOn: 2, expectedSelectedCell: 1 }, - { keyCode: 'k', cellIndexToPressKeysOn: 1, expectedSelectedCell: 0 }, - // Arrow up on last cell is a noop. - { keyCode: 'k', cellIndexToPressKeysOn: 0, expectedSelectedCell: 0 } - ]; - - // keypress on first cell, then second, then third. - // Test navigation through all cells, by traversing up and down. - for (const testItem of keyCodesAndPositions) { - simulateKeyPressOnCell(testItem.cellIndexToPressKeysOn, { code: testItem.keyCode }); - - // Check if it is selected. - // Only the cell at the index should be selected, as that's what we click. - assert.ok(isCellSelected(wrapper, 'NativeCell', testItem.expectedSelectedCell) === true); - } - }); + clickCell(1); + assert.ok(isCellSelected(wrapper, 'NativeCell', 1)); + assert.equal(isCellSelected(wrapper, 'NativeCell', 0), false); + assert.equal(isCellSelected(wrapper, 'NativeCell', 2), false); - test('Traverse cells by using ArrowUp and ArrowDown, k and j', async () => { - const keyCodesAndPositions = [ - // When we press arrow down in the first cell, then second cell gets selected. - { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 0, expectedIndex: 1 }, - { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 1, expectedIndex: 2 }, - // Arrow down on last cell is a noop. - { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 2, expectedIndex: 2 }, - // When we press arrow up in the last cell, then second cell (from bottom) gets selected. - { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 2, expectedIndex: 1 }, - { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 1, expectedIndex: 0 }, - // Arrow up on last cell is a noop. - { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 0, expectedIndex: 0 } - ]; - - // keypress on first cell, then second, then third. - // Test navigation through all cells, by traversing up and down. - for (const testItem of keyCodesAndPositions) { - simulateKeyPressOnCell(testItem.cellIndexToPressKeysOn, { code: testItem.keyCode }); - - // Check if it is selected. - // Only the cell at the index should be selected, as that's what we click. - assert.ok(isCellSelected(wrapper, 'NativeCell', testItem.expectedIndex) === true); - } + clickCell(2); + assert.ok(isCellSelected(wrapper, 'NativeCell', 2)); + assert.equal(isCellSelected(wrapper, 'NativeCell', 0), false); + assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); + }); }); - test('Pressing \'Enter\' on a selected cell, results in focus being set to the code', async () => { - // For some reason we cannot allow setting focus to monaco editor. - // Tests are known to fall over if allowed. - const editor = wrapper - .find(NativeCell) - .at(1) - .find(Editor) - .first(); - (editor.instance() as Editor).giveFocus = () => editor.props().focused!(); - - const update = waitForUpdate(wrapper, NativeEditor, 1); - clickCell(1); - simulateKeyPressOnCell(1, { code: 'Enter', editorInfo: undefined }); - await update; - - // The second cell should be selected. - assert.ok(isCellFocused(wrapper, 'NativeCell', 1)); - }); + suite('Keyboard Shortcuts', () => { + setup(async function() { + initIoc(); + // tslint:disable-next-line: no-invalid-this + await setupFunction.call(this); + }); - test('Pressing \'Escape\' on a focused cell results in the cell being selected', async () => { - // First focus the cell. - let update = waitForUpdate(wrapper, NativeEditor, 1); - clickCell(1); - simulateKeyPressOnCell(1, { code: 'Enter', editorInfo: undefined }); - await update; - - // The second cell should be selected. - assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); - assert.equal(isCellFocused(wrapper, 'NativeCell', 1), true); - - // Now hit escape. - update = waitForUpdate(wrapper, NativeEditor, 1); - simulateKeyPressOnCell(1, { code: 'Escape' }); - await update; - - // Confirm it is no longer focused, and it is selected. - assert.equal(isCellSelected(wrapper, 'NativeCell', 1), true); - assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false); - }); + test('Traverse cells by using ArrowUp and ArrowDown, k and j', async () => { + const keyCodesAndPositions = [ + // When we press arrow down in the first cell, then second cell gets selected. + { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 0, expectedSelectedCell: 1 }, + { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 1, expectedSelectedCell: 2 }, + // Arrow down on last cell is a noop. + { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 2, expectedSelectedCell: 2 }, + // When we press arrow up in the last cell, then second cell (from bottom) gets selected. + { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 2, expectedSelectedCell: 1 }, + { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 1, expectedSelectedCell: 0 }, + // Arrow up on last cell is a noop. + { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 0, expectedSelectedCell: 0 }, + + // Same tests as above with k and j. + { keyCode: 'j', cellIndexToPressKeysOn: 0, expectedSelectedCell: 1 }, + { keyCode: 'j', cellIndexToPressKeysOn: 1, expectedSelectedCell: 2 }, + // Arrow down on last cell is a noop. + { keyCode: 'j', cellIndexToPressKeysOn: 2, expectedSelectedCell: 2 }, + { keyCode: 'k', cellIndexToPressKeysOn: 2, expectedSelectedCell: 1 }, + { keyCode: 'k', cellIndexToPressKeysOn: 1, expectedSelectedCell: 0 }, + // Arrow up on last cell is a noop. + { keyCode: 'k', cellIndexToPressKeysOn: 0, expectedSelectedCell: 0 } + ]; + + // keypress on first cell, then second, then third. + // Test navigation through all cells, by traversing up and down. + for (const testItem of keyCodesAndPositions) { + simulateKeyPressOnCell(testItem.cellIndexToPressKeysOn, { code: testItem.keyCode }); + + // Check if it is selected. + // Only the cell at the index should be selected, as that's what we click. + assert.ok(isCellSelected(wrapper, 'NativeCell', testItem.expectedSelectedCell) === true); + } + }); - test('Pressing \'Shift+Enter\' on a selected cell executes the cell and advances to the next cell', async () => { - clickCell(1); - const update = waitForUpdate(wrapper, NativeEditor, 7); - simulateKeyPressOnCell(1, { code: 'Enter', shiftKey: true, editorInfo: undefined }); - await update; - wrapper.update(); + test('Traverse cells by using ArrowUp and ArrowDown, k and j', async () => { + const keyCodesAndPositions = [ + // When we press arrow down in the first cell, then second cell gets selected. + { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 0, expectedIndex: 1 }, + { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 1, expectedIndex: 2 }, + // Arrow down on last cell is a noop. + { keyCode: 'ArrowDown', cellIndexToPressKeysOn: 2, expectedIndex: 2 }, + // When we press arrow up in the last cell, then second cell (from bottom) gets selected. + { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 2, expectedIndex: 1 }, + { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 1, expectedIndex: 0 }, + // Arrow up on last cell is a noop. + { keyCode: 'ArrowUp', cellIndexToPressKeysOn: 0, expectedIndex: 0 } + ]; + + // keypress on first cell, then second, then third. + // Test navigation through all cells, by traversing up and down. + for (const testItem of keyCodesAndPositions) { + simulateKeyPressOnCell(testItem.cellIndexToPressKeysOn, { code: testItem.keyCode }); + + // Check if it is selected. + // Only the cell at the index should be selected, as that's what we click. + assert.ok(isCellSelected(wrapper, 'NativeCell', testItem.expectedIndex) === true); + } + }); - // Ensure cell was executed. - verifyHtmlOnCell(wrapper, 'NativeCell', '2', 1); + test('Pressing \'Enter\' on a selected cell, results in focus being set to the code', async () => { + // For some reason we cannot allow setting focus to monaco editor. + // Tests are known to fall over if allowed. + const editor = wrapper + .find(NativeCell) + .at(1) + .find(Editor) + .first(); + (editor.instance() as Editor).giveFocus = () => editor.props().focused!(); + + const update = waitForUpdate(wrapper, NativeEditor, 1); + clickCell(1); + simulateKeyPressOnCell(1, { code: 'Enter', editorInfo: undefined }); + await update; - // The third cell should be selected. - assert.ok(isCellSelected(wrapper, 'NativeCell', 2)); - }); + // The second cell should be selected. + assert.ok(isCellFocused(wrapper, 'NativeCell', 1)); + }); + + test('Pressing \'Escape\' on a focused cell results in the cell being selected', async () => { + // First focus the cell. + let update = waitForUpdate(wrapper, NativeEditor, 1); + clickCell(1); + simulateKeyPressOnCell(1, { code: 'Enter', editorInfo: undefined }); + await update; - test('Pressing \'Ctrl+Enter\' on a selected cell executes the cell and cell selection is not changed', async () => { - const update = waitForUpdate(wrapper, NativeEditor, 7); - clickCell(1); - simulateKeyPressOnCell(1, { code: 'Enter', ctrlKey: true, editorInfo: undefined }); - await update; + // The second cell should be selected. + assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); + assert.equal(isCellFocused(wrapper, 'NativeCell', 1), true); - // Ensure cell was executed. - verifyHtmlOnCell(wrapper, 'NativeCell', '2', 1); + // Now hit escape. + update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(1, { code: 'Escape' }); + await update; - // The first cell should be selected. - assert.ok(isCellSelected(wrapper, 'NativeCell', 1)); - }); + // Confirm it is no longer focused, and it is selected. + assert.equal(isCellSelected(wrapper, 'NativeCell', 1), true); + assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false); + }); - test('Pressing \'Altr+Enter\' on a selected cell adds a new cell below it', async () => { - // Initially 3 cells. - assert.equal(wrapper.find('NativeCell').length, 3); + test('Pressing \'Shift+Enter\' on a selected cell executes the cell and advances to the next cell', async () => { + clickCell(1); + const update = waitForUpdate(wrapper, NativeEditor, 7); + simulateKeyPressOnCell(1, { code: 'Enter', shiftKey: true, editorInfo: undefined }); + await update; + wrapper.update(); - const update = waitForUpdate(wrapper, NativeEditor, 1); - clickCell(1); - simulateKeyPressOnCell(1, { code: 'Enter', altKey: true, editorInfo: undefined }); - await update; + // Ensure cell was executed. + verifyHtmlOnCell(wrapper, 'NativeCell', '2', 1); - // The second cell should be selected. - assert.ok(isCellSelected(wrapper, 'NativeCell', 2)); - // There should be 4 cells. - assert.equal(wrapper.find('NativeCell').length, 4); - }); + // The third cell should be selected. + assert.ok(isCellSelected(wrapper, 'NativeCell', 2)); + }); - test('Pressing \'d\' on a selected cell twice deletes the cell', async () => { - // Initially 3 cells. - assert.equal(wrapper.find('NativeCell').length, 3); + test('Pressing \'Ctrl+Enter\' on a selected cell executes the cell and cell selection is not changed', async () => { + const update = waitForUpdate(wrapper, NativeEditor, 7); + clickCell(1); + simulateKeyPressOnCell(1, { code: 'Enter', ctrlKey: true, editorInfo: undefined }); + await update; - clickCell(2); - simulateKeyPressOnCell(2, { code: 'd' }); - simulateKeyPressOnCell(2, { code: 'd' }); + // Ensure cell was executed. + verifyHtmlOnCell(wrapper, 'NativeCell', '2', 1); - // There should be 2 cells. - assert.equal(wrapper.find('NativeCell').length, 2); - }); + // The first cell should be selected. + assert.ok(isCellSelected(wrapper, 'NativeCell', 1)); + }); - test('Pressing \'a\' on a selected cell adds a cell at the current position', async () => { - // Initially 3 cells. - assert.equal(wrapper.find('NativeCell').length, 3); + test('Pressing \'Altr+Enter\' on a selected cell adds a new cell below it', async () => { + // Initially 3 cells. + assert.equal(wrapper.find('NativeCell').length, 3); - // const secondCell = wrapper.find('NativeCell').at(1); + const update = waitForUpdate(wrapper, NativeEditor, 1); + clickCell(1); + simulateKeyPressOnCell(1, { code: 'Enter', altKey: true, editorInfo: undefined }); + await update; - clickCell(0); - const update = waitForUpdate(wrapper, NativeEditor, 1); - simulateKeyPressOnCell(0, { code: 'a' }); - await update; + // The second cell should be selected. + assert.ok(isCellSelected(wrapper, 'NativeCell', 2)); + // There should be 4 cells. + assert.equal(wrapper.find('NativeCell').length, 4); + }); - // There should be 4 cells. - assert.equal(wrapper.find('NativeCell').length, 4); + test('Pressing \'d\' on a selected cell twice deletes the cell', async () => { + // Initially 3 cells. + assert.equal(wrapper.find('NativeCell').length, 3); - // Verify cell indexes of old items. - verifyCellIndex(wrapper, 'div[id="NotebookImport#0"]', 1); - verifyCellIndex(wrapper, 'div[id="NotebookImport#1"]', 2); - verifyCellIndex(wrapper, 'div[id="NotebookImport#2"]', 3); - }); + clickCell(2); + simulateKeyPressOnCell(2, { code: 'd' }); + simulateKeyPressOnCell(2, { code: 'd' }); - test('Pressing \'b\' on a selected cell adds a cell after the current position', async () => { - // Initially 3 cells. - assert.equal(wrapper.find('NativeCell').length, 3); + // There should be 2 cells. + assert.equal(wrapper.find('NativeCell').length, 2); + }); - clickCell(1); - const update = waitForUpdate(wrapper, NativeEditor, 1); - simulateKeyPressOnCell(1, { code: 'b' }); - await update; + test('Pressing \'a\' on a selected cell adds a cell at the current position', async () => { + // Initially 3 cells. + assert.equal(wrapper.find('NativeCell').length, 3); - // There should be 4 cells. - assert.equal(wrapper.find('NativeCell').length, 4); + // const secondCell = wrapper.find('NativeCell').at(1); - // Verify cell indexes of old items. - verifyCellIndex(wrapper, 'div[id="NotebookImport#0"]', 0); - verifyCellIndex(wrapper, 'div[id="NotebookImport#1"]', 1); - verifyCellIndex(wrapper, 'div[id="NotebookImport#2"]', 3); - }); + clickCell(0); + const update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(0, { code: 'a' }); + await update; - test('Toggle visibility of output', async () => { - // First execute contents of last cell. - let update = waitForUpdate(wrapper, NativeEditor, 7); - clickCell(2); - simulateKeyPressOnCell(2, { code: 'Enter', ctrlKey: true, editorInfo: undefined }); - await update; + // There should be 4 cells. + assert.equal(wrapper.find('NativeCell').length, 4); - // Ensure cell was executed. - verifyHtmlOnCell(wrapper, 'NativeCell', '3', 2); + // Verify cell indexes of old items. + verifyCellIndex(wrapper, 'div[id="NotebookImport#0"]', 1); + verifyCellIndex(wrapper, 'div[id="NotebookImport#1"]', 2); + verifyCellIndex(wrapper, 'div[id="NotebookImport#2"]', 3); + }); - // Hide the output - update = waitForUpdate(wrapper, NativeEditor, 1); - simulateKeyPressOnCell(2, { code: 'o' }); - await update; + test('Pressing \'b\' on a selected cell adds a cell after the current position', async () => { + // Initially 3 cells. + assert.equal(wrapper.find('NativeCell').length, 3); - // Ensure cell output is hidden (looking for cell results will throw an exception). - assert.throws(() => verifyHtmlOnCell(wrapper, 'NativeCell', '3', 2)); + clickCell(1); + const update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(1, { code: 'b' }); + await update; - // Display the output - update = waitForUpdate(wrapper, NativeEditor, 1); - simulateKeyPressOnCell(2, { code: 'o' }); - await update; + // There should be 4 cells. + assert.equal(wrapper.find('NativeCell').length, 4); - // Ensure cell output is visible again. - verifyHtmlOnCell(wrapper, 'NativeCell', '3', 2); - }); + // Verify cell indexes of old items. + verifyCellIndex(wrapper, 'div[id="NotebookImport#0"]', 0); + verifyCellIndex(wrapper, 'div[id="NotebookImport#1"]', 1); + verifyCellIndex(wrapper, 'div[id="NotebookImport#2"]', 3); + }); + + test('Toggle visibility of output', async () => { + // First execute contents of last cell. + let update = waitForUpdate(wrapper, NativeEditor, 7); + clickCell(2); + simulateKeyPressOnCell(2, { code: 'Enter', ctrlKey: true, editorInfo: undefined }); + await update; + + // Ensure cell was executed. + verifyHtmlOnCell(wrapper, 'NativeCell', '3', 2); + + // Hide the output + update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(2, { code: 'o' }); + await update; + + // Ensure cell output is hidden (looking for cell results will throw an exception). + assert.throws(() => verifyHtmlOnCell(wrapper, 'NativeCell', '3', 2)); - test('Toggle line numbers using the \'l\' key', async () => { - clickCell(1); + // Display the output + update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(2, { code: 'o' }); + await update; - const monacoEditorComponent = wrapper.find(NativeCell).at(1).find(MonacoEditor).first(); - const editor = (monacoEditorComponent.instance().state as IMonacoEditorState).editor!; - const oldUpdateOptions = editor.updateOptions.bind(editor); + // Ensure cell output is visible again. + verifyHtmlOnCell(wrapper, 'NativeCell', '3', 2); + }); - let lineNumberSetting: any = ''; - editor.updateOptions = (options: monacoEditor.editor.IEditorConstructionOptions) => { - lineNumberSetting = options.lineNumbers; - oldUpdateOptions(options); - }; + test('Toggle line numbers using the \'l\' key', async () => { + clickCell(1); + + const monacoEditorComponent = wrapper + .find(NativeCell) + .at(1) + .find(MonacoEditor) + .first(); + const editor = (monacoEditorComponent.instance().state as IMonacoEditorState).editor!; + const optionsUpdated = sinon.spy(editor, 'updateOptions'); + + // Display line numbers. + simulateKeyPressOnCell(1, { code: 'l' }); + // Confirm monaco editor got updated with line numbers set to turned on. + assert.equal(optionsUpdated.lastCall.args[0].lineNumbers, 'on'); + + // toggle the display of line numbers. + simulateKeyPressOnCell(1, { code: 'l' }); + // Confirm monaco editor got updated with line numbers set to turned ff. + assert.equal(optionsUpdated.lastCall.args[0].lineNumbers, 'off'); + }); - // Display line numbers. - simulateKeyPressOnCell(1, { code: 'l' }); - assert.equal(lineNumberSetting, 'on'); + test('Toggle markdown and code modes using \'y\' and \'m\' keys', async () => { + clickCell(1); + + // Switch to markdown + simulateKeyPressOnCell(1, { code: 'm' }); + + // Confirm output cell is rendered and monaco editor is not. + assert.equal( + wrapper + .find(NativeCell) + .at(1) + .find(CellOutput).length, + 1 + ); + assert.equal( + wrapper + .find(NativeCell) + .at(1) + .find(MonacoEditor).length, + 0 + ); + + // Switch back to code mode. + // At this moment, there's no cell input element, hence send key strokes to the wrapper. + const wrapperElement = wrapper + .find(NativeCell) + .at(1) + .find('.cell-wrapper') + .first(); + wrapperElement.simulate('keyDown', { key: 'y' }); + + // Confirm output cell is not rendered (remember we don't have any output) and monaco editor is rendered. + assert.equal( + wrapper + .find(NativeCell) + .at(1) + .find(CellOutput).length, + 0 + ); + assert.equal( + wrapper + .find(NativeCell) + .at(1) + .find(MonacoEditor).length, + 1 + ); + }); - // toggle the display of line numbers. - simulateKeyPressOnCell(1, { code: 'l' }); - assert.equal(lineNumberSetting, 'off'); + test('Test undo using the key \'z\'', async () => { + clickCell(0); + + // Add, then undo, keep doing at least 3 times and confirm it works as expected. + for (let i = 0; i < 3; i += 1) { + // Add a new cell + let update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(0, { code: 'a' }); + await update; + + // There should be 4 cells and first cell is selected & nothing focused. + assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true); + assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); + assert.equal(isCellFocused(wrapper, 'NativeCell', 0), false); + assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false); + assert.equal(wrapper.find('NativeCell').length, 4); + + // Press 'z' to undo. + update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(0, { code: 'z' }); + await update; + + // There should be 3 cells and first cell is selected & nothing focused. + assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true); + assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); + assert.equal(wrapper.find('NativeCell').length, 3); + } + }); }); - test('Toggle markdown and code modes using \'y\' and \'m\' keys', async () => { - clickCell(1); + suite('Auto Save', () => { + let controller: NativeEditorStateController; + let windowStateChangeHandlers: ((e: WindowState) => any)[] = []; + let handleMessageSpy: sinon.SinonSpy<[string, any?], boolean>; + setup(async function() { + handleMessageSpy = sinon.spy(NativeEditorStateController.prototype, 'handleMessage'); + initIoc(); - // Switch to markdown - simulateKeyPressOnCell(1, { code: 'm' }); + windowStateChangeHandlers = []; + // Keep track of all handlers for the onDidChangeWindowState event. + ioc.applicationShell.setup(app => app.onDidChangeWindowState(TypeMoq.It.isAny())).callback(cb => windowStateChangeHandlers.push(cb)); - // Confirm output cell is rendered and monaco editor is not. - assert.equal(wrapper.find(NativeCell).at(1).find(CellOutput).length, 1); - assert.equal(wrapper.find(NativeCell).at(1).find(MonacoEditor).length, 0); + // tslint:disable-next-line: no-invalid-this + await setupFunction.call(this); - // Switch back to code mode. - // At this moment, there's no cell input element, hence send key strokes to the wrapper. - const wrapperElement = wrapper.find(NativeCell).at(1).find('.cell-wrapper').first(); - wrapperElement.simulate('keyDown', {key: 'y'}); + controller = (wrapper + .find(NativeEditor) + .first() + .instance() as NativeEditor).stateController; + }); + teardown(() => sinon.restore()); + + /** + * Wait for a particular message to be received by the editor component. + * If message isn't reiceived within a time out, then reject with a timeout error message. + * + * @param {string} message + * @param {number} timeout + * @returns {Promise} + */ + async function waitForMessageReceivedEditorComponent(message: string, timeout: number = 5000): Promise { + const errorMessage = `Timeout waiting for message ${message}`; + await waitForCondition(async () => handleMessageSpy.calledWith(message, sinon.match.any), timeout, errorMessage); + } - // Confirm output cell is not rendered (remember we don't have any output) and monaco editor is rendered. - assert.equal(wrapper.find(NativeCell).at(1).find(CellOutput).length, 0); - assert.equal(wrapper.find(NativeCell).at(1).find(MonacoEditor).length, 1); - }); + /** + * Wait for notebook to be marked as dirty (within a timeout of 5s). + * + * @param {boolean} [dirty=true] + * @returns {Promise} + */ + async function waitForNotebookToBeDirty(): Promise { + // Wait for the notebook to be marked as dirty (the NotebookDirty message will be sent). + await waitForMessageReceivedEditorComponent(InteractiveWindowMessages.NotebookDirty, 5_000); + // Wait for the state to get updated. + await waitForCondition(async () => controller.getState().dirty === true, 1_000, `Timeout waiting for dirty state to get updated to true`); + } - test('Test undo using the key \'z\'', async () => { - clickCell(0); + /** + * Wait for notebook to be marked as clean (within a timeout of 5s). + * + * @param {boolean} [dirty=true] + * @returns {Promise} + */ + async function waitForNotebookToBeClean(): Promise { + // Wait for the notebook to be marked as dirty (the NotebookDirty message will be sent). + await waitForMessageReceivedEditorComponent(InteractiveWindowMessages.NotebookDirty, 5_000); + + // Wait for the state to get updated. + await waitForCondition(async () => controller.getState().dirty === false, 1_000, `Timeout waiting for dirty state to get updated to false`); + } - // Add, then undo, keep doing at least 3 times and confirm it works as expected. - for (let i = 0; i < 3; i += 1){ - // Add a new cell - let update = waitForUpdate(wrapper, NativeEditor, 1); + /** + * Make some kind of a change to the notebook. + * + * @param {number} cellIndex + */ + async function modifyNotebook() { + // (Add a cell into the UI and wait for it to render) + clickCell(0); + const update = waitForUpdate(wrapper, NativeEditor, 2); simulateKeyPressOnCell(0, { code: 'a' }); await update; + } - // There should be 4 cells and first cell is selected & nothing focused. - assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true); - assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); - assert.equal(isCellFocused(wrapper, 'NativeCell', 0), false); - assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false); - assert.equal(wrapper.find('NativeCell').length, 4); + test('Auto save notebook every 1s', async () => { + // Configure notebook to save automatically ever 1s. + when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('afterDelay'); + when(ioc.mockedWorkspaceConfig.get('autoSaveDelay', anything())).thenReturn(1_000); + ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + + /** + * Make some changes to a cell of a notebook, then verify the notebook is auto saved. + * + * @param {number} cellIndex + */ + async function makeChangesAndConfirmFileIsUpdated() { + const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); + + await modifyNotebook(); + await waitForNotebookToBeDirty(); + + // At this point a message should be sent to extension asking it to save. + // After the save, the extension should send a message to react letting it know that it was saved successfully. + + await waitForNotebookToBeClean(); + // Confirm file has been updated as well. + const newFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); + assert.notEqual(newFileContents, notebookFileContents); + } - // Press 'z' to undo. - update = waitForUpdate(wrapper, NativeEditor, 1); - simulateKeyPressOnCell(0, { code: 'z' }); - await update; + // Make changes & validate (try a couple of times). + await makeChangesAndConfirmFileIsUpdated(); + await makeChangesAndConfirmFileIsUpdated(); + await makeChangesAndConfirmFileIsUpdated(); + }); - // There should be 3 cells and first cell is selected & nothing focused. - assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true); - assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false); - assert.equal(wrapper.find('NativeCell').length, 3); + test('Should not auto save notebook, ever', async () => { + const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); + + // Configure notebook to to never save. + when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('off'); + when(ioc.mockedWorkspaceConfig.get('autoSaveDelay', anything())).thenReturn(1000); + // Update the settings and wait for the component to receive it and process it. + const promise = waitForMessageReceivedEditorComponent(InteractiveWindowMessages.UpdateSettings, 1_000); + ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + await promise; + + await modifyNotebook(); + await waitForNotebookToBeDirty(); + + // Now that the notebook is dirty, change the active editor. + const docManager = ioc.get(IDocumentManager) as MockDocumentManager; + docManager.didChangeEmitter.fire(); + // Also, send notification about changes to window state. + windowStateChangeHandlers.forEach(item => item({ focused: false })); + windowStateChangeHandlers.forEach(item => item({ focused: true })); + + // Confirm the message is not clean, trying to wait for it to get saved will timeout (i.e. rejected). + await expect(waitForNotebookToBeClean()).to.eventually.be.rejected; + // Confirm file has not been updated as well. + assert.equal(await fs.readFile(notebookFile.filePath, 'utf8'), notebookFileContents); + }); + + async function testAutoSavingWhenEditorFocusChanges(newEditor: TextEditor | undefined) { + const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); + + await modifyNotebook(); + await waitForNotebookToBeDirty(); + + // Configure notebook to save when active editor changes. + when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onFocusChange'); + ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + + // Now that the notebook is dirty, change the active editor. + const docManager = ioc.get(IDocumentManager) as MockDocumentManager; + docManager.didChangeEmitter.fire(newEditor); + + // At this point a message should be sent to extension asking it to save. + // After the save, the extension should send a message to react letting it know that it was saved successfully. + + await waitForNotebookToBeClean(); + // Confirm file has been updated as well. + assert.notEqual(await fs.readFile(notebookFile.filePath, 'utf8'), notebookFileContents); } + + test('Auto save notebook when focus changes from active editor to none', () => testAutoSavingWhenEditorFocusChanges(undefined)); + + test('Auto save notebook when focus changes from active editor to something else', () => + testAutoSavingWhenEditorFocusChanges(TypeMoq.Mock.ofType().object)); + + test('Should not auto save notebook when active editor changes', async () => { + const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); + + await modifyNotebook(); + await waitForNotebookToBeDirty(); + + // Configure notebook to save when window state changes. + when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onWindowChange'); + ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + + // Now that the notebook is dirty, change the active editor. + // This should not trigger a save of notebook (as its configured to save only when window state changes). + const docManager = ioc.get(IDocumentManager) as MockDocumentManager; + docManager.didChangeEmitter.fire(); + + // Confirm the message is not clean, trying to wait for it to get saved will timeout (i.e. rejected). + await expect(waitForNotebookToBeClean()).to.eventually.be.rejected; + // Confirm file has not been updated as well. + assert.equal(await fs.readFile(notebookFile.filePath, 'utf8'), notebookFileContents); + }); + + async function testAutoSavingWithChangesToWindowState(focused: boolean) { + const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); + + await modifyNotebook(); + await waitForNotebookToBeDirty(); + + // Configure notebook to save when active editor changes. + when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onWindowChange'); + ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + + // Now that the notebook is dirty, send notification about changes to window state. + windowStateChangeHandlers.forEach(item => item({ focused })); + + // At this point a message should be sent to extension asking it to save. + // After the save, the extension should send a message to react letting it know that it was saved successfully. + + await waitForNotebookToBeClean(); + // Confirm file has been updated as well. + assert.notEqual(await fs.readFile(notebookFile.filePath, 'utf8'), notebookFileContents); + } + + test('Auto save notebook when window state changes to being not focused', async () => testAutoSavingWithChangesToWindowState(false)); + test('Auto save notebook when window state changes to being focused', async () => testAutoSavingWithChangesToWindowState(true)); + + test('Should not auto save notebook when window state changes', async () => { + const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); + + await modifyNotebook(); + await waitForNotebookToBeDirty(); + + // Configure notebook to save when active editor changes. + when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onFocusChange'); + ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + + // Now that the notebook is dirty, change window state. + // This should not trigger a save of notebook (as its configured to save only when focus is changed). + windowStateChangeHandlers.forEach(item => item({ focused: false })); + windowStateChangeHandlers.forEach(item => item({ focused: true })); + + // Confirm the message is not clean, trying to wait for it to get saved will timeout (i.e. rejected). + await expect(waitForNotebookToBeClean()).to.eventually.be.rejected; + // Confirm file has not been updated as well. + assert.equal(await fs.readFile(notebookFile.filePath, 'utf8'), notebookFileContents); + }); }); }); }); diff --git a/src/test/datascience/nativeEditorTestHelpers.tsx b/src/test/datascience/nativeEditorTestHelpers.tsx index d0421752c580..0ddb5ee20e18 100644 --- a/src/test/datascience/nativeEditorTestHelpers.tsx +++ b/src/test/datascience/nativeEditorTestHelpers.tsx @@ -31,9 +31,9 @@ export async function createNewEditor(ioc: DataScienceIocContainer): Promise { +export async function openEditor(ioc: DataScienceIocContainer, contents: string, filePath: string = '/usr/home/test.ipynb'): Promise { const loaded = waitForMessage(ioc, InteractiveWindowMessages.LoadAllCellsComplete); - const uri = Uri.parse('file:////usr/home/test.ipynb'); + const uri = Uri.file(filePath); const result = await getOrCreateNativeEditor(ioc, uri, contents); await loaded; return result; diff --git a/src/test/datascience/reactHelpers.ts b/src/test/datascience/reactHelpers.ts index d2b96ce4419f..47cda6423320 100644 --- a/src/test/datascience/reactHelpers.ts +++ b/src/test/datascience/reactHelpers.ts @@ -359,7 +359,11 @@ export function setUpDomEnvironment() { collapseCellInputCodeByDefault: true, allowInput: true, showJupyterVariableExplorer: true, - variableExplorerExclude: 'module;function;builtin_function_or_method' + variableExplorerExclude: 'module;function;builtin_function_or_method', + files : { + autoSave: 'off', + autoSaveDelay: 1000 + } }; }; diff --git a/src/test/datascience/testHelpers.tsx b/src/test/datascience/testHelpers.tsx index c13b66d11d4a..e727e268b833 100644 --- a/src/test/datascience/testHelpers.tsx +++ b/src/test/datascience/testHelpers.tsx @@ -499,3 +499,32 @@ export function escapePath(p: string) { export function srcDirectory() { return path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience'); } + +// /** +// * Wait for a condition to be fulfilled within a timeout. +// * +// * @param {string} message +// * @param {number} timeout +// * @returns {Promise} +// */ +// export function waitForCondition(predicate: Function, timeout: number, errorMessage?: string): Promise { +// return new Promise((resolve, reject) => { +// // Reject after timeout exceeds. +// const timeoutTimer = setTimeout(() => { +// // tslint:disable-next-line: no-use-before-declare +// clearInterval(timer as any); +// reject(new Error(errorMessage || 'Timeout waiting for condition')); +// }, timeout); +// // Look for the message. +// const timer = setInterval(() => { +// try { +// predicate(); +// clearInterval(timer as any); +// clearTimeout(timeoutTimer); +// resolve(); +// } catch { +// noop(); +// } +// }, 10); +// }); +// } From f37a3af8fcdb9dbc86d6be40fdf9cd743671fa20 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 09:12:02 -0700 Subject: [PATCH 10/23] Revert unnecessary formatting changes --- .../nativeEditor.functional.test.tsx | 383 ++++++++---------- 1 file changed, 168 insertions(+), 215 deletions(-) diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 549a7405e002..65d8a4624c11 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -71,16 +71,13 @@ suite('DataScience Native Editor', () => { ioc.registerDataScienceTypes(); const appShell = TypeMoq.Mock.ofType(); - appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns(_e => Promise.resolve('')); + appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns((_e) => Promise.resolve('')); appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve('')); - appShell - .setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns((_a1: string, a2: string, _a3: string) => Promise.resolve(a2)); - appShell - .setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns((_a1: string, _a2: any, _a3: string, a4: string) => Promise.resolve(a4)); + appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((_a1: string, a2: string, _a3: string) => Promise.resolve(a2)); + appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((_a1: string, _a2: any, _a3: string, a4: string) => Promise.resolve(a4)); appShell.setup(a => a.showSaveDialog(TypeMoq.It.isAny())).returns(() => Promise.resolve(Uri.file('foo.ipynb'))); ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); + }); teardown(async () => { @@ -102,37 +99,29 @@ suite('DataScience Native Editor', () => { // asyncDump(); // }); - runMountedTest( - 'Simple text', - async wrapper => { - // Create an editor so something is listening to messages - await createNewEditor(ioc); + runMountedTest('Simple text', async (wrapper) => { + // Create an editor so something is listening to messages + await createNewEditor(ioc); - // Add a cell into the UI and wait for it to render - await addCell(wrapper, 'a=1\na'); + // Add a cell into the UI and wait for it to render + await addCell(wrapper, 'a=1\na'); - verifyHtmlOnCell(wrapper, 'NativeCell', '1', CellPosition.Last); - }, - () => { - return ioc; - } - ); + verifyHtmlOnCell(wrapper, 'NativeCell', '1', CellPosition.Last); + }, () => { return ioc; }); - runMountedTest( - 'Mime Types', - async wrapper => { - // Create an editor so something is listening to messages - await createNewEditor(ioc); + runMountedTest('Mime Types', async (wrapper) => { + // Create an editor so something is listening to messages + await createNewEditor(ioc); - const badPanda = `import pandas as pd + const badPanda = `import pandas as pd df = pd.read("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") df.head()`; - const goodPanda = `import pandas as pd + const goodPanda = `import pandas as pd df = pd.read_csv("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") df.head()`; - const matPlotLib = 'import matplotlib.pyplot as plt\r\nimport numpy as np\r\nx = np.linspace(0,20,100)\r\nplt.plot(x, np.sin(x))\r\nplt.show()'; - const matPlotLibResults = 'img'; - const spinningCursor = `import sys + const matPlotLib = 'import matplotlib.pyplot as plt\r\nimport numpy as np\r\nx = np.linspace(0,20,100)\r\nplt.plot(x, np.sin(x))\r\nplt.show()'; + const matPlotLibResults = 'img'; + const spinningCursor = `import sys import time def spinning_cursor(): while True: @@ -145,201 +134,165 @@ suite('DataScience Native Editor', () => { time.sleep(0.1) sys.stdout.write('\\r')`; - addMockData(ioc, badPanda, `pandas has no attribute 'read'`, 'text/html', 'error'); - addMockData(ioc, goodPanda, `A table`, 'text/html'); - addMockData(ioc, matPlotLib, matPlotLibResults, 'text/html'); - const cursors = ['|', '/', '-', '\\']; - let cursorPos = 0; - let loops = 3; - addContinuousMockData(ioc, spinningCursor, async _c => { - const result = `${cursors[cursorPos]}\r`; - cursorPos += 1; - if (cursorPos >= cursors.length) { - cursorPos = 0; - loops -= 1; - } - return Promise.resolve({ result: result, haveMore: loops > 0 }); - }); - - await addCell(wrapper, badPanda, true); - verifyHtmlOnCell(wrapper, 'NativeCell', `has no attribute 'read'`, CellPosition.Last); - - await addCell(wrapper, goodPanda, true); - verifyHtmlOnCell(wrapper, 'NativeCell', ``, CellPosition.Last); - - await addCell(wrapper, matPlotLib, true, 6); - verifyHtmlOnCell(wrapper, 'NativeCell', matPlotLibResults, CellPosition.Last); + addMockData(ioc, badPanda, `pandas has no attribute 'read'`, 'text/html', 'error'); + addMockData(ioc, goodPanda, `A table`, 'text/html'); + addMockData(ioc, matPlotLib, matPlotLibResults, 'text/html'); + const cursors = ['|', '/', '-', '\\']; + let cursorPos = 0; + let loops = 3; + addContinuousMockData(ioc, spinningCursor, async (_c) => { + const result = `${cursors[cursorPos]}\r`; + cursorPos += 1; + if (cursorPos >= cursors.length) { + cursorPos = 0; + loops -= 1; + } + return Promise.resolve({ result: result, haveMore: loops > 0 }); + }); - await addCell(wrapper, spinningCursor, true, 4 + (ioc.mockJupyter ? cursors.length * 3 : 50)); - verifyHtmlOnCell(wrapper, 'NativeCell', '
', CellPosition.Last); - }, - () => { - return ioc; - } - ); + await addCell(wrapper, badPanda, true); + verifyHtmlOnCell(wrapper, 'NativeCell', `has no attribute 'read'`, CellPosition.Last); + + await addCell(wrapper, goodPanda, true); + verifyHtmlOnCell(wrapper, 'NativeCell', ``, CellPosition.Last); + + await addCell(wrapper, matPlotLib, true, 6); + verifyHtmlOnCell(wrapper, 'NativeCell', matPlotLibResults, CellPosition.Last); + + await addCell(wrapper, spinningCursor, true, 4 + (ioc.mockJupyter ? (cursors.length * 3) : 50)); + verifyHtmlOnCell(wrapper, 'NativeCell', '
', CellPosition.Last); + }, () => { return ioc; }); + + runMountedTest('Click buttons', async (wrapper) => { + // Goto source should cause the visible editor to be picked as long as its filename matches + const showedEditor = createDeferred(); + const textEditors: TextEditor[] = []; + const docManager = TypeMoq.Mock.ofType(); + const visibleEditor = TypeMoq.Mock.ofType(); + const dummyDocument = TypeMoq.Mock.ofType(); + dummyDocument.setup(d => d.fileName).returns(() => 'foo.py'); + visibleEditor.setup(v => v.show()).returns(() => showedEditor.resolve()); + visibleEditor.setup(v => v.revealRange(TypeMoq.It.isAny())).returns(noop); + visibleEditor.setup(v => v.document).returns(() => dummyDocument.object); + textEditors.push(visibleEditor.object); + docManager.setup(a => a.visibleTextEditors).returns(() => textEditors); + ioc.serviceManager.rebindInstance(IDocumentManager, docManager.object); + // Create an editor so something is listening to messages + await createNewEditor(ioc); - runMountedTest( - 'Click buttons', - async wrapper => { - // Goto source should cause the visible editor to be picked as long as its filename matches - const showedEditor = createDeferred(); - const textEditors: TextEditor[] = []; - const docManager = TypeMoq.Mock.ofType(); - const visibleEditor = TypeMoq.Mock.ofType(); - const dummyDocument = TypeMoq.Mock.ofType(); - dummyDocument.setup(d => d.fileName).returns(() => 'foo.py'); - visibleEditor.setup(v => v.show()).returns(() => showedEditor.resolve()); - visibleEditor.setup(v => v.revealRange(TypeMoq.It.isAny())).returns(noop); - visibleEditor.setup(v => v.document).returns(() => dummyDocument.object); - textEditors.push(visibleEditor.object); - docManager.setup(a => a.visibleTextEditors).returns(() => textEditors); - ioc.serviceManager.rebindInstance(IDocumentManager, docManager.object); - // Create an editor so something is listening to messages - await createNewEditor(ioc); - - // Get a cell into the list - await addCell(wrapper, 'a=1\na'); - - // find the buttons on the cell itself - let cell = getLastOutputCell(wrapper, 'NativeCell'); - let ImageButtons = cell.find(ImageButton); - assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); - let deleteButton = ImageButtons.at(6); - - // Make sure delete works - let afterDelete = await getNativeCellResults(wrapper, 1, async () => { - deleteButton.simulate('click'); - return Promise.resolve(); - }); - assert.equal(afterDelete.length, 1, `Delete should remove a cell`); - - // Secondary delete should NOT delete the cell as there should ALWAYS be at - // least one cell in the file. - cell = getLastOutputCell(wrapper, 'NativeCell'); - ImageButtons = cell.find(ImageButton); - assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); - deleteButton = ImageButtons.at(6); - - afterDelete = await getNativeCellResults(wrapper, 1, async () => { - deleteButton.simulate('click'); - return Promise.resolve(); - }); - assert.equal(afterDelete.length, 1, `Delete should NOT remove the last cell`); - }, - () => { - return ioc; - } - ); + // Get a cell into the list + await addCell(wrapper, 'a=1\na'); - runMountedTest( - 'Export', - async wrapper => { - // Export should cause the export dialog to come up. Remap appshell so we can check - const dummyDisposable = { - dispose: () => { - return; - } - }; - let exportCalled = false; - const appShell = TypeMoq.Mock.ofType(); - appShell - .setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())) - .returns(e => { - throw e; - }); - appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve('')); - appShell - .setup(a => a.showSaveDialog(TypeMoq.It.isAny())) - .returns(() => { - exportCalled = true; - return Promise.resolve(undefined); - }); - appShell.setup(a => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable); - ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); - - // Make sure to create the interactive window after the rebind or it gets the wrong application shell. - await createNewEditor(ioc); - await addCell(wrapper, 'a=1\na'); - - // Export should cause exportCalled to change to true - const exportButton = findButton(wrapper, NativeEditor, 6); - await waitForMessageResponse(ioc, () => exportButton!.simulate('click')); - assert.equal(exportCalled, true, 'Export should have been called'); - }, - () => { - return ioc; - } - ); + // find the buttons on the cell itself + let cell = getLastOutputCell(wrapper, 'NativeCell'); + let ImageButtons = cell.find(ImageButton); + assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); + let deleteButton = ImageButtons.at(6); - runMountedTest( - 'RunAllCells', - async wrapper => { - addMockData(ioc, 'b=2\nb', 2); - addMockData(ioc, 'c=3\nc', 3); + // Make sure delete works + let afterDelete = await getNativeCellResults(wrapper, 1, async () => { + deleteButton.simulate('click'); + return Promise.resolve(); + }); + assert.equal(afterDelete.length, 1, `Delete should remove a cell`); + + // Secondary delete should NOT delete the cell as there should ALWAYS be at + // least one cell in the file. + cell = getLastOutputCell(wrapper, 'NativeCell'); + ImageButtons = cell.find(ImageButton); + assert.equal(ImageButtons.length, 7, 'Cell buttons not found'); + deleteButton = ImageButtons.at(6); + + afterDelete = await getNativeCellResults(wrapper, 1, async () => { + deleteButton.simulate('click'); + return Promise.resolve(); + }); + assert.equal(afterDelete.length, 1, `Delete should NOT remove the last cell`); + }, () => { return ioc; }); + + runMountedTest('Export', async (wrapper) => { + // Export should cause the export dialog to come up. Remap appshell so we can check + const dummyDisposable = { + dispose: () => { return; } + }; + let exportCalled = false; + const appShell = TypeMoq.Mock.ofType(); + appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns((e) => { throw e; }); + appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve('')); + appShell.setup(a => a.showSaveDialog(TypeMoq.It.isAny())).returns(() => { + exportCalled = true; + return Promise.resolve(undefined); + }); + appShell.setup(a => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable); + ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); - const baseFile = [ - { id: 'NotebookImport#0', data: { source: 'a=1\na' } }, - { id: 'NotebookImport#1', data: { source: 'b=2\nb' } }, - { id: 'NotebookImport#2', data: { source: 'c=3\nc' } } - ]; - const runAllCells = baseFile.map(cell => { - return createFileCell(cell, cell.data); - }); - const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); - await openEditor(ioc, JSON.stringify(notebook)); + // Make sure to create the interactive window after the rebind or it gets the wrong application shell. + await createNewEditor(ioc); + await addCell(wrapper, 'a=1\na'); + + // Export should cause exportCalled to change to true + const exportButton = findButton(wrapper, NativeEditor, 6); + await waitForMessageResponse(ioc, () => exportButton!.simulate('click')); + assert.equal(exportCalled, true, 'Export should have been called'); + }, () => { return ioc; }); + + runMountedTest('RunAllCells', async (wrapper) => { + addMockData(ioc, 'b=2\nb', 2); + addMockData(ioc, 'c=3\nc', 3); + + const baseFile = [ {id: 'NotebookImport#0', data: {source: 'a=1\na'}}, + {id: 'NotebookImport#1', data: {source: 'b=2\nb'}}, + {id: 'NotebookImport#2', data: {source: 'c=3\nc'}} ]; + const runAllCells = baseFile.map(cell => { + return createFileCell(cell, cell.data); + }); + const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); + await openEditor(ioc, JSON.stringify(notebook)); - // Export should cause exportCalled to change to true - const runAllButton = findButton(wrapper, NativeEditor, 3); - await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); + // Export should cause exportCalled to change to true + const runAllButton = findButton(wrapper, NativeEditor, 3); + await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); - await waitForUpdate(wrapper, NativeEditor, 16); + await waitForUpdate(wrapper, NativeEditor, 16); - verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); - verifyHtmlOnCell(wrapper, 'NativeCell', `2`, 1); - verifyHtmlOnCell(wrapper, 'NativeCell', `3`, 2); - }, - () => { - return ioc; - } - ); + verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); + verifyHtmlOnCell(wrapper, 'NativeCell', `2`, 1); + verifyHtmlOnCell(wrapper, 'NativeCell', `3`, 2); + }, () => { return ioc; }); - runMountedTest( - 'Startup and shutdown', - async wrapper => { - addMockData(ioc, 'b=2\nb', 2); - addMockData(ioc, 'c=3\nc', 3); + runMountedTest('Startup and shutdown', async (wrapper) => { + addMockData(ioc, 'b=2\nb', 2); + addMockData(ioc, 'c=3\nc', 3); - const baseFile = [ - { id: 'NotebookImport#0', data: { source: 'a=1\na' } }, - { id: 'NotebookImport#1', data: { source: 'b=2\nb' } }, - { id: 'NotebookImport#2', data: { source: 'c=3\nc' } } - ]; - const runAllCells = baseFile.map(cell => { - return createFileCell(cell, cell.data); - }); - const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); - let editor = await openEditor(ioc, JSON.stringify(notebook)); - - // Run everything - let runAllButton = findButton(wrapper, NativeEditor, 3); - await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); - await waitForUpdate(wrapper, NativeEditor, 16); - - // Close editor. Should still have the server up - await closeNotebook(editor, wrapper); - const jupyterExecution = ioc.serviceManager.get(IJupyterExecution); - const editorProvider = ioc.serviceManager.get(INotebookEditorProvider); - const server = await jupyterExecution.getServer(await editorProvider.getNotebookOptions()); - assert.ok(server, 'Server was destroyed on notebook shutdown'); - - // Reopen, and rerun - editor = await openEditor(ioc, JSON.stringify(notebook)); - runAllButton = findButton(wrapper, NativeEditor, 3); - await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); - await waitForUpdate(wrapper, NativeEditor, 15); - verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); - }, - () => { + const baseFile = [ {id: 'NotebookImport#0', data: {source: 'a=1\na'}}, + {id: 'NotebookImport#1', data: {source: 'b=2\nb'}}, + {id: 'NotebookImport#2', data: {source: 'c=3\nc'}} ]; + const runAllCells = baseFile.map(cell => { + return createFileCell(cell, cell.data); + }); + const notebook = await ioc.get(INotebookExporter).translateToNotebook(runAllCells, undefined); + let editor = await openEditor(ioc, JSON.stringify(notebook)); + + // Run everything + let runAllButton = findButton(wrapper, NativeEditor, 3); + await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); + await waitForUpdate(wrapper, NativeEditor, 16); + + // Close editor. Should still have the server up + await closeNotebook(editor, wrapper); + const jupyterExecution = ioc.serviceManager.get(IJupyterExecution); + const editorProvider = ioc.serviceManager.get(INotebookEditorProvider); + const server = await jupyterExecution.getServer(await editorProvider.getNotebookOptions()); + assert.ok(server, 'Server was destroyed on notebook shutdown'); + + // Reopen, and rerun + editor = await openEditor(ioc, JSON.stringify(notebook)); + runAllButton = findButton(wrapper, NativeEditor, 3); + await waitForMessageResponse(ioc, () => runAllButton!.simulate('click')); + await waitForUpdate(wrapper, NativeEditor, 15); + verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 0); + }, + () => { // Disable the warning displayed by nodejs when there are too many listeners. EventEmitter.defaultMaxListeners = 15; return ioc; From 83dc1f7378fb70cdfb33cdd4c27dd9b5397aab19 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 09:13:36 -0700 Subject: [PATCH 11/23] Added news entry --- news/1 Enhancements/7831.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1 Enhancements/7831.md diff --git a/news/1 Enhancements/7831.md b/news/1 Enhancements/7831.md new file mode 100644 index 000000000000..cd8ca8a6c628 --- /dev/null +++ b/news/1 Enhancements/7831.md @@ -0,0 +1 @@ +Added ability to auto-save chagnes made to the notebook. From 355b8276866feaf35754f4c55b9470c89e6f2021 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 09:29:15 -0700 Subject: [PATCH 12/23] remove try catch --- src/test/common.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/test/common.ts b/src/test/common.ts index 8cf22d833fde..83807480abf7 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -18,7 +18,7 @@ import { IServiceContainer, IServiceManager } from '../client/ioc/types'; import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_MULTI_ROOT_TEST, IS_PERF_TEST, IS_SMOKE_TEST } from './constants'; -import { noop, sleep } from './core'; +import { noop } from './core'; const StreamZip = require('node-stream-zip'); @@ -428,16 +428,12 @@ export async function waitForCondition(condition: () => Promise, timeou reject(new Error(errorMessage)); }, timeoutMs); const timer = setInterval(async () => { - try { - if (!await condition()){ - throw new Error('failed'); - } - clearTimeout(timeout); - clearTimeout(timer); - resolve(); - } catch { - noop(); + if (!await condition().catch(() => false)) { + return; } + clearTimeout(timeout); + clearTimeout(timer); + resolve(); }, 10); }); } From cf22f3c884cbce9a3783e6dbcd4c6c3e3e98d4eb Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 09:51:04 -0700 Subject: [PATCH 13/23] Fix sonar cloud issue --- src/datascience-ui/native-editor/autoSaveService.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts index 76996edab282..f4f192a5fd44 100644 --- a/src/datascience-ui/native-editor/autoSaveService.ts +++ b/src/datascience-ui/native-editor/autoSaveService.ts @@ -11,7 +11,7 @@ import { getSettings } from '../react-common/settingsReactSide'; import { NativeEditorStateController } from './nativeEditorStateController'; export class AutoSaveService implements IDisposable, IMessageHandler { - private timeout?: NodeJS.Timer | number; + private timeout?: ReturnType; private settings?: FileSettings; // private fileSettings: typeof constructor(private readonly controller: NativeEditorStateController) { @@ -69,8 +69,7 @@ export class AutoSaveService implements IDisposable, IMessageHandler { } private clearTimeout() { if (this.timeout) { - // tslint:disable-next-line: no-any - clearTimeout(this.timeout as any); + clearTimeout(this.timeout); this.timeout = undefined; } } From 639347a8984f34841493efcd4c5d4bbbf6041444 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 10:00:55 -0700 Subject: [PATCH 14/23] Skip sonar cloud message --- src/datascience-ui/react-common/postOffice.ts | 2 +- src/datascience-ui/react-common/settingsReactSide.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/datascience-ui/react-common/postOffice.ts b/src/datascience-ui/react-common/postOffice.ts index e71953ed68e9..afb52319e863 100644 --- a/src/datascience-ui/react-common/postOffice.ts +++ b/src/datascience-ui/react-common/postOffice.ts @@ -67,7 +67,7 @@ export class PostOffice implements IDisposable { // Only do this once as it crashes if we ask more than once // tslint:disable-next-line:no-typeof-undefined if (!this.vscodeApi && typeof acquireVsCodeApi !== 'undefined') { - this.vscodeApi = acquireVsCodeApi(); + this.vscodeApi = acquireVsCodeApi(); // NOSONAR } if (!this.registered) { this.registered = true; diff --git a/src/datascience-ui/react-common/settingsReactSide.ts b/src/datascience-ui/react-common/settingsReactSide.ts index 6fd517a7a569..8a94987c76c4 100644 --- a/src/datascience-ui/react-common/settingsReactSide.ts +++ b/src/datascience-ui/react-common/settingsReactSide.ts @@ -28,7 +28,7 @@ export function updateSettings(jsonSettingsString: string) { function load() { // tslint:disable-next-line:no-typeof-undefined if (typeof getInitialSettings !== 'undefined') { - loadedSettings = getInitialSettings(); + loadedSettings = getInitialSettings(); // NOSONAR } else { // Default settings for tests loadedSettings = { From 2815fa0344cab0509cdb70683f33405fd6d42eed Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 15:03:44 -0700 Subject: [PATCH 15/23] Code review changes --- .../interactiveWindowTypes.ts | 4 +- .../interactive-ipynb/autoSaveService.ts | 123 +++++++++++++++--- .../interactive-ipynb/nativeEditor.ts | 78 ++++++----- src/client/datascience/types.ts | 1 + .../native-editor/autoSaveService.ts | 82 +----------- 5 files changed, 158 insertions(+), 130 deletions(-) diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts index 2e03977bc7a1..3c438ca51a56 100644 --- a/src/client/datascience/interactive-common/interactiveWindowTypes.ts +++ b/src/client/datascience/interactive-common/interactiveWindowTypes.ts @@ -27,8 +27,8 @@ export namespace InteractiveWindowMessages { export const Interrupt = 'interrupt'; export const SubmitNewCell = 'submit_new_cell'; export const UpdateSettings = SharedMessages.UpdateSettings; - export const WindowStateChanged = 'WindowStateChanged'; - export const ActiveTextEditorChanged = 'ActiveTextEditorChanged'; + // Message sent to React component from extension asking it to save the notebook. + export const DoSave = 'DoSave'; export const SendInfo = 'send_info'; export const Started = SharedMessages.Started; export const AddedSysInfo = 'added_sys_info'; diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index 106aceb16877..7f0e6c781a50 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -4,21 +4,22 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { Event, EventEmitter, TextEditor, WindowState } from 'vscode'; -import { IApplicationShell, IDocumentManager } from '../../common/application/types'; +import { ConfigurationChangeEvent, Event, EventEmitter, TextEditor, Uri } from 'vscode'; +import { IApplicationShell, IDocumentManager, IWorkspaceService } from '../../common/application/types'; import '../../common/extensions'; +import { traceError } from '../../common/logger'; +import { IFileSystem } from '../../common/platform/types'; import { IDisposable } from '../../common/types'; -import { noop } from '../../common/utils/misc'; -import { InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes'; -import { IInteractiveWindowListener } from '../types'; +import { INotebookIdentity, InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes'; +import { FileSettings, IInteractiveWindowListener, INotebookEditor, INotebookEditorProvider } from '../types'; // tslint:disable: no-any /** - * Sends notifications to Notebooks related to auto saving of files. - * If window state changes, then notify notebooks. - * If active editor changes, then notify notebooks. - * This information is necessary for the implementation of automatically saving notebooks. + * Sends notifications to Notebooks to save the notebook. + * Based on auto save settings, this class will regularly check for changes and send a save requet. + * If window state changes or active editor changes, then notify notebooks (if auto save is configured to do so). + * Monitor save and modified events on editor to determine its current dirty state. * * @export * @class AutoSaveService @@ -28,7 +29,17 @@ import { IInteractiveWindowListener } from '../types'; export class AutoSaveService implements IInteractiveWindowListener { private postEmitter: EventEmitter<{ message: string; payload: any }> = new EventEmitter<{ message: string; payload: any }>(); private disposables: IDisposable[] = []; - constructor(@inject(IApplicationShell) appShell: IApplicationShell, @inject(IDocumentManager) documentManager: IDocumentManager) { + private notebookUri?: Uri; + private isDirty: boolean = false; + private timeout?: ReturnType; + constructor( + @inject(IApplicationShell) appShell: IApplicationShell, + @inject(IDocumentManager) documentManager: IDocumentManager, + @inject(INotebookEditorProvider) private readonly notebookProvider: INotebookEditorProvider, + @inject(IFileSystem) private readonly fileSytem: IFileSystem, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService + ) { + this.workspace.onDidChangeConfiguration(this.onSettingsChanded.bind(this), this, this.disposables); this.disposables.push(appShell.onDidChangeWindowState(this.onDidChangeWindowState.bind(this))); this.disposables.push(documentManager.onDidChangeActiveTextEditor(this.onDidChangeActiveTextEditor.bind(this))); } @@ -37,17 +48,97 @@ export class AutoSaveService implements IInteractiveWindowListener { return this.postEmitter.event; } - public onMessage(_message: string, _payload?: any): void { - noop(); + public onMessage(message: string, payload?: any): void { + if (message === InteractiveWindowMessages.NotebookIdentity) { + this.notebookUri = Uri.parse((payload as INotebookIdentity).resource); + } + if (message === InteractiveWindowMessages.LoadAllCellsComplete) { + const notebook = this.getNotebook(); + if (!notebook) { + traceError(`Received message ${message}, but there is no notebook for ${this.notebookUri ? this.notebookUri.fsPath : undefined}`); + return; + } + this.disposables.push(notebook.modified(this.onNotebookModified, this, this.disposables)); + this.disposables.push(notebook.saved(this.onNotebookSaved, this, this.disposables)); + } } public dispose(): void | undefined { this.disposables.filter(item => !!item).forEach(item => item.dispose()); + this.clearTimeout(); } - - private onDidChangeWindowState(e: WindowState) { - this.postEmitter.fire({ message: InteractiveWindowMessages.WindowStateChanged, payload: e }); + private onNotebookModified(_: INotebookEditor) { + this.isDirty = true; + // If we haven't started a timer, then start if necessary. + if (!this.timeout) { + this.setTimer(); + } + } + private onNotebookSaved(_: INotebookEditor) { + this.isDirty = false; + // If we haven't started a timer, then start if necessary. + if (!this.timeout) { + this.setTimer(); + } + } + private getNotebook(): INotebookEditor | undefined { + const uri = this.notebookUri; + if (!uri) { + return; + } + return this.notebookProvider.editors.find(item => this.fileSytem.arePathsSame(item.file.fsPath, uri.fsPath)); + } + private getAutoSaveSettings(): FileSettings { + const filesConfig = this.workspace.getConfiguration('files', this.notebookUri); + return { + autoSave: filesConfig.get('autoSave', 'off'), + autoSaveDelay: filesConfig.get('autoSaveDelay', 1000) + }; + } + private onSettingsChanded(e: ConfigurationChangeEvent) { + if (e.affectsConfiguration('files.autoSave') || e.affectsConfiguration('files.autoSave')) { + // Reset the timer, as we may have increased it, turned it off or other. + this.clearTimeout(); + this.setTimer(); + } + } + private setTimer() { + const settings = this.getAutoSaveSettings(); + if (!settings || settings.autoSave === 'off') { + return; + } + if (settings && settings.autoSave === 'afterDelay') { + // Add a timeout to save after n milli seconds. + // Do not use setInterval, as that will cause all handlers to queue up. + this.timeout = setTimeout(() => { + this.save(); + }, settings.autoSaveDelay); + } + } + private clearTimeout() { + if (this.timeout) { + clearTimeout(this.timeout); + this.timeout = undefined; + } + } + private save() { + this.clearTimeout(); + if (this.isDirty) { + // Notify webview to perform a save. + this.postEmitter.fire({ message: InteractiveWindowMessages.DoSave, payload: undefined }); + } else { + this.setTimer(); + } + } + private onDidChangeWindowState() { + const settings = this.getAutoSaveSettings(); + if (settings && settings.autoSave === 'onWindowChange') { + this.save(); + } } private onDidChangeActiveTextEditor(_e?: TextEditor) { - this.postEmitter.fire({ message: InteractiveWindowMessages.ActiveTextEditorChanged, payload: undefined }); + const settings = this.getAutoSaveSettings(); + if (settings && settings.autoSave === 'onFocusChange') { + this.save(); + } } } diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index affa86420c7c..10e9a4a8c5ee 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -9,14 +9,7 @@ import * as path from 'path'; import * as uuid from 'uuid/v4'; import { Event, EventEmitter, Memento, Uri, ViewColumn } from 'vscode'; -import { - IApplicationShell, - ICommandManager, - IDocumentManager, - ILiveShareApi, - IWebPanelProvider, - IWorkspaceService -} from '../../common/application/types'; +import { IApplicationShell, ICommandManager, IDocumentManager, ILiveShareApi, IWebPanelProvider, IWorkspaceService } from '../../common/application/types'; import { ContextKey } from '../../common/contextKey'; import { traceError } from '../../common/logger'; import { IFileSystem, TemporaryFile } from '../../common/platform/types'; @@ -28,21 +21,9 @@ import { EXTENSION_ROOT_DIR } from '../../constants'; import { IInterpreterService } from '../../interpreter/contracts'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { concatMultilineString } from '../common'; -import { - EditorContexts, - Identifiers, - NativeKeyboardCommandTelemetryLookup, - NativeMouseCommandTelemetryLookup, - Telemetry -} from '../constants'; +import { EditorContexts, Identifiers, NativeKeyboardCommandTelemetryLookup, NativeMouseCommandTelemetryLookup, Telemetry } from '../constants'; import { InteractiveBase } from '../interactive-common/interactiveBase'; -import { - IEditCell, - INativeCommand, - InteractiveWindowMessages, - ISaveAll, - ISubmitNewCell -} from '../interactive-common/interactiveWindowTypes'; +import { IEditCell, INativeCommand, InteractiveWindowMessages, ISaveAll, ISubmitNewCell } from '../interactive-common/interactiveWindowTypes'; import { CellState, ICell, @@ -74,6 +55,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { private closedEvent: EventEmitter = new EventEmitter(); private executedEvent: EventEmitter = new EventEmitter(); private modifiedEvent: EventEmitter = new EventEmitter(); + private savedEvent: EventEmitter = new EventEmitter(); private loadedPromise: Deferred = createDeferred(); private _file: Uri = Uri.file(''); private _dirty: boolean = false; @@ -129,7 +111,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { errorHandler, path.join(EXTENSION_ROOT_DIR, 'out', 'datascience-ui', 'native-editor', 'index_bundle.js'), localize.DataScience.nativeEditorTitle(), - ViewColumn.Active); + ViewColumn.Active + ); } public get visible(): boolean { @@ -187,6 +170,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this.modifiedEvent.event; } + public get saved(): Event { + return this.savedEvent.event; + } + // tslint:disable-next-line: no-any public onMessage(message: string, payload: any) { super.onMessage(message, payload); @@ -269,9 +256,19 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { this.submitCode(info.code, Identifiers.EmptyFileName, 0, info.id).ignoreErrors(); // Activate the other side, and send as if came from a file - this.ipynbProvider.show(this.file).then(_v => { - this.shareMessage(InteractiveWindowMessages.RemoteAddCode, { code: info.code, file: Identifiers.EmptyFileName, line: 0, id: info.id, originator: this.id, debug: false }); - }).ignoreErrors(); + this.ipynbProvider + .show(this.file) + .then(_v => { + this.shareMessage(InteractiveWindowMessages.RemoteAddCode, { + code: info.code, + file: Identifiers.EmptyFileName, + line: 0, + id: info.id, + originator: this.id, + debug: false + }); + }) + .ignoreErrors(); } } @@ -289,7 +286,14 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Activate the other side, and send as if came from a file await this.ipynbProvider.show(this.file); - this.shareMessage(InteractiveWindowMessages.RemoteReexecuteCode, { code: info.code, file: Identifiers.EmptyFileName, line: 0, id: info.id, originator: this.id, debug: false }); + this.shareMessage(InteractiveWindowMessages.RemoteReexecuteCode, { + code: info.code, + file: Identifiers.EmptyFileName, + line: 0, + id: info.id, + originator: this.id, + debug: false + }); } } catch (exc) { // Make this error our cell output @@ -298,10 +302,12 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { data: { source: info.code, cell_type: 'code', - outputs: [{ - output_type: 'error', - evalue: exc.toString() - }], + outputs: [ + { + output_type: 'error', + evalue: exc.toString() + } + ], metadata: {}, execution_count: null }, @@ -318,7 +324,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Handle an error await this.errorHandler.handleError(exc); - } } @@ -395,8 +400,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { cell_type: 'code', outputs: [], source: [], - metadata: { - }, + metadata: {}, execution_count: null } }; @@ -535,10 +539,13 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { private async setDirty(): Promise { if (!this._dirty) { this._dirty = true; + console.log('is dirty'); this.setTitle(`${path.basename(this.file.fsPath)}*`); await this.postMessage(InteractiveWindowMessages.NotebookDirty); // Tell listeners we're dirty this.modifiedEvent.fire(this); + } else { + console.log(this._dirty ? 'is dirty' : 'is NOT dirty'); } } @@ -570,7 +577,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { if (contents) { await this.viewDocument(contents); } - } catch (e) { await this.errorHandler.handleError(e); } finally { @@ -615,8 +621,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Update our file name and dirty state this._file = fileToSaveTo; await this.setClean(); + this.savedEvent.fire(this); } - } catch (e) { traceError(e); } diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 30be457f786e..e911f8ea05e2 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -250,6 +250,7 @@ export interface INotebookEditor extends IInteractiveBase { closed: Event; executed: Event; modified: Event; + saved: Event; readonly file: Uri; readonly visible: boolean; readonly active: boolean; diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts index f4f192a5fd44..f75777b3b16e 100644 --- a/src/datascience-ui/native-editor/autoSaveService.ts +++ b/src/datascience-ui/native-editor/autoSaveService.ts @@ -4,92 +4,22 @@ 'use strict'; import { IDisposable } from '../../client/common/types'; +import { noop } from '../../client/common/utils/misc'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; -import { FileSettings } from '../../client/datascience/types'; import { IMessageHandler } from '../react-common/postOffice'; -import { getSettings } from '../react-common/settingsReactSide'; import { NativeEditorStateController } from './nativeEditorStateController'; export class AutoSaveService implements IDisposable, IMessageHandler { - private timeout?: ReturnType; - private settings?: FileSettings; - // private fileSettings: typeof - constructor(private readonly controller: NativeEditorStateController) { - this.setTimer(); - } + constructor(private readonly controller: NativeEditorStateController) {} public dispose() { - this.clearTimeout(); + noop(); } // tslint:disable-next-line: no-any public handleMessage(type: string, _payload?: any): boolean { - switch (type) { - // When clean message is sent, this means notebook was saved. - // We need to reset the timer to start again (timer starts from last save). - case InteractiveWindowMessages.NotebookClean: { - this.setTimer(); - return true; - } - // When settings have been updated, its possible the timer has changed. - // We need to reset the timer to start again. - case InteractiveWindowMessages.UpdateSettings: { - const settings = getSettings().files; - if (this.settings && this.settings.autoSave === settings.autoSave && this.settings.autoSaveDelay === settings.autoSaveDelay) { - return true; - } - this.setTimer(); - return true; - } - case InteractiveWindowMessages.ActiveTextEditorChanged: { - // Save if active text editor changes. - if (this.settings && this.settings.autoSave === 'onFocusChange') { - this.save(); - } - return true; - } - case InteractiveWindowMessages.WindowStateChanged: { - // Save if window state changes. - if (this.settings && this.settings.autoSave === 'onWindowChange') { - this.save(); - } - return true; - } - default: { - return false; - } - } - } - public setTimer() { - this.clearTimeout(); - this.settings = getSettings().files; - if (this.settings && this.settings.autoSave === 'afterDelay') { - // Add a timeout to save after n milli seconds. - // Do not use setInterval, as that will cause all handlers to queue up. - this.timeout = setTimeout(() => this.save(), this.settings.autoSaveDelay); - } - } - private clearTimeout() { - if (this.timeout) { - clearTimeout(this.timeout); - this.timeout = undefined; - } - } - /** - * Save the notebook if it is dirty. - * If auto save is turned off or the notebook is not dirty, then this method is a noop. - * - * @private - * @returns - * @memberof AutoSaveService - */ - private save() { - const settings = getSettings().files; - if (settings.autoSave === 'off') { - return; - } - if (this.controller.getState().dirty === true) { + if (type === InteractiveWindowMessages.DoSave) { this.controller.save(); - } else { - this.setTimer(); + return true; } + return false; } } From 1dd857fe139ff0b032485e1e925f7bf808ae9db6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 15:05:39 -0700 Subject: [PATCH 16/23] Oops --- src/client/datascience/interactive-ipynb/nativeEditor.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 10e9a4a8c5ee..347acd69f300 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -539,13 +539,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { private async setDirty(): Promise { if (!this._dirty) { this._dirty = true; - console.log('is dirty'); this.setTitle(`${path.basename(this.file.fsPath)}*`); await this.postMessage(InteractiveWindowMessages.NotebookDirty); // Tell listeners we're dirty this.modifiedEvent.fire(this); - } else { - console.log(this._dirty ? 'is dirty' : 'is NOT dirty'); } } From b71a3f2aff57be431c40083e73d9541cc5e2a206 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 15:08:45 -0700 Subject: [PATCH 17/23] Fixes --- src/client/datascience/types.ts | 2 +- src/client/datascience/webViewHost.ts | 4 ---- src/datascience-ui/react-common/settingsReactSide.ts | 4 ---- src/test/datascience/reactHelpers.ts | 6 +----- 4 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index e911f8ea05e2..b8282f121fd8 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -399,8 +399,8 @@ export type FileSettings = { autoSaveDelay: number; autoSave: 'afterDelay' | 'off' | 'onFocusChange' | 'onWindowChange'; }; + export interface IDataScienceExtraSettings extends IDataScienceSettings { - files: FileSettings; extraSettings: { editorCursor: string; editorCursorBlink: string; diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index ae4c46d11767..a884de67618a 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -168,10 +168,6 @@ export class WebViewHost implements IDisposable { const theme = !workbench ? DefaultTheme : workbench.get('colorTheme', DefaultTheme); return { ...this.configService.getSettings().datascience, - files: { - autoSaveDelay: this.getValue(files, 'autoSaveDelay', 1000), - autoSave: this.getValue(files, 'autoSave', 'off') - }, extraSettings: { editorCursor: this.getValue(editor, 'cursorStyle', 'line'), editorCursorBlink: this.getValue(editor, 'cursorBlinking', 'blink'), diff --git a/src/datascience-ui/react-common/settingsReactSide.ts b/src/datascience-ui/react-common/settingsReactSide.ts index 8a94987c76c4..ca1a370e78a5 100644 --- a/src/datascience-ui/react-common/settingsReactSide.ts +++ b/src/datascience-ui/react-common/settingsReactSide.ts @@ -53,10 +53,6 @@ function load() { showJupyterVariableExplorer: true, variableExplorerExclude: 'module;function;builtin_function_or_method', enablePlotViewer: true, - files : { - autoSave: 'off', - autoSaveDelay: 1000 - }, extraSettings: { editorCursor: 'line', editorCursorBlink: 'blink', diff --git a/src/test/datascience/reactHelpers.ts b/src/test/datascience/reactHelpers.ts index 47cda6423320..d2b96ce4419f 100644 --- a/src/test/datascience/reactHelpers.ts +++ b/src/test/datascience/reactHelpers.ts @@ -359,11 +359,7 @@ export function setUpDomEnvironment() { collapseCellInputCodeByDefault: true, allowInput: true, showJupyterVariableExplorer: true, - variableExplorerExclude: 'module;function;builtin_function_or_method', - files : { - autoSave: 'off', - autoSaveDelay: 1000 - } + variableExplorerExclude: 'module;function;builtin_function_or_method' }; }; From 17cb1b16c5b17a10843d5ac050a781b41ea37f6b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 15:09:21 -0700 Subject: [PATCH 18/23] Fixes --- src/test/datascience/testHelpers.tsx | 29 ---------------------------- 1 file changed, 29 deletions(-) diff --git a/src/test/datascience/testHelpers.tsx b/src/test/datascience/testHelpers.tsx index e727e268b833..c13b66d11d4a 100644 --- a/src/test/datascience/testHelpers.tsx +++ b/src/test/datascience/testHelpers.tsx @@ -499,32 +499,3 @@ export function escapePath(p: string) { export function srcDirectory() { return path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience'); } - -// /** -// * Wait for a condition to be fulfilled within a timeout. -// * -// * @param {string} message -// * @param {number} timeout -// * @returns {Promise} -// */ -// export function waitForCondition(predicate: Function, timeout: number, errorMessage?: string): Promise { -// return new Promise((resolve, reject) => { -// // Reject after timeout exceeds. -// const timeoutTimer = setTimeout(() => { -// // tslint:disable-next-line: no-use-before-declare -// clearInterval(timer as any); -// reject(new Error(errorMessage || 'Timeout waiting for condition')); -// }, timeout); -// // Look for the message. -// const timer = setInterval(() => { -// try { -// predicate(); -// clearInterval(timer as any); -// clearTimeout(timeoutTimer); -// resolve(); -// } catch { -// noop(); -// } -// }, 10); -// }); -// } From 0acd88428bac324fd34f600e58c086a023da15d6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 15:56:21 -0700 Subject: [PATCH 19/23] Code review changes --- src/client/common/application/types.ts | 2 +- src/client/datascience/interactive-ipynb/autoSaveService.ts | 4 ++-- src/client/datascience/webViewHost.ts | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index 2c0f6a56be71..a63f538e6a8e 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -66,7 +66,7 @@ import { ICommandNameArgumentTypeMapping } from './commands'; export const IApplicationShell = Symbol('IApplicationShell'); export interface IApplicationShell { /** - * An [event](#Event) which fires when the focus s tate of the current window + * An [event](#Event) which fires when the focus state of the current window * changes. The value of the event represents whether the window is focused. */ readonly onDidChangeWindowState: Event; diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index 7f0e6c781a50..23b07d2f0228 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -36,7 +36,7 @@ export class AutoSaveService implements IInteractiveWindowListener { @inject(IApplicationShell) appShell: IApplicationShell, @inject(IDocumentManager) documentManager: IDocumentManager, @inject(INotebookEditorProvider) private readonly notebookProvider: INotebookEditorProvider, - @inject(IFileSystem) private readonly fileSytem: IFileSystem, + @inject(IFileSystem) private readonly fileSystem: IFileSystem, @inject(IWorkspaceService) private readonly workspace: IWorkspaceService ) { this.workspace.onDidChangeConfiguration(this.onSettingsChanded.bind(this), this, this.disposables); @@ -85,7 +85,7 @@ export class AutoSaveService implements IInteractiveWindowListener { if (!uri) { return; } - return this.notebookProvider.editors.find(item => this.fileSytem.arePathsSame(item.file.fsPath, uri.fsPath)); + return this.notebookProvider.editors.find(item => this.fileSystem.arePathsSame(item.file.fsPath, uri.fsPath)); } private getAutoSaveSettings(): FileSettings { const filesConfig = this.workspace.getConfiguration('files', this.notebookUri); diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index a884de67618a..a28255d4fec0 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -163,7 +163,6 @@ export class WebViewHost implements IDisposable { protected generateDataScienceExtraSettings(): IDataScienceExtraSettings { const editor = this.workspaceService.getConfiguration('editor'); - const files = this.workspaceService.getConfiguration('files'); const workbench = this.workspaceService.getConfiguration('workbench'); const theme = !workbench ? DefaultTheme : workbench.get('colorTheme', DefaultTheme); return { From 182b01cde3b07c1e684a3d88928593edbe60dfdd Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 16:03:58 -0700 Subject: [PATCH 20/23] Sonar fixes --- src/client/datascience/interactive-ipynb/autoSaveService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index 23b07d2f0228..4c27e9b56a9d 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -95,7 +95,7 @@ export class AutoSaveService implements IInteractiveWindowListener { }; } private onSettingsChanded(e: ConfigurationChangeEvent) { - if (e.affectsConfiguration('files.autoSave') || e.affectsConfiguration('files.autoSave')) { + if (e.affectsConfiguration('files.autoSave') || e.affectsConfiguration('files.autoSaveDelay')) { // Reset the timer, as we may have increased it, turned it off or other. this.clearTimeout(); this.setTimer(); From aca10ed632a21d526bb7e465eb000641c18c5c36 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 16:16:29 -0700 Subject: [PATCH 21/23] Fix code review comments --- src/client/datascience/interactive-ipynb/autoSaveService.ts | 6 ++---- src/client/datascience/interactive-ipynb/nativeEditor.ts | 4 ++++ src/client/datascience/types.ts | 5 +++++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/client/datascience/interactive-ipynb/autoSaveService.ts b/src/client/datascience/interactive-ipynb/autoSaveService.ts index 4c27e9b56a9d..61f69e225490 100644 --- a/src/client/datascience/interactive-ipynb/autoSaveService.ts +++ b/src/client/datascience/interactive-ipynb/autoSaveService.ts @@ -30,7 +30,6 @@ export class AutoSaveService implements IInteractiveWindowListener { private postEmitter: EventEmitter<{ message: string; payload: any }> = new EventEmitter<{ message: string; payload: any }>(); private disposables: IDisposable[] = []; private notebookUri?: Uri; - private isDirty: boolean = false; private timeout?: ReturnType; constructor( @inject(IApplicationShell) appShell: IApplicationShell, @@ -67,14 +66,12 @@ export class AutoSaveService implements IInteractiveWindowListener { this.clearTimeout(); } private onNotebookModified(_: INotebookEditor) { - this.isDirty = true; // If we haven't started a timer, then start if necessary. if (!this.timeout) { this.setTimer(); } } private onNotebookSaved(_: INotebookEditor) { - this.isDirty = false; // If we haven't started a timer, then start if necessary. if (!this.timeout) { this.setTimer(); @@ -122,7 +119,8 @@ export class AutoSaveService implements IInteractiveWindowListener { } private save() { this.clearTimeout(); - if (this.isDirty) { + const notebook = this.getNotebook(); + if (notebook && notebook.isDirty) { // Notify webview to perform a save. this.postEmitter.fire({ message: InteractiveWindowMessages.DoSave, payload: undefined }); } else { diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 347acd69f300..29b4fd644ea7 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -174,6 +174,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this.savedEvent.event; } + public get isDirty(): boolean { + return this._dirty; + } + // tslint:disable-next-line: no-any public onMessage(message: string, payload: any) { super.onMessage(message, payload); diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index b8282f121fd8..ab56bf121c3a 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -247,10 +247,15 @@ export interface INotebookEditorProvider { // For native editing, the INotebookEditor acts like a TextEditor and a TextDocument together export const INotebookEditor = Symbol('INotebookEditor'); export interface INotebookEditor extends IInteractiveBase { + /** + * `true` if there are unpersisted changes. + */ + readonly isDirty: boolean; closed: Event; executed: Event; modified: Event; saved: Event; + readonly dirty: boolean; readonly file: Uri; readonly visible: boolean; readonly active: boolean; From e44fe92e95d38c125f882ae4ed43f3fb8535a659 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 16:19:28 -0700 Subject: [PATCH 22/23] Address code review comments --- .../native-editor/autoSaveService.ts | 25 ------------------- .../nativeEditorStateController.ts | 17 +++---------- 2 files changed, 3 insertions(+), 39 deletions(-) delete mode 100644 src/datascience-ui/native-editor/autoSaveService.ts diff --git a/src/datascience-ui/native-editor/autoSaveService.ts b/src/datascience-ui/native-editor/autoSaveService.ts deleted file mode 100644 index f75777b3b16e..000000000000 --- a/src/datascience-ui/native-editor/autoSaveService.ts +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { IDisposable } from '../../client/common/types'; -import { noop } from '../../client/common/utils/misc'; -import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; -import { IMessageHandler } from '../react-common/postOffice'; -import { NativeEditorStateController } from './nativeEditorStateController'; - -export class AutoSaveService implements IDisposable, IMessageHandler { - constructor(private readonly controller: NativeEditorStateController) {} - public dispose() { - noop(); - } - // tslint:disable-next-line: no-any - public handleMessage(type: string, _payload?: any): boolean { - if (type === InteractiveWindowMessages.DoSave) { - this.controller.save(); - return true; - } - return false; - } -} diff --git a/src/datascience-ui/native-editor/nativeEditorStateController.ts b/src/datascience-ui/native-editor/nativeEditorStateController.ts index b43773cfc496..6ceb10dda9a6 100644 --- a/src/datascience-ui/native-editor/nativeEditorStateController.ts +++ b/src/datascience-ui/native-editor/nativeEditorStateController.ts @@ -14,28 +14,14 @@ import { } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { createEmptyCell, extractInputText, ICellViewModel } from '../interactive-common/mainState'; import { IMainStateControllerProps, MainStateController } from '../interactive-common/mainStateController'; -import { IMessageHandler } from '../react-common/postOffice'; import { getSettings } from '../react-common/settingsReactSide'; -import { AutoSaveService } from './autoSaveService'; export class NativeEditorStateController extends MainStateController { private waitingForLoadRender: boolean = false; - private listeners: IMessageHandler[]; // tslint:disable-next-line:max-func-body-length constructor(props: IMainStateControllerProps) { super(props); - this.listeners = [new AutoSaveService(this)]; - this.listeners.forEach(listener => this.postOffice.addHandler(listener)); - } - public dispose(){ - this.listeners.forEach(listener => { - this.postOffice.removeHandler(listener); - if (listener.dispose){ - listener.dispose(); - } - }); - super.dispose(); } // tslint:disable-next-line: no-any public handleMessage(msg: string, payload?: any) { @@ -67,6 +53,9 @@ export class NativeEditorStateController extends MainStateController { case InteractiveWindowMessages.NotebookAddCellBelow: this.addNewCell(); break; + case InteractiveWindowMessages.DoSave: + this.save(); + break; default: break; From 0e366e737e6a4f0eb011dc162098b5c4873e7a7a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 9 Oct 2019 16:31:45 -0700 Subject: [PATCH 23/23] Oops --- src/client/datascience/types.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index ab56bf121c3a..9b4266655e8c 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -247,15 +247,14 @@ export interface INotebookEditorProvider { // For native editing, the INotebookEditor acts like a TextEditor and a TextDocument together export const INotebookEditor = Symbol('INotebookEditor'); export interface INotebookEditor extends IInteractiveBase { - /** - * `true` if there are unpersisted changes. - */ - readonly isDirty: boolean; closed: Event; executed: Event; modified: Event; saved: Event; - readonly dirty: boolean; + /** + * `true` if there are unpersisted changes. + */ + readonly isDirty: boolean; readonly file: Uri; readonly visible: boolean; readonly active: boolean;