diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts index 8a691f7f88..3612ea3532 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts @@ -277,7 +277,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges { } } - getNodeIndex(n) { + getNodeIndex(n: Node): number { let i = 0; // walk up the node to the top and get it's index n = n.previousSibling; diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts index b218c6019a..8609c80218 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts @@ -55,6 +55,8 @@ import { NotebookViewsExtension } from 'sql/workbench/services/notebook/browser/ import { MaskedLabeledMenuItemActionItem } from 'sql/platform/actions/browser/menuEntryActionViewItem'; import { IActionViewItem } from 'vs/base/browser/ui/actionbar/actionbar'; import { Emitter } from 'vs/base/common/event'; +import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; +import { KeyCode } from 'vs/base/common/keyCodes'; export const NOTEBOOK_SELECTOR: string = 'notebook-component'; @@ -118,6 +120,19 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe this._register(this._configurationService.onDidChangeConfiguration(e => { this.doubleClickEditEnabled = this._configurationService.getValue('notebook.enableDoubleClickEdit'); })); + this._register(DOM.addDisposableListener(window, DOM.EventType.KEY_DOWN, (e: KeyboardEvent) => { + // Prevent the undo/redo from happening in other notebooks and to prevent the execution of undo/redo in the cell. + if (this.isActive() && this.activeCellId === '') { + let event = new StandardKeyboardEvent(e); + if ((event.metaKey && event.shiftKey && event.keyCode === KeyCode.KEY_Z) || event.ctrlKey && event.keyCode === KeyCode.KEY_Y) { + DOM.EventHelper.stop(event, true); + this._model.redo(); + } else if ((event.ctrlKey || event.metaKey) && event.keyCode === KeyCode.KEY_Z) { + DOM.EventHelper.stop(event, true); + this._model.undo(); + } + } + })); } ngOnInit() { diff --git a/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts b/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts index 15a5327705..f900ec04fd 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts @@ -28,6 +28,7 @@ import { NotebookViewsExtension } from 'sql/workbench/services/notebook/browser/ import { INotebookView } from 'sql/workbench/services/notebook/browser/notebookViews/notebookViews'; import { Deferred } from 'sql/base/common/promise'; import { NotebookChangeType } from 'sql/workbench/services/notebook/common/contracts'; +import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; export const NOTEBOOKEDITOR_SELECTOR: string = 'notebookeditor-component'; @@ -61,6 +62,7 @@ export class NotebookEditorComponent extends AngularDisposable { @Inject(forwardRef(() => ChangeDetectorRef)) private _changeRef: ChangeDetectorRef, @Inject(IConfigurationService) private _configurationService: IConfigurationService, @Inject(IConnectionManagementService) private connectionManagementService: IConnectionManagementService, + @Inject(IUndoRedoService) private _undoService: IUndoRedoService, ) { super(); this.updateProfile(); @@ -116,7 +118,7 @@ export class NotebookEditorComponent extends AngularDisposable { layoutChanged: this._notebookParams.input.layoutChanged, capabilitiesService: this.capabilitiesService, editorLoadedTimestamp: this._notebookParams.input.editorOpenedTimestamp - }, this.profile, this.logService, this.notificationService, this.adstelemetryService, this.connectionManagementService, this._configurationService, this.capabilitiesService); + }, this.profile, this.logService, this.notificationService, this.adstelemetryService, this.connectionManagementService, this._configurationService, this._undoService, this.capabilitiesService); let trusted = await this.notebookService.isNotebookTrustCached(this._notebookParams.notebookUri, this.isDirty()); this.model = this._register(model); diff --git a/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts index 0abe606048..c9820150c5 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts @@ -218,5 +218,5 @@ export async function createandLoadNotebookModel(codeContent?: nb.INotebookConte layoutChanged: undefined, capabilitiesService: undefined }; - return new NotebookModel(defaultModelOptions, undefined, undefined, undefined, undefined, undefined, undefined); + return new NotebookModel(defaultModelOptions, undefined, undefined, undefined, undefined, undefined, undefined, undefined); } diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts index d9f6ac696f..35f713e24b 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts @@ -31,6 +31,10 @@ import { CellTypes } from 'sql/workbench/services/notebook/common/contracts'; import { NotebookViewsExtension } from 'sql/workbench/services/notebook/browser/notebookViews/notebookViewsExtension'; import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService'; +import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService'; +import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; let initialNotebookContent: nb.INotebookContents = { cells: [{ @@ -57,8 +61,10 @@ let initialNotebookContent: nb.INotebookContents = { let defaultUri = URI.file('/some/path.ipynb'); let notificationService: TypeMoq.Mock; let capabilitiesService: TypeMoq.Mock; +let dialogService: TypeMoq.Mock; let instantiationService: IInstantiationService; let configurationService: IConfigurationService; +let undoRedoService: IUndoRedoService; suite('NotebookViews', function (): void { let defaultViewName = 'Default New View'; @@ -148,6 +154,8 @@ suite('NotebookViews', function (): void { executeManagers[0].sessionManager = mockSessionManager.object; notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); capabilitiesService = TypeMoq.Mock.ofType(TestCapabilitiesService); + dialogService = TypeMoq.Mock.ofType(TestDialogService, TypeMoq.MockBehavior.Loose); + undoRedoService = new UndoRedoService(dialogService.object, notificationService.object); memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, ''); memento.setup(x => x.getMemento(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => void 0); queryConnectionService = TypeMoq.Mock.ofType(TestConnectionManagementService, TypeMoq.MockBehavior.Loose, memento.object, undefined, new TestStorageService()); @@ -176,7 +184,7 @@ suite('NotebookViews', function (): void { mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(initialNotebookContent)); defaultModelOptions.contentLoader = mockContentManager.object; - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undefined); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService, undefined); await model.loadContents(); await model.requestModelLoad(); diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts index da0157e02e..07863f1736 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts @@ -8,6 +8,7 @@ import * as os from 'os'; import * as assert from 'assert'; import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/testCapabilitiesService'; +import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService'; import { ConnectionManagementService } from 'sql/workbench/services/connection/browser/connectionManagementService'; import { CellModel } from 'sql/workbench/services/notebook/browser/models/cell'; import { CellTypes, NotebookChangeType } from 'sql/workbench/services/notebook/common/contracts'; @@ -39,6 +40,8 @@ import { IStorageService } from 'vs/platform/storage/common/storage'; import { TestStorageService, TestTextResourcePropertiesService } from 'vs/workbench/test/common/workbenchTestServices'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; import { IProductService } from 'vs/platform/product/common/productService'; +import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService'; class ServiceAccessor { @@ -74,6 +77,8 @@ suite('Notebook Editor Model', function (): void { let defaultModelOptions: INotebookModelOptions; const logService = new NullLogService(); const notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); + const dialogService = TypeMoq.Mock.ofType(TestDialogService, TypeMoq.MockBehavior.Loose); + const undoRedoService = new UndoRedoService(dialogService.object, notificationService.object); let memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, ''); memento.setup(x => x.getMemento(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => void 0); let testinstantiationService = new TestInstantiationService(); @@ -978,7 +983,7 @@ suite('Notebook Editor Model', function (): void { let options: INotebookModelOptions = Object.assign({}, defaultModelOptions, >{ factory: mockModelFactory.object }); - notebookModel = new NotebookModel(options, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undefined); + notebookModel = new NotebookModel(options, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService, undefined); await notebookModel.loadContents(); } diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts index 1bd215a6cd..cd26b5d1f4 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts @@ -10,6 +10,8 @@ import * as sinon from 'sinon'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; +import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService'; + import { URI } from 'vs/base/common/uri'; import { ExecuteManagerStub, SerializationManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; @@ -37,6 +39,9 @@ import { uriPrefixes } from 'sql/platform/connection/common/utils'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; +import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService'; let expectedNotebookContent: nb.INotebookContents = { cells: [{ @@ -133,6 +138,8 @@ let clientSessionOptions: IClientSessionOptions; let sessionReady: Deferred; let mockModelFactory: TypeMoq.Mock; let notificationService: TypeMoq.Mock; +let dialogService: TypeMoq.Mock; +let undoRedoService: IUndoRedoService; let capabilitiesService: ICapabilitiesService; let instantiationService: IInstantiationService; let configurationService: IConfigurationService; @@ -150,6 +157,8 @@ suite('notebook model', function (): void { executeManagers[0].sessionManager = mockSessionManager.object; sessionReady = new Deferred(); notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); + dialogService = TypeMoq.Mock.ofType(TestDialogService, TypeMoq.MockBehavior.Loose); + undoRedoService = new UndoRedoService(dialogService.object, notificationService.object); capabilitiesService = new TestCapabilitiesService(); memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, ''); memento.setup(x => x.getMemento(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => void 0); @@ -206,7 +215,7 @@ suite('notebook model', function (): void { mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(emptyNotebook)); defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); // Then I expect to have 0 code cell as the contents @@ -222,7 +231,7 @@ suite('notebook model', function (): void { mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(true); await model.requestModelLoad(); @@ -239,7 +248,7 @@ suite('notebook model', function (): void { // When I initalize the model // Then it should throw - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); assert.strictEqual(model.inErrorState, false); await assert.rejects(async () => { await model.loadContents(); }); assert.strictEqual(model.inErrorState, true); @@ -252,7 +261,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); // Then I expect all cells to be in the model @@ -280,7 +289,7 @@ suite('notebook model', function (): void { defaultModelOptions.providerId = 'jupyter'; // When I initalize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); // I expect the default provider to be jupyter @@ -290,7 +299,7 @@ suite('notebook model', function (): void { defaultModelOptions.providerId = 'SQL'; // When I initalize the model - model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); // I expect the default provider to be SQL @@ -315,7 +324,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); let activeCellChangeCount = 0; @@ -372,7 +381,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); assert.strictEqual(model.notebookUri, defaultModelOptions.notebookUri, 'Notebook model has incorrect URI'); @@ -400,7 +409,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); assert.strictEqual(model.notebookUri, defaultModelOptions.notebookUri, 'Notebook model has incorrect URI'); @@ -424,7 +433,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); assert.strictEqual(model.notebookUri, defaultModelOptions.notebookUri, 'Notebook model has incorrect URI'); @@ -445,7 +454,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); assert.strictEqual(model.notebookUri, defaultModelOptions.notebookUri, 'Notebook model has incorrect URI'); @@ -464,7 +473,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); assert.strictEqual(model.notebookUri, defaultModelOptions.notebookUri, 'Notebook model has incorrect URI'); @@ -484,7 +493,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); assert.strictEqual(model.notebookUri, defaultModelOptions.notebookUri, 'Notebook model has incorrect URI'); @@ -505,7 +514,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); // Count number of times onError event is fired @@ -550,13 +559,59 @@ suite('notebook model', function (): void { assert(isUndefinedOrNull(notebookContentChange), 'There still should be no content change after an error is recorded'); }); + test('Should undo/redo changes after deleting cell', async function (): Promise { + // Given a notebook with 2 cells + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); + mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); + defaultModelOptions.contentLoader = mockContentManager.object; + + // When I initalize the model + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); + await model.loadContents(); + + // Then I expect all cells to be in the model + assert.strictEqual(model.cells.length, 2, 'Cell count in model is incorrect'); + // Delete the first cell + model.deleteCell(model.cells[0]); + assert.strictEqual(model.cells.length, 1, 'Cell model length should be 1 after cell deletion'); + model.undo(); + assert.strictEqual(model.cells.length, 2, 'Cell model length should be 2 after undo'); + assert.deepStrictEqual(model.cells[0].source, expectedNotebookContent.cells[0].source, 'Expected cell source is incorrect'); + model.redo(); + assert.strictEqual(model.cells.length, 1, 'Cell model length should be 1 after redo'); + assert.strictEqual(model.findCellIndex(model.cells[0]), 0, 'findCellIndex returned wrong cell info for only remaining cell'); + }); + + test('Should undo/redo changes after moving cell', async function (): Promise { + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); + mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); + defaultModelOptions.contentLoader = mockContentManager.object; + + // When I initialize the model + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); + await model.loadContents(); + + let firstCell = model.cells[0]; + let secondCell = model.cells[1]; + // Move Second Cell up + model.moveCell(secondCell, 0); + assert.strictEqual(model.cells.indexOf(firstCell), 1, 'First Cell did not move down correctly'); + assert.strictEqual(model.cells.indexOf(secondCell), 0, 'Second Cell did not move up correctly'); + model.undo(); + assert.strictEqual(model.cells.indexOf(firstCell), 0, 'Failed to undo first cell to its original position'); + assert.strictEqual(model.cells.indexOf(secondCell), 1, 'Failed to undo second Cell to its orignal position'); + model.redo(); + assert.strictEqual(model.cells.indexOf(firstCell), 1, 'Failed to redo'); + assert.strictEqual(model.cells.indexOf(secondCell), 0, 'Failed to redo'); + }); + test('Should notify cell on metadata change', async function (): Promise { let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); let notebookContentChange: NotebookContentChange; @@ -572,7 +627,7 @@ suite('notebook model', function (): void { mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); defaultModelOptions.contentLoader = mockContentManager.object; - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); let newCell: ICellModel; @@ -604,7 +659,7 @@ suite('notebook model', function (): void { sessionReady.resolve(); let sessionFired = false; - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); model.onClientSessionReady((session) => sessionFired = true); await model.loadContents(); await model.requestModelLoad(); @@ -634,7 +689,7 @@ suite('notebook model', function (): void { let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); defaultModelOptions.contentLoader = mockContentManager.object; - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.requestModelLoad(); let actualChanged: NotebookContentChange; @@ -703,7 +758,7 @@ suite('notebook model', function (): void { mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined, queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined, queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); let output = model.toJSON(); @@ -836,7 +891,7 @@ suite('notebook model', function (): void { sinon.stub(configurationService, 'getValue').returns(true); // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); // I expect the saved connection name to be read @@ -865,7 +920,7 @@ suite('notebook model', function (): void { defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); // I expect multiConnectionMode to be set to true @@ -896,7 +951,7 @@ suite('notebook model', function (): void { // Given I have a session that fails to start sessionReady.resolve(); - let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); + let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService); await model.loadContents(); await model.requestModelLoad(); @@ -918,7 +973,7 @@ suite('notebook model', function (): void { let options: INotebookModelOptions = Object.assign({}, defaultModelOptions, >{ factory: mockModelFactory.object }); - let model = new NotebookModel(options, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, capabilitiesService); + let model = new NotebookModel(options, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService, undoRedoService, capabilitiesService); model.onClientSessionReady((session) => actualSession = session); await model.requestModelLoad(); diff --git a/src/sql/workbench/services/notebook/browser/models/cellEdit.ts b/src/sql/workbench/services/notebook/browser/models/cellEdit.ts new file mode 100644 index 0000000000..1c3925a83a --- /dev/null +++ b/src/sql/workbench/services/notebook/browser/models/cellEdit.ts @@ -0,0 +1,78 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IResourceUndoRedoElement, UndoRedoElementType } from 'vs/platform/undoRedo/common/undoRedo'; +import { ICellModel, MoveDirection } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; +import { localize } from 'vs/nls'; + +export class MoveCellEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = localize('moveCellEdit', "Move Cell"); + resource = this.model.notebookUri; + + constructor(private model: NotebookModel, private cell: ICellModel, private moveDirection: MoveDirection) { + } + + undo(): void { + const direction = this.moveDirection === MoveDirection.Down ? MoveDirection.Up : MoveDirection.Down; + this.model.moveCell(this.cell, direction, false); + } + + redo(): void { + this.model.moveCell(this.cell, this.moveDirection, false); + } +} + +export class SplitCellEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = localize('splitCellEdit', "Split Cell"); + resource = this.model.notebookUri; + + constructor(private model: NotebookModel, private firstCell: ICellModel, private secondCell: ICellModel, private newLinesRemoved: string[]) { + } + + undo(): void { + this.model.mergeCells(this.firstCell, this.secondCell, this.newLinesRemoved); + } + + redo(): void { + // no-op currently, will add support on next release + } +} + +export class DeleteCellEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = localize('deleteCellEdit', "Delete Cell"); + resource = this.model.notebookUri; + + constructor(private model: NotebookModel, private cell: ICellModel, private index: number) { + } + + undo(): void { + this.model.insertCell(this.cell, this.index, false); + } + + redo(): void { + this.model.deleteCell(this.cell, false); + } +} + +export class AddCellEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = localize('addCellEdit', "Add Cell"); + resource = this.model.notebookUri; + + constructor(private model: NotebookModel, private cell: ICellModel, private index: number) { + } + + undo(): void { + this.model.deleteCell(this.cell, false); + } + + redo(): void { + this.model.insertCell(this.cell, this.index, false); + } +} diff --git a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts index c099f6f2c6..5df7f59a67 100644 --- a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts +++ b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts @@ -35,7 +35,8 @@ import { isUUID } from 'vs/base/common/uuid'; import { TextModel } from 'vs/editor/common/model/textModel'; import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor'; import { CodeEditorWidget } from 'vs/editor/browser/widget/codeEditorWidget'; - +import { AddCellEdit, DeleteCellEdit, MoveCellEdit, SplitCellEdit } from 'sql/workbench/services/notebook/browser/models/cellEdit'; +import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; /* * Used to control whether a message in a dialog/wizard is displayed as an error, * warning, or informational message. Default is error. @@ -123,7 +124,8 @@ export class NotebookModel extends Disposable implements INotebookModel { @IAdsTelemetryService private readonly adstelemetryService: IAdsTelemetryService, @IConnectionManagementService private connectionManagementService: IConnectionManagementService, @IConfigurationService private configurationService: IConfigurationService, - @ICapabilitiesService private _capabilitiesService?: ICapabilitiesService + @IUndoRedoService private undoService: IUndoRedoService, + @ICapabilitiesService private _capabilitiesService?: ICapabilitiesService, ) { super(); if (!_notebookOptions || !_notebookOptions.notebookUri || !_notebookOptions.executeManagers) { @@ -538,12 +540,11 @@ export class NotebookModel extends Disposable implements INotebookModel { if (this.inErrorState) { return undefined; } - let cell = this.createCell(cellType); return this.insertCell(cell, index); } - public splitCell(cellType: CellType, notebookService: INotebookService, index?: number): ICellModel | undefined { + public splitCell(cellType: CellType, notebookService: INotebookService, index?: number, addToUndoStack: boolean = true): ICellModel | undefined { if (this.inErrorState) { return undefined; } @@ -564,6 +565,7 @@ export class NotebookModel extends Disposable implements INotebookModel { let newCell = undefined, tailCell = undefined, partialSource = undefined; let newCellIndex = index; let tailCellIndex = index; + let newLinesRemoved: string[] = []; // Save UI state let showMarkdown = this.cells[index].showMarkdown; @@ -619,7 +621,7 @@ export class NotebookModel extends Disposable implements INotebookModel { newCell = this.createCell(cellType); newCell.source = newSource; newCellIndex++; - this.insertCell(newCell, newCellIndex); + this.insertCell(newCell, newCellIndex, false); } else { //update the existing cell this.cells[index].source = newSource; @@ -636,19 +638,24 @@ export class NotebookModel extends Disposable implements INotebookModel { } //Remove the trailing empty line after the cursor if (tailSource[0] === '\r\n' || tailSource[0] === '\n') { - tailSource.splice(0, 1); + newLinesRemoved = tailSource.splice(0, 1); } tailCell.source = tailSource; tailCellIndex = newCellIndex + 1; - this.insertCell(tailCell, tailCellIndex); + this.insertCell(tailCell, tailCellIndex, false); } let activeCell = newCell ? newCell : (headContent.length ? tailCell : this.cells[index]); let activeCellIndex = newCell ? newCellIndex : (headContent.length ? tailCellIndex : index); + if (addToUndoStack) { + let headCell = newCell ? newCell : this.cells[index]; + this.undoService.pushElement(new SplitCellEdit(this, headCell, tailCell, newLinesRemoved)); + } //make new cell Active this.updateActiveCell(activeCell); activeCell.isEditMode = true; + this._contentChangedEmitter.fire({ changeType: NotebookChangeType.CellsModified, cells: [activeCell], @@ -664,7 +671,23 @@ export class NotebookModel extends Disposable implements INotebookModel { return undefined; } - public insertCell(cell: ICellModel, index?: number): ICellModel | undefined { + public mergeCells(cell: ICellModel, secondCell: ICellModel, newLinesRemoved: string[] | undefined): void { + let index = this._cells.findIndex(cell => cell.equals(cell)); + if (index > -1) { + cell.source = newLinesRemoved.length > 0 ? [...cell.source, ...newLinesRemoved, ...secondCell.source] : [...cell.source, ...secondCell.source]; + cell.isEditMode = true; + // Set newly created cell as active cell + this.updateActiveCell(cell); + this._contentChangedEmitter.fire({ + changeType: NotebookChangeType.CellsModified, + cells: [cell], + cellIndex: index + }); + this.deleteCell(secondCell, false); + } + } + + public insertCell(cell: ICellModel, index?: number, addToUndoStack: boolean = true): ICellModel | undefined { if (this.inErrorState) { return undefined; } @@ -674,9 +697,14 @@ export class NotebookModel extends Disposable implements INotebookModel { this._cells.push(cell); index = undefined; } - // Set newly created cell as active cell - this.updateActiveCell(cell); cell.isEditMode = true; + if (addToUndoStack) { + // Only make cell active when inserting the cell. If we update the active cell when undoing/redoing, the user would have to deselect the cell first + // and to undo multiple times. + this.updateActiveCell(cell); + this.undoService.pushElement(new AddCellEdit(this, cell, index)); + } + this._contentChangedEmitter.fire({ changeType: NotebookChangeType.CellsModified, cells: [cell], @@ -712,7 +740,7 @@ export class NotebookModel extends Disposable implements INotebookModel { } } - moveCell(cell: ICellModel, direction: MoveDirection): void { + moveCell(cell: ICellModel, direction: MoveDirection, addToUndoStack: boolean = true): void { if (this.inErrorState) { return; } @@ -735,10 +763,13 @@ export class NotebookModel extends Disposable implements INotebookModel { this._cells.splice(index - 1, 0, cell); } + if (addToUndoStack) { + this.undoService.pushElement(new MoveCellEdit(this, cell, direction)); + // If we update the active cell when undoing/redoing, the user would have to deselect the cell first and to undo multiple times. + this.updateActiveCell(cell); + } index = this.findCellIndex(cell); - // Set newly created cell as active cell - this.updateActiveCell(cell); this._contentChangedEmitter.fire({ changeType: NotebookChangeType.CellsModified, cells: [cell], @@ -784,12 +815,16 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._notebookOptions.factory.createCell(singleCell, { notebook: this, isTrusted: true }); } - deleteCell(cellModel: ICellModel): void { + deleteCell(cellModel: ICellModel, addToUndoStack: boolean = true): void { if (this.inErrorState || !this._cells) { return; } let index = this._cells.findIndex(cell => cell.equals(cellModel)); if (index > -1) { + if (addToUndoStack) { + this.undoService.pushElement(new DeleteCellEdit(this, cellModel, index)); + } + this._cells.splice(index, 1); if (this._activeCell === cellModel) { this.updateActiveCell(); @@ -816,6 +851,7 @@ export class NotebookModel extends Disposable implements INotebookModel { // TODO: should we validate and complete required missing parameters? let contents: nb.ICellContents = edit.cell as nb.ICellContents; newCells.push(this._notebookOptions.factory.createCell(contents, { notebook: this, isTrusted: this._trustedMode })); + this.undoService.pushElement(new AddCellEdit(this, newCells[0], edit.range.start)); } this._cells.splice(edit.range.start, edit.range.end - edit.range.start, ...newCells); if (newCells.length > 0) { @@ -1490,4 +1526,16 @@ export class NotebookModel extends Disposable implements INotebookModel { this._contentChangedEmitter.fire(changeInfo); } + + public undo(): void { + if (this.undoService.canUndo(this.notebookUri)) { + this.undoService.undo(this.notebookUri); + } + } + + public redo(): void { + if (this.undoService.canRedo(this.notebookUri)) { + this.undoService.redo(this.notebookUri); + } + } }