From 5d9e5655572d188fa6756d9aaa6605f2c1891fa0 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 18 Jun 2020 19:22:08 -0700 Subject: [PATCH 01/16] :notebook: Separate selections and text model --- .../src/notebook.test.ts | 19 +- .../api/browser/mainThreadNotebook.ts | 2 +- .../notebook/browser/contrib/coreActions.ts | 2 +- .../notebook/browser/notebookBrowser.ts | 6 +- .../notebook/browser/notebookEditorWidget.ts | 33 +-- .../browser/viewModel/baseCellViewModel.ts | 19 +- .../notebook/browser/viewModel/cellEdit.ts | 193 ++--------------- .../browser/viewModel/codeCellViewModel.ts | 8 +- .../viewModel/markdownCellViewModel.ts | 8 +- .../browser/viewModel/notebookViewModel.ts | 203 +++++------------- .../contrib/notebook/common/model/cellEdit.ts | 184 ++++++++++++++++ .../common/model/notebookCellTextModel.ts | 32 ++- .../common/model/notebookTextModel.ts | 142 +++++++++++- .../contrib/notebook/common/notebookCommon.ts | 2 +- .../notebook/test/notebookViewModel.test.ts | 2 +- .../notebook/test/testNotebookEditor.ts | 8 +- 16 files changed, 485 insertions(+), 378 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/common/model/cellEdit.ts diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index e07bb1a91258..d0f4cfc2d500 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -222,6 +222,13 @@ suite('API tests', () => { await vscode.commands.executeCommand('workbench.action.files.save'); await vscode.commands.executeCommand('workbench.action.closeAllEditors'); + + await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); + const firstEditor = vscode.notebook.activeNotebookEditor; + assert.equal(firstEditor?.document.cells.length, 1); + + await vscode.commands.executeCommand('workbench.action.files.save'); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); test('notebook editor active/visible', async function () { @@ -290,7 +297,7 @@ suite('API tests', () => { assert.equal(cellChangeEventRet.changes[0].items[0], vscode.notebook.activeNotebookEditor!.document.cells[1]); await vscode.commands.executeCommand('workbench.action.files.save'); - await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); test('initialzation should not emit cell change events.', async function () { @@ -766,6 +773,9 @@ suite('metadata', () => { assert.equal(vscode.notebook.activeNotebookEditor!.document.metadata.custom!['testMetadata'] as boolean, false); assert.equal(vscode.notebook.activeNotebookEditor!.selection?.metadata.custom!['testCellMetadata'] as number, 123); assert.equal(vscode.notebook.activeNotebookEditor!.selection?.language, 'typescript'); + + await vscode.commands.executeCommand('workbench.action.files.saveAll'); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); @@ -781,6 +791,9 @@ suite('metadata', () => { const activeCell = vscode.notebook.activeNotebookEditor!.selection; assert.equal(vscode.notebook.activeNotebookEditor!.document.cells.indexOf(activeCell!), 1); assert.equal(activeCell?.metadata.custom!['testCellMetadata'] as number, 123); + + await vscode.commands.executeCommand('workbench.action.files.saveAll'); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); }); @@ -808,7 +821,7 @@ suite('regression', () => { await vscode.commands.executeCommand('vscode.openWith', resource, 'default'); assert.equal(vscode.window.activeTextEditor?.document.uri.path, resource.path); - await vscode.commands.executeCommand('workbench.action.files.saveAll'); + await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); @@ -824,7 +837,7 @@ suite('regression', () => { assert.notEqual(vscode.notebook.activeNotebookEditor, undefined, 'notebook first'); assert.notEqual(vscode.window.activeTextEditor, undefined); - // await vscode.commands.executeCommand('workbench.action.files.saveAll'); + await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index d78dc231e8d3..a100bcf462ac 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -40,7 +40,7 @@ export class MainThreadNotebookDocument extends Disposable { ) { super(); - this._textModel = new NotebookTextModel(handle, viewType, supportBackup, uri); + this._textModel = new NotebookTextModel(handle, viewType, supportBackup, uri, undoRedoService); this._register(this._textModel.onDidModelChangeProxy(e => { this._proxy.$acceptModelChanged(this.uri, e); this._proxy.$acceptEditorPropertiesChanged(uri, { selections: { selections: this._textModel.selections }, metadata: null }); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index eec6514f8c51..e8b56e68a847 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -654,7 +654,7 @@ async function moveCell(context: INotebookCellActionContext, direction: 'up' | ' if (result) { // move cell command only works when the cell container has focus - await context.notebookEditor.focusNotebookCell(context.cell, 'container'); + await context.notebookEditor.focusNotebookCell(result, 'container'); } } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index ba5c8e903bd1..028aad61771a 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -210,17 +210,17 @@ export interface INotebookEditor extends IEditor { /** * Move a cell up one spot */ - moveCellUp(cell: ICellViewModel): Promise; + moveCellUp(cell: ICellViewModel): Promise; /** * Move a cell down one spot */ - moveCellDown(cell: ICellViewModel): Promise; + moveCellDown(cell: ICellViewModel): Promise; /** * Move a cell above or below another cell */ - moveCell(cell: ICellViewModel, relativeToCell: ICellViewModel, direction: 'above' | 'below'): Promise; + moveCell(cell: ICellViewModel, relativeToCell: ICellViewModel, direction: 'above' | 'below'): Promise; /** * Focus the container of a cell (the monaco editor inside is not focused). diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index d304dd048b21..9af17edbdeea 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -889,7 +889,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor (direction === 'above' ? index : nextIndex) : index; const newCell = this._notebookViewModel!.createCell(insertIndex, initialText.split(/\r?\n/g), language, type, cell?.metadata, true); - return newCell; + return newCell as CellViewModel; } async splitNotebookCell(cell: ICellViewModel): Promise { @@ -929,41 +929,41 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return true; } - async moveCellDown(cell: ICellViewModel): Promise { + async moveCellDown(cell: ICellViewModel): Promise { if (!this._notebookViewModel!.metadata.editable) { - return false; + return null; } const index = this._notebookViewModel!.getCellIndex(cell); if (index === this._notebookViewModel!.length - 1) { - return false; + return null; } const newIdx = index + 1; return this._moveCellToIndex(index, newIdx); } - async moveCellUp(cell: ICellViewModel): Promise { + async moveCellUp(cell: ICellViewModel): Promise { if (!this._notebookViewModel!.metadata.editable) { - return false; + return null; } const index = this._notebookViewModel!.getCellIndex(cell); if (index === 0) { - return false; + return null; } const newIdx = index - 1; return this._moveCellToIndex(index, newIdx); } - async moveCell(cell: ICellViewModel, relativeToCell: ICellViewModel, direction: 'above' | 'below'): Promise { + async moveCell(cell: ICellViewModel, relativeToCell: ICellViewModel, direction: 'above' | 'below'): Promise { if (!this._notebookViewModel!.metadata.editable) { - return false; + return null; } if (cell === relativeToCell) { - return false; + return null; } const originalIdx = this._notebookViewModel!.getCellIndex(cell); @@ -977,23 +977,24 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return this._moveCellToIndex(originalIdx, newIdx); } - private async _moveCellToIndex(index: number, newIdx: number): Promise { + private async _moveCellToIndex(index: number, newIdx: number): Promise { if (index === newIdx) { - return false; + return null; } if (!this._notebookViewModel!.moveCellToIdx(index, newIdx, true)) { throw new Error('Notebook Editor move cell, index out of range'); } - let r: (val: boolean) => void; + let r: (val: ICellViewModel | null) => void; DOM.scheduleAtNextAnimationFrame(() => { if (this._isDisposed) { - r(false); + r(null); } - this._list?.revealElementInView(this._notebookViewModel!.viewCells[newIdx]); - r(true); + const viewCell = this._notebookViewModel!.viewCells[newIdx]; + this._list?.revealElementInView(viewCell); + r(viewCell); }); return new Promise(resolve => { r = resolve; }); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts index f5bb93b389ee..8856fe60e994 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts @@ -94,14 +94,17 @@ export abstract class BaseCellViewModel extends Disposable { options: model.IModelDeltaDecoration; }>(); private _lastDecorationId: number = 0; - protected _textModel?: model.ITextModel; get textModel(): model.ITextModel | undefined { - return this._textModel; + return this.model.textModel; + } + + set textModel(m: model.ITextModel | undefined) { + this.model.textModel = m; } hasModel(): this is IEditableCellViewModel { - return !!this._textModel; + return !!this.model.textModel; } private _dragging: boolean = false; @@ -131,7 +134,7 @@ export abstract class BaseCellViewModel extends Disposable { abstract onDeselect(): void; assertTextModelAttached(): boolean { - if (this._textModel && this._textEditor && this._textEditor.getModel() === this._textModel) { + if (this.textModel && this._textEditor && this._textEditor.getModel() === this.textModel) { return true; } @@ -152,7 +155,7 @@ export abstract class BaseCellViewModel extends Disposable { } this._textEditor = editor; - this._textModel = this._textEditor.getModel() || undefined; + this.textModel = this._textEditor.getModel() || undefined; if (this._editorViewStates) { this._restoreViewState(this._editorViewStates); @@ -187,7 +190,7 @@ export abstract class BaseCellViewModel extends Disposable { }); this._textEditor = undefined; - this._textModel = undefined; + this.textModel = undefined; this._cursorChangeListener?.dispose(); this._cursorChangeListener = null; this._onDidChangeEditorAttachState.fire(); @@ -319,7 +322,7 @@ export abstract class BaseCellViewModel extends Disposable { } const firstViewLineTop = this._textEditor.getTopForPosition(1, 1); - const lastViewLineTop = this._textEditor.getTopForPosition(this._textModel!.getLineCount(), this._textModel!.getLineLength(this._textModel!.getLineCount())); + const lastViewLineTop = this._textEditor.getTopForPosition(this.textModel!.getLineCount(), this.textModel!.getLineLength(this.textModel!.getLineCount())); const selectionTop = this._textEditor.getTopForPosition(selection.startLineNumber, selection.startColumn); if (selectionTop === lastViewLineTop) { @@ -347,7 +350,7 @@ export abstract class BaseCellViewModel extends Disposable { let cellMatches: model.FindMatch[] = []; if (this.assertTextModelAttached()) { - cellMatches = this._textModel!.findMatches(value, false, false, false, null, false); + cellMatches = this.textModel!.findMatches(value, false, false, false, null, false); } else { const lineCount = this.textBuffer.getLineCount(); const fullRange = new Range(1, 1, lineCount, this.textBuffer.getLineLength(lineCount) + 1); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/cellEdit.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/cellEdit.ts index 030188a6a8fa..709c41859b2d 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/cellEdit.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/cellEdit.ts @@ -5,181 +5,24 @@ import { Range } from 'vs/editor/common/core/range'; import { Selection } from 'vs/editor/common/core/selection'; -import { ICell, CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { IResourceUndoRedoElement, UndoRedoElementType } from 'vs/platform/undoRedo/common/undoRedo'; import { URI } from 'vs/base/common/uri'; import { BaseCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel'; -import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { CellFocusMode } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; +import { ITextCellEditingDelegate } from 'vs/workbench/contrib/notebook/common/model/cellEdit'; -/** - * It should not modify Undo/Redo stack - */ -export interface ICellEditingDelegate { - insertCell?(index: number, viewCell: BaseCellViewModel): void; - deleteCell?(index: number): void; - moveCell?(fromIndex: number, toIndex: number): void; - createCellViewModel?(cell: ICell): BaseCellViewModel; + +export interface IViewCellEditingDelegate extends ITextCellEditingDelegate { + createCellViewModel?(cell: NotebookCellTextModel): BaseCellViewModel; createCell?(index: number, source: string | string[], language: string, type: CellKind): BaseCellViewModel; - setSelections(selections: number[]): void; -} - -export class InsertCellEdit implements IResourceUndoRedoElement { - type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; - label: string = 'Insert Cell'; - constructor( - public resource: URI, - private insertIndex: number, - private cell: BaseCellViewModel, - private editingDelegate: ICellEditingDelegate, - private beforedSelections: number[], - private endSelections: number[] - ) { - } - - undo(): void | Promise { - if (!this.editingDelegate.deleteCell) { - throw new Error('Notebook Delete Cell not implemented for Undo/Redo'); - } - - this.editingDelegate.deleteCell(this.insertIndex); - this.editingDelegate.setSelections(this.beforedSelections); - } - redo(): void | Promise { - if (!this.editingDelegate.insertCell) { - throw new Error('Notebook Insert Cell not implemented for Undo/Redo'); - } - - this.editingDelegate.insertCell(this.insertIndex, this.cell); - this.editingDelegate.setSelections(this.endSelections); - } -} - -export class DeleteCellEdit implements IResourceUndoRedoElement { - type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; - label: string = 'Delete Cell'; - - private _rawCell: ICell; - constructor( - public resource: URI, - private insertIndex: number, - cell: BaseCellViewModel, - private editingDelegate: ICellEditingDelegate, - private beforedSelections: number[], - private endSelections: number[] - ) { - this._rawCell = cell.model; - - // save inmem text to `ICell` - // no needed any more as the text buffer is transfered to `raw_cell` - // this._rawCell.source = [cell.getText()]; - } - - undo(): void | Promise { - if (!this.editingDelegate.insertCell || !this.editingDelegate.createCellViewModel) { - throw new Error('Notebook Insert Cell not implemented for Undo/Redo'); - } - - const cell = this.editingDelegate.createCellViewModel(this._rawCell); - this.editingDelegate.insertCell(this.insertIndex, cell); - this.editingDelegate.setSelections(this.beforedSelections); - } - - redo(): void | Promise { - if (!this.editingDelegate.deleteCell) { - throw new Error('Notebook Delete Cell not implemented for Undo/Redo'); - } - - this.editingDelegate.deleteCell(this.insertIndex); - this.editingDelegate.setSelections(this.endSelections); - } -} - -export class MoveCellEdit implements IResourceUndoRedoElement { - type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; - label: string = 'Delete Cell'; - - constructor( - public resource: URI, - private fromIndex: number, - private toIndex: number, - private editingDelegate: ICellEditingDelegate, - private beforedSelections: number[], - private endSelections: number[] - ) { - } - - undo(): void | Promise { - if (!this.editingDelegate.moveCell) { - throw new Error('Notebook Move Cell not implemented for Undo/Redo'); - } - - this.editingDelegate.moveCell(this.toIndex, this.fromIndex); - this.editingDelegate.setSelections(this.beforedSelections); - } - - redo(): void | Promise { - if (!this.editingDelegate.moveCell) { - throw new Error('Notebook Move Cell not implemented for Undo/Redo'); - } - - this.editingDelegate.moveCell(this.fromIndex, this.toIndex); - this.editingDelegate.setSelections(this.endSelections); - } -} - -export class SpliceCellsEdit implements IResourceUndoRedoElement { - type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; - label: string = 'Insert Cell'; - constructor( - public resource: URI, - private diffs: [number, CellViewModel[], CellViewModel[]][], - private editingDelegate: ICellEditingDelegate, - private beforeHandles: number[], - private endHandles: number[] - ) { - } - - undo(): void | Promise { - if (!this.editingDelegate.deleteCell || !this.editingDelegate.insertCell) { - throw new Error('Notebook Insert/Delete Cell not implemented for Undo/Redo'); - } - - this.diffs.forEach(diff => { - for (let i = 0; i < diff[2].length; i++) { - this.editingDelegate.deleteCell!(diff[0]); - } - - diff[1].reverse().forEach(cell => { - this.editingDelegate.insertCell!(diff[0], cell); - }); - }); - this.editingDelegate.setSelections(this.beforeHandles); - } - - redo(): void | Promise { - if (!this.editingDelegate.deleteCell || !this.editingDelegate.insertCell) { - throw new Error('Notebook Insert/Delete Cell not implemented for Undo/Redo'); - } - - this.diffs.reverse().forEach(diff => { - for (let i = 0; i < diff[1].length; i++) { - this.editingDelegate.deleteCell!(diff[0]); - } - - diff[2].reverse().forEach(cell => { - this.editingDelegate.insertCell!(diff[0], cell); - }); - }); - - this.editingDelegate.setSelections(this.endHandles); - } } export class JoinCellEdit implements IResourceUndoRedoElement { type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; label: string = 'Join Cell'; - private _deletedRawCell: ICell; + private _deletedRawCell: NotebookCellTextModel; constructor( public resource: URI, private index: number, @@ -189,7 +32,7 @@ export class JoinCellEdit implements IResourceUndoRedoElement { private inverseRange: Range, private insertContent: string, private removedCell: BaseCellViewModel, - private editingDelegate: ICellEditingDelegate, + private editingDelegate: IViewCellEditingDelegate, ) { this._deletedRawCell = this.removedCell.model; } @@ -209,12 +52,12 @@ export class JoinCellEdit implements IResourceUndoRedoElement { const cell = this.editingDelegate.createCellViewModel(this._deletedRawCell); if (this.direction === 'above') { - this.editingDelegate.insertCell(this.index, cell); - this.editingDelegate.setSelections([cell.handle]); + this.editingDelegate.insertCell(this.index, this._deletedRawCell); + this.editingDelegate.emitSelections([cell.handle]); cell.focusMode = CellFocusMode.Editor; } else { - this.editingDelegate.insertCell(this.index, cell); - this.editingDelegate.setSelections([this.cell.handle]); + this.editingDelegate.insertCell(this.index, cell.model); + this.editingDelegate.emitSelections([this.cell.handle]); this.cell.focusMode = CellFocusMode.Editor; } } @@ -230,7 +73,7 @@ export class JoinCellEdit implements IResourceUndoRedoElement { ]); this.editingDelegate.deleteCell(this.index); - this.editingDelegate.setSelections([this.cell.handle]); + this.editingDelegate.emitSelections([this.cell.handle]); this.cell.focusMode = CellFocusMode.Editor; } } @@ -247,13 +90,13 @@ export class SplitCellEdit implements IResourceUndoRedoElement { private cellContents: string[], private language: string, private cellKind: CellKind, - private editingDelegate: ICellEditingDelegate + private editingDelegate: IViewCellEditingDelegate ) { } async undo(): Promise { - if (!this.editingDelegate.deleteCell || !this.editingDelegate.createCellViewModel) { + if (!this.editingDelegate.deleteCell) { throw new Error('Notebook Delete Cell not implemented for Undo/Redo'); } @@ -270,12 +113,12 @@ export class SplitCellEdit implements IResourceUndoRedoElement { this.editingDelegate.deleteCell(this.index + 1); } - this.editingDelegate.setSelections([this.cell.handle]); + this.editingDelegate.emitSelections([this.cell.handle]); this.cell.focusMode = CellFocusMode.Editor; } async redo(): Promise { - if (!this.editingDelegate.insertCell || !this.editingDelegate.createCell) { + if (!this.editingDelegate.createCell) { throw new Error('Notebook Insert Cell not implemented for Undo/Redo'); } @@ -291,7 +134,7 @@ export class SplitCellEdit implements IResourceUndoRedoElement { } if (lastCell) { - this.editingDelegate.setSelections([lastCell.handle]); + this.editingDelegate.emitSelections([lastCell.handle]); lastCell.focusMode = CellFocusMode.Editor; } } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts index d6089f551504..33f6c1ef33ee 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts @@ -173,17 +173,17 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod * Text model is used for editing. */ async resolveTextModel(): Promise { - if (!this._textModel) { + if (!this.textModel) { const ref = await this._modelService.createModelReference(this.model.uri); - this._textModel = ref.object.textEditorModel; + this.textModel = ref.object.textEditorModel; this._register(ref); - this._register(this._textModel.onDidChangeContent(() => { + this._register(this.textModel.onDidChangeContent(() => { this.editState = CellEditState.Editing; this._onDidChangeState.fire({ contentChanged: true }); })); } - return this._textModel; + return this.textModel; } onDeselect() { diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts index 73f293399c3b..1c803a808f37 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts @@ -142,16 +142,16 @@ export class MarkdownCellViewModel extends BaseCellViewModel implements ICellVie } async resolveTextModel(): Promise { - if (!this._textModel) { + if (!this.textModel) { const ref = await this._modelService.createModelReference(this.model.uri); - this._textModel = ref.object.textEditorModel; + this.textModel = ref.object.textEditorModel; this._register(ref); - this._register(this._textModel.onDidChangeContent(() => { + this._register(this.textModel.onDidChangeContent(() => { this._html = null; this._onDidChangeState.fire({ contentChanged: true }); })); } - return this._textModel; + return this.textModel; } onDeselect() { diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index ac222a4a6c73..d19bd1f2e8ea 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -19,7 +19,6 @@ import { WorkspaceTextEdit } from 'vs/editor/common/modes'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; import { CellEditState, CellFindMatch, ICellRange, ICellViewModel, NotebookLayoutInfo, IEditableCellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { DeleteCellEdit, InsertCellEdit, MoveCellEdit, SpliceCellsEdit, JoinCellEdit, SplitCellEdit } from 'vs/workbench/contrib/notebook/browser/viewModel/cellEdit'; import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel'; import { NotebookEventDispatcher, NotebookMetadataChangedEvent } from 'vs/workbench/contrib/notebook/browser/viewModel/eventDispatcher'; import { CellFoldingState, EditorFoldingStateDelegate } from 'vs/workbench/contrib/notebook/browser/contrib/fold/foldingModel'; @@ -31,6 +30,8 @@ import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/no import { MarkdownRenderer } from 'vs/workbench/contrib/notebook/browser/view/renderers/mdRenderer'; import { dirname } from 'vs/base/common/resources'; import { IPosition, Position } from 'vs/editor/common/core/position'; +import { SplitCellEdit, JoinCellEdit } from 'vs/workbench/contrib/notebook/browser/viewModel/cellEdit'; +import { BaseCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel'; import { PieceTreeTextBuffer } from 'vs/editor/common/model/pieceTreeTextBuffer/pieceTreeTextBuffer'; export interface INotebookEditorViewState { @@ -259,23 +260,20 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD this._instanceId = strings.singleLetterHash(MODEL_ID); this._register(this._notebook.onDidChangeCells(e => { - const diffs = e.map(splice => { + const diffs = e.splices.map(splice => { return [splice[0], splice[1], splice[2].map(cell => { return createCellViewModel(this._instantiationService, this, cell as NotebookCellTextModel); })] as [number, number, CellViewModel[]]; }); - const undoDiff = diffs.map(diff => { - const deletedCells = this.viewCells.slice(diff[0], diff[0] + diff[1]); - - return [diff[0], deletedCells, diff[2]] as [number, CellViewModel[], CellViewModel[]]; - }); - diffs.reverse().forEach(diff => { const deletedCells = this._viewCells.splice(diff[0], diff[1], ...diff[2]); + this._decorationsTree.acceptReplace(diff[0], diff[1], diff[2].length, true); deletedCells.forEach(cell => { this._handleToViewCellMapping.delete(cell.handle); + // dispsoe the cell to release ref to the cell text document + cell.dispose(); }); diff[2].forEach(cell => { @@ -285,7 +283,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD }); this._onDidChangeViewCells.fire({ - synchronous: true, + synchronous: e.synchronous, splices: diffs }); @@ -315,12 +313,6 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } } - this._undoService.pushElement(new SpliceCellsEdit(this.uri, undoDiff, { - insertCell: this._insertCellDelegate.bind(this), - deleteCell: this._deleteCellDelegate.bind(this), - setSelections: this._setSelectionsDelegate.bind(this) - }, this.selectionHandles, endSelectionHandles)); - this.selectionHandles = endSelectionHandles; })); @@ -328,6 +320,13 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD this.eventDispatcher.emit([new NotebookMetadataChangedEvent(e)]); })); + this._register(this._notebook.emitSelections(selections => { + // text model emit selection change (for example, undo/redo) + // we should update the selection handle wisely + // TODO, if the editor is note selected, undo/redo should not change the focused element selection + this.selectionHandles = selections; + })); + this._register(this.eventDispatcher.onDidChangeLayout((e) => { this._layoutInfo = e.value; @@ -582,87 +581,20 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return result; } - private _createCellDelegate(index: number, source: string | string[], language: string, type: CellKind) { - const cell = this._notebook.createCellTextModel(source, language, type, [], undefined); - let newCell: CellViewModel = createCellViewModel(this._instantiationService, this, cell); - this._viewCells!.splice(index, 0, newCell); - this._handleToViewCellMapping.set(newCell.handle, newCell); - this._notebook.insertNewCell(index, [cell]); - this._localStore.add(newCell); - this._decorationsTree.acceptReplace(index, 0, 1, true); - this._onDidChangeViewCells.fire({ synchronous: true, splices: [[index, 0, [newCell]]] }); - return newCell; - } - - private _insertCellDelegate(insertIndex: number, insertCell: CellViewModel) { - this._viewCells!.splice(insertIndex, 0, insertCell); - this._handleToViewCellMapping.set(insertCell.handle, insertCell); - this._notebook.insertNewCell(insertIndex, [insertCell.model as NotebookCellTextModel]); - this._localStore.add(insertCell); - this._onDidChangeViewCells.fire({ synchronous: true, splices: [[insertIndex, 0, [insertCell]]] }); - } - - private _deleteCellDelegate(deleteIndex: number) { - const deleteCell = this._viewCells[deleteIndex]; - this._viewCells.splice(deleteIndex, 1); - this._handleToViewCellMapping.delete(deleteCell.handle); - - this._notebook.removeCell(deleteIndex, 1); - this._decorationsTree.acceptReplace(deleteIndex, 1, 0, true); - this._onDidChangeViewCells.fire({ synchronous: true, splices: [[deleteIndex, 1, []]] }); - } - - private _setSelectionsDelegate(selections: number[]) { - this.selectionHandles = selections; - } - createCell(index: number, source: string | string[], language: string, type: CellKind, metadata: NotebookCellMetadata | undefined, synchronous: boolean, pushUndoStop: boolean = true) { - const cell = this._notebook.createCellTextModel(source, language, type, [], metadata); - let newCell: CellViewModel = createCellViewModel(this._instantiationService, this, cell); - this._viewCells!.splice(index, 0, newCell); - this._handleToViewCellMapping.set(newCell.handle, newCell); - this._notebook.insertNewCell(index, [cell]); - this._localStore.add(newCell); - - if (pushUndoStop) { - this._undoService.pushElement(new InsertCellEdit(this.uri, index, newCell, { - insertCell: this._insertCellDelegate.bind(this), - deleteCell: this._deleteCellDelegate.bind(this), - setSelections: this._setSelectionsDelegate.bind(this) - }, this.selectionHandles, this.selectionHandles)); - } - - this._decorationsTree.acceptReplace(index, 0, 1, true); - this._onDidChangeViewCells.fire({ synchronous: synchronous, splices: [[index, 0, [newCell]]] }); - return newCell; + this._notebook.createCell2(index, source, language, type, metadata, synchronous, pushUndoStop, undefined, undefined); + // TODO, rely on createCell to be sync + return this.viewCells[index]; } - insertCell(index: number, cell: NotebookCellTextModel, synchronous: boolean): CellViewModel { - let newCell: CellViewModel = createCellViewModel(this._instantiationService, this, cell); - this._viewCells!.splice(index, 0, newCell); - this._handleToViewCellMapping.set(newCell.handle, newCell); - - this._notebook.insertNewCell(index, [newCell.model]); - this._localStore.add(newCell); - this._undoService.pushElement(new InsertCellEdit(this.uri, index, newCell, { - insertCell: this._insertCellDelegate.bind(this), - deleteCell: this._deleteCellDelegate.bind(this), - setSelections: this._setSelectionsDelegate.bind(this) - }, this.selectionHandles, this.selectionHandles)); - - this._decorationsTree.acceptReplace(index, 0, 1, true); - this._onDidChangeViewCells.fire({ synchronous: synchronous, splices: [[index, 0, [newCell]]] }); - return newCell; + insertCell(index: number, cell: NotebookCellTextModel, synchronous: boolean, pushUndoStop: boolean = true): CellViewModel { + this._notebook.insertCell2(index, cell, synchronous, pushUndoStop); + // TODO, rely on createCell to be sync // this will trigger it to synchronous update + return this._viewCells[index]; } deleteCell(index: number, synchronous: boolean, pushUndoStop: boolean = true) { const primarySelectionIndex = this.selectionHandles.length ? this._viewCells.indexOf(this.getCellByHandle(this.selectionHandles[0])!) : null; - - let viewCell = this._viewCells[index]; - this._viewCells.splice(index, 1); - this._handleToViewCellMapping.delete(viewCell.handle); - this._notebook.removeCell(index, 1); - let endSelections: number[] = []; if (this.selectionHandles.length) { const primarySelectionHandle = this.selectionHandles[0]; @@ -680,23 +612,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } } - if (pushUndoStop) { - this._undoService.pushElement(new DeleteCellEdit(this.uri, index, viewCell, { - insertCell: this._insertCellDelegate.bind(this), - deleteCell: this._deleteCellDelegate.bind(this), - createCellViewModel: (cell: NotebookCellTextModel) => { - return createCellViewModel(this._instantiationService, this, cell); - }, - setSelections: this._setSelectionsDelegate.bind(this) - }, this.selectionHandles, endSelections)); - } - - this.selectionHandles = endSelections; - - this._decorationsTree.acceptReplace(index, 1, 0, true); - - this._onDidChangeViewCells.fire({ synchronous: synchronous, splices: [[index, 1, []]] }); - viewCell.dispose(); + this._notebook.deleteCell2(index, synchronous, pushUndoStop, this.selectionHandles, endSelections); } moveCellToIdx(index: number, newIdx: number, synchronous: boolean, pushedToUndoStack: boolean = true): boolean { @@ -705,24 +621,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return false; } - this.viewCells.splice(index, 1); - this.viewCells!.splice(newIdx, 0, viewCell); - this._notebook.moveCellToIdx(index, newIdx); - - if (pushedToUndoStack) { - this._undoService.pushElement(new MoveCellEdit(this.uri, index, newIdx, { - moveCell: (fromIndex: number, toIndex: number) => { - this.moveCellToIdx(fromIndex, toIndex, true, false); - }, - setSelections: this._setSelectionsDelegate.bind(this) - }, this.selectionHandles, this.selectionHandles)); - } - - this.selectionHandles = this.selectionHandles; - - this._onDidChangeViewCells.fire({ synchronous: synchronous, splices: [[index, 1, []]] }); - this._onDidChangeViewCells.fire({ synchronous: synchronous, splices: [[newIdx, 0, [viewCell]]] }); - + this._notebook.moveCellToIdx2(index, newIdx, synchronous, pushedToUndoStack, undefined, [viewCell.handle]); return true; } @@ -806,23 +705,10 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD let newLinesContents = this._computeCellLinesContents(cell, splitPoints); if (newLinesContents) { - const editorSelections = cell.getSelections(); - // update the contents of the first cell - cell.textModel.applyEdits([ - { range: cell.textModel.getFullModelRange(), text: newLinesContents[0] } - ], false); - - // create new cells based on the new text models - const language = cell.model.language; + this._notebook.splitNotebookCell(index, newLinesContents, this.selectionHandles); + const language = cell.language; const kind = cell.cellKind; - let insertIndex = this.getCellIndex(cell) + 1; - const newCells = []; - for (let j = 1; j < newLinesContents.length; j++, insertIndex++) { - newCells.push(this.createCell(insertIndex, newLinesContents[j], language, kind, undefined, true, false)); - } - - this.selectionHandles = [cell.handle]; this._undoService.pushElement(new SplitCellEdit( this.uri, @@ -833,16 +719,17 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD language, kind, { - insertCell: this._insertCellDelegate.bind(this), - deleteCell: this._deleteCellDelegate.bind(this), - createCellViewModel: (cell: NotebookCellTextModel) => { - return createCellViewModel(this._instantiationService, this, cell); + createCell: (index: number, source: string | string[], language: string, type: CellKind) => { + return this.createCell(index, source, language, type, undefined, true, false) as BaseCellViewModel; }, - createCell: this._createCellDelegate.bind(this), - setSelections: this._setSelectionsDelegate.bind(this) + deleteCell: (index: number) => { + this.deleteCell(index, true, false); + }, + emitSelections: (selections: number[]) => { + this.selectionHandles = selections; + } } )); - return newCells; } } @@ -908,12 +795,18 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD insertContent, cell, { - insertCell: this._insertCellDelegate.bind(this), - deleteCell: this._deleteCellDelegate.bind(this), + insertCell: (index: number, cell: NotebookCellTextModel) => { + this.insertCell(index, cell, true, false); + }, + deleteCell: (index: number) => { + this.deleteCell(index, true, false); + }, createCellViewModel: (cell: NotebookCellTextModel) => { return createCellViewModel(this._instantiationService, this, cell); }, - setSelections: this._setSelectionsDelegate.bind(this) + emitSelections: (selections: number[]) => { + this.selectionHandles = selections; + } }) ); @@ -956,12 +849,18 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD insertContent, below, { - insertCell: this._insertCellDelegate.bind(this), - deleteCell: this._deleteCellDelegate.bind(this), + insertCell: (index: number, cell: NotebookCellTextModel) => { + this.insertCell(index, cell, true, false); + }, + deleteCell: (index: number) => { + this.deleteCell(index, true, false); + }, createCellViewModel: (cell: NotebookCellTextModel) => { return createCellViewModel(this._instantiationService, this, cell); }, - setSelections: this._setSelectionsDelegate.bind(this) + emitSelections: (selections: number[]) => { + this.selectionHandles = selections; + } }) ); diff --git a/src/vs/workbench/contrib/notebook/common/model/cellEdit.ts b/src/vs/workbench/contrib/notebook/common/model/cellEdit.ts new file mode 100644 index 000000000000..2bf2293f03cc --- /dev/null +++ b/src/vs/workbench/contrib/notebook/common/model/cellEdit.ts @@ -0,0 +1,184 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IResourceUndoRedoElement, UndoRedoElementType } from 'vs/platform/undoRedo/common/undoRedo'; +import { URI } from 'vs/base/common/uri'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; + +/** + * It should not modify Undo/Redo stack + */ +export interface ITextCellEditingDelegate { + insertCell?(index: number, cell: NotebookCellTextModel): void; + deleteCell?(index: number): void; + moveCell?(fromIndex: number, toIndex: number, beforeSelections: number[] | undefined, endSelections: number[] | undefined): void; + emitSelections(selections: number[]): void; +} + + +export class InsertCellEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = 'Insert Cell'; + constructor( + public resource: URI, + private insertIndex: number, + private cell: NotebookCellTextModel, + private editingDelegate: ITextCellEditingDelegate, + private beforedSelections: number[] | undefined, + private endSelections: number[] | undefined + ) { + } + + undo(): void | Promise { + if (!this.editingDelegate.deleteCell) { + throw new Error('Notebook Delete Cell not implemented for Undo/Redo'); + } + + this.editingDelegate.deleteCell(this.insertIndex); + if (this.beforedSelections) { + this.editingDelegate.emitSelections(this.beforedSelections); + } + } + redo(): void | Promise { + if (!this.editingDelegate.insertCell) { + throw new Error('Notebook Insert Cell not implemented for Undo/Redo'); + } + + this.editingDelegate.insertCell(this.insertIndex, this.cell); + if (this.endSelections) { + this.editingDelegate.emitSelections(this.endSelections); + } + } +} + +export class DeleteCellEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = 'Delete Cell'; + constructor( + public resource: URI, + private insertIndex: number, + private _cell: NotebookCellTextModel, + private editingDelegate: ITextCellEditingDelegate, + private beforedSelections: number[] | undefined, + private endSelections: number[] | undefined + ) { + + // save inmem text to `ICell` + // no needed any more as the text buffer is transfered to `raw_cell` + // this._rawCell.source = [cell.getText()]; + } + + undo(): void | Promise { + if (!this.editingDelegate.insertCell) { + throw new Error('Notebook Insert Cell not implemented for Undo/Redo'); + } + + this.editingDelegate.insertCell(this.insertIndex, this._cell); + if (this.beforedSelections) { + this.editingDelegate.emitSelections(this.beforedSelections); + } + } + + redo(): void | Promise { + if (!this.editingDelegate.deleteCell) { + throw new Error('Notebook Delete Cell not implemented for Undo/Redo'); + } + + this.editingDelegate.deleteCell(this.insertIndex); + if (this.endSelections) { + this.editingDelegate.emitSelections(this.endSelections); + } + } +} + +export class MoveCellEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = 'Delete Cell'; + + constructor( + public resource: URI, + private fromIndex: number, + private toIndex: number, + private editingDelegate: ITextCellEditingDelegate, + private beforedSelections: number[] | undefined, + private endSelections: number[] | undefined + ) { + } + + undo(): void | Promise { + if (!this.editingDelegate.moveCell) { + throw new Error('Notebook Move Cell not implemented for Undo/Redo'); + } + + this.editingDelegate.moveCell(this.toIndex, this.fromIndex, this.endSelections, this.beforedSelections); + if (this.beforedSelections) { + this.editingDelegate.emitSelections(this.beforedSelections); + } + } + + redo(): void | Promise { + if (!this.editingDelegate.moveCell) { + throw new Error('Notebook Move Cell not implemented for Undo/Redo'); + } + + this.editingDelegate.moveCell(this.fromIndex, this.toIndex, this.beforedSelections, this.endSelections); + if (this.endSelections) { + this.editingDelegate.emitSelections(this.endSelections); + } + } +} + +export class SpliceCellsEdit implements IResourceUndoRedoElement { + type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + label: string = 'Insert Cell'; + constructor( + public resource: URI, + private diffs: [number, NotebookCellTextModel[], NotebookCellTextModel[]][], + private editingDelegate: ITextCellEditingDelegate, + private beforeHandles: number[] | undefined, + private endHandles: number[] | undefined + ) { + } + + undo(): void | Promise { + if (!this.editingDelegate.deleteCell || !this.editingDelegate.insertCell) { + throw new Error('Notebook Insert/Delete Cell not implemented for Undo/Redo'); + } + + this.diffs.forEach(diff => { + for (let i = 0; i < diff[2].length; i++) { + this.editingDelegate.deleteCell!(diff[0]); + } + + diff[1].reverse().forEach(cell => { + this.editingDelegate.insertCell!(diff[0], cell); + }); + }); + + if (this.beforeHandles) { + this.editingDelegate.emitSelections(this.beforeHandles); + } + } + + redo(): void | Promise { + if (!this.editingDelegate.deleteCell || !this.editingDelegate.insertCell) { + throw new Error('Notebook Insert/Delete Cell not implemented for Undo/Redo'); + } + + this.diffs.reverse().forEach(diff => { + for (let i = 0; i < diff[1].length; i++) { + this.editingDelegate.deleteCell!(diff[0]); + } + + diff[2].reverse().forEach(cell => { + this.editingDelegate.insertCell!(diff[0], cell); + }); + }); + + if (this.endHandles) { + this.editingDelegate.emitSelections(this.endHandles); + } + } +} diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts index d9f2e3d35599..ccab2b085c3b 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Emitter, Event } from 'vs/base/common/event'; -import { ICell, IProcessedOutput, NotebookCellOutputsSplice, CellKind, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ICell, IProcessedOutput, NotebookCellOutputsSplice, CellKind, NotebookCellMetadata, NotebookDocumentMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { PieceTreeTextBufferBuilder } from 'vs/editor/common/model/pieceTreeTextBuffer/pieceTreeTextBufferBuilder'; import { URI } from 'vs/base/common/uri'; import * as model from 'vs/editor/common/model'; @@ -69,6 +69,16 @@ export class NotebookCellTextModel extends Disposable implements ICell { return this._textBuffer; } + private _textModel?: model.ITextModel; + + get textModel(): model.ITextModel | undefined { + return this._textModel; + } + + set textModel(m: model.ITextModel | undefined) { + this._textModel = m; + } + constructor( readonly uri: URI, public handle: number, @@ -109,4 +119,24 @@ export class NotebookCellTextModel extends Disposable implements ICell { this._onDidChangeOutputs.fire(splices); } + + getEvaluatedMetadata(documentMetadata: NotebookDocumentMetadata): NotebookCellMetadata { + const editable = this.metadata?.editable ?? + documentMetadata.cellEditable; + + const runnable = this.metadata?.runnable ?? + documentMetadata.cellRunnable; + + const hasExecutionOrder = this.metadata?.hasExecutionOrder ?? + documentMetadata.cellHasExecutionOrder; + + return { + ...(this.metadata || {}), + ...{ + editable, + runnable, + hasExecutionOrder + } + }; + } } diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index c3070a89d7cf..360a327a663b 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -9,6 +9,8 @@ import { URI } from 'vs/base/common/uri'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { INotebookTextModel, NotebookCellOutputsSplice, NotebookCellTextModelSplice, NotebookDocumentMetadata, NotebookCellMetadata, ICellEditOperation, CellEditType, CellUri, ICellInsertEdit, NotebookCellsChangedEvent, CellKind, IProcessedOutput, notebookDocumentMetadataDefaults, diff, ICellDeleteEdit, NotebookCellsChangeType, ICellDto2, IMainCellDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ITextSnapshot } from 'vs/editor/common/model'; +import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; +import { InsertCellEdit, DeleteCellEdit, MoveCellEdit, SpliceCellsEdit } from 'vs/workbench/contrib/notebook/common/model/cellEdit'; function compareRangesUsingEnds(a: [number, number], b: [number, number]): number { if (a[1] === b[1]) { @@ -68,8 +70,10 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel private readonly _onWillDispose: Emitter = this._register(new Emitter()); readonly onWillDispose: Event = this._onWillDispose.event; - private readonly _onDidChangeCells = new Emitter(); - get onDidChangeCells(): Event { return this._onDidChangeCells.event; } + private readonly _onDidChangeCells = new Emitter<{ synchronous: boolean, splices: NotebookCellTextModelSplice[] }>(); + get onDidChangeCells() { return this._onDidChangeCells.event; } + private readonly _emitSelections = new Emitter(); + get emitSelections() { return this._emitSelections.event; } private _onDidModelChangeProxy = new Emitter(); get onDidModelChangeProxy(): Event { return this._onDidModelChangeProxy.event; } private _onDidSelectionChangeProxy = new Emitter(); @@ -110,7 +114,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel public handle: number, public viewType: string, public supportBackup: boolean, - public uri: URI + public uri: URI, + private _undoService: IUndoRedoService ) { super(); this.cells = []; @@ -242,7 +247,19 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel }); } - this._onDidChangeCells.fire(diffs); + const undoDiff = diffs.map(diff => { + const deletedCells = this.cells.slice(diff[0], diff[0] + diff[1]); + + return [diff[0], deletedCells, diff[2]] as [number, NotebookCellTextModel[], NotebookCellTextModel[]]; + }); + + this._undoService.pushElement(new SpliceCellsEdit(this.uri, undoDiff, { + insertCell: this._insertCellDelegate.bind(this), + deleteCell: this._deleteCellDelegate.bind(this), + emitSelections: this._emitSelectionsDelegate.bind(this) + }, undefined, undefined)); + + this._onDidChangeCells.fire({ synchronous: true, splices: diffs }); return true; } @@ -446,6 +463,123 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.CellsClearOutput, versionId: this._versionId }); } + //#region Notebook Text Model Edit API + + private _insertCellDelegate(insertIndex: number, insertCell: NotebookCellTextModel) { + this.insertNewCell(insertIndex, [insertCell]); + this._onDidChangeCells.fire({ synchronous: true, splices: [[insertIndex, 0, [insertCell]]] }); + } + + private _deleteCellDelegate(deleteIndex: number) { + this.removeCell(deleteIndex, 1); + this._onDidChangeCells.fire({ synchronous: true, splices: [[deleteIndex, 1, []]] }); + } + + private _emitSelectionsDelegate(selections: number[]) { + this._emitSelections.fire(selections); + } + + createCell2(index: number, source: string | string[], language: string, type: CellKind, metadata: NotebookCellMetadata | undefined, synchronous: boolean, pushUndoStop: boolean, beforeSelections: number[] | undefined, endSelections: number[] | undefined) { + const cell = this.createCellTextModel(source, language, type, [], metadata); + + if (pushUndoStop) { + this._undoService.pushElement(new InsertCellEdit(this.uri, index, cell, { + insertCell: this._insertCellDelegate.bind(this), + deleteCell: this._deleteCellDelegate.bind(this), + emitSelections: this._emitSelectionsDelegate.bind(this) + }, beforeSelections, endSelections)); + } + + + this.insertNewCell(index, [cell]); + + this._onDidChangeCells.fire({ synchronous, splices: [[index, 0, [cell]]] }); + + if (endSelections) { + this._emitSelections.fire(endSelections); + } + return cell; + } + + insertCell2(index: number, cell: NotebookCellTextModel, synchronous: boolean, pushUndoStop: boolean): void { + if (pushUndoStop) { + this._undoService.pushElement(new InsertCellEdit(this.uri, index, cell, { + insertCell: this._insertCellDelegate.bind(this), + deleteCell: this._deleteCellDelegate.bind(this), + emitSelections: this._emitSelectionsDelegate.bind(this) + }, undefined, undefined)); + } + + this.insertNewCell(index, [cell]); + this._onDidChangeCells.fire({ synchronous: synchronous, splices: [[index, 0, [cell]]] }); + } + + deleteCell2(index: number, synchronous: boolean, pushUndoStop: boolean, beforeSelections: number[] | undefined, endSelections: number[] | undefined) { + const cell = this.cells[index]; + if (pushUndoStop) { + this._undoService.pushElement(new DeleteCellEdit(this.uri, index, cell, { + insertCell: this._insertCellDelegate.bind(this), + deleteCell: this._deleteCellDelegate.bind(this), + emitSelections: this._emitSelectionsDelegate.bind(this) + }, beforeSelections, endSelections)); + } + + this.removeCell(index, 1); + this._onDidChangeCells.fire({ synchronous: synchronous, splices: [[index, 1, []]] }); + if (endSelections) { + this._emitSelections.fire(endSelections); + } + } + + moveCellToIdx2(index: number, newIdx: number, synchronous: boolean, pushedToUndoStack: boolean, beforeSelections: number[] | undefined, endSelections: number[] | undefined): boolean { + const cell = this.cells[index]; + if (pushedToUndoStack) { + this._undoService.pushElement(new MoveCellEdit(this.uri, index, newIdx, { + moveCell: (fromIndex: number, toIndex: number, beforeSelections: number[] | undefined, endSelections: number[] | undefined) => { + this.moveCellToIdx2(fromIndex, toIndex, true, false, beforeSelections, endSelections); + }, + emitSelections: this._emitSelectionsDelegate.bind(this) + }, beforeSelections, endSelections)); + } + + this.moveCellToIdx(index, newIdx); + // todo, we can't emit this change as it will create a new view model and that will hold + // a new reference to the document, thus + this._onDidChangeCells.fire({ synchronous: synchronous, splices: [[index, 1, []]] }); + this._onDidChangeCells.fire({ synchronous: synchronous, splices: [[newIdx, 0, [cell]]] }); + if (endSelections) { + this._emitSelections.fire(endSelections); + } + + return true; + } + + async splitNotebookCell(index: number, newLinesContents: string[], endSelections: number[]) { + const cell = this.cells[index]; + + if (!cell.textModel) { + return; + } + + cell.textModel.applyEdits([ + { range: cell.textModel.getFullModelRange(), text: newLinesContents[0] } + ], false); + + // create new cells based on the new text models + const language = cell.language; + const kind = cell.cellKind; + let insertIndex = index + 1; + const newCells = []; + for (let j = 1; j < newLinesContents.length; j++, insertIndex++) { + newCells.push(this.createCell2(insertIndex, newLinesContents[j], language, kind, undefined, true, false, undefined, undefined)); + } + + if (endSelections) { + this._emitSelections.fire(endSelections); + } + } + //#endregion + dispose() { this._onWillDispose.fire(); this._cellListeners.forEach(val => val.dispose()); diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index e69fb76a155e..5d585f07c341 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -269,7 +269,7 @@ export interface INotebookTextModel { languages: string[]; cells: ICell[]; renderers: Set; - onDidChangeCells?: Event; + onDidChangeCells?: Event<{ synchronous: boolean, splices: NotebookCellTextModelSplice[] }>; onDidChangeContent: Event; onWillDispose(listener: () => void): IDisposable; } diff --git a/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts index ba8b2228b3af..5892c87147f3 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts @@ -23,7 +23,7 @@ suite('NotebookViewModel', () => { instantiationService.spy(IUndoRedoService, 'pushElement'); test('ctor', function () { - const notebook = new NotebookTextModel(0, 'notebook', false, URI.parse('test')); + const notebook = new NotebookTextModel(0, 'notebook', false, URI.parse('test'), undoRedoService); const model = new NotebookEditorTestModel(notebook); const eventDispatcher = new NotebookEventDispatcher(); const viewModel = new NotebookViewModel('notebook', model.notebook, eventDispatcher, null, instantiationService, blukEditService, undoRedoService); diff --git a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts index 475b54342dce..af796e0707bd 100644 --- a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts @@ -122,15 +122,15 @@ export class TestNotebookEditor implements INotebookEditor { throw new Error('Method not implemented.'); } - moveCellDown(cell: CellViewModel): Promise { + moveCellDown(cell: CellViewModel): Promise { throw new Error('Method not implemented.'); } - moveCellUp(cell: CellViewModel): Promise { + moveCellUp(cell: CellViewModel): Promise { throw new Error('Method not implemented.'); } - moveCell(cell: ICellViewModel, relativeToCell: ICellViewModel, direction: 'above' | 'below'): Promise { + moveCell(cell: ICellViewModel, relativeToCell: ICellViewModel, direction: 'above' | 'below'): Promise { throw new Error('Method not implemented.'); } @@ -305,7 +305,7 @@ export class NotebookEditorTestModel extends EditorModel implements INotebookEdi export function withTestNotebook(instantiationService: IInstantiationService, blukEditService: IBulkEditService, undoRedoService: IUndoRedoService, cells: [string[], string, CellKind, IProcessedOutput[], NotebookCellMetadata][], callback: (editor: TestNotebookEditor, viewModel: NotebookViewModel, textModel: NotebookTextModel) => void) { const viewType = 'notebook'; const editor = new TestNotebookEditor(); - const notebook = new NotebookTextModel(0, viewType, false, URI.parse('test')); + const notebook = new NotebookTextModel(0, viewType, false, URI.parse('test'), undoRedoService); notebook.cells = cells.map((cell, index) => { return new NotebookCellTextModel(notebook.uri, index, cell[0], cell[1], cell[2], cell[3], cell[4]); }); From b91c21b80ea65af62e1b975c6e7449858002fca9 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 11:28:12 -0700 Subject: [PATCH 02/16] protect list and webview from revert. --- src/vs/vscode.proposed.d.ts | 2 +- .../notebook/browser/notebookBrowser.ts | 1 + .../notebook/browser/notebookEditor.ts | 4 + .../notebook/browser/notebookEditorWidget.ts | 13 ++- .../browser/notebookEditorWidgetService.ts | 3 +- .../notebook/browser/view/notebookCellList.ts | 83 +++++++++++-------- .../view/renderers/backLayerWebView.ts | 4 + 7 files changed, 66 insertions(+), 44 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index b98f9591efdf..8cdb4b1ad326 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1693,7 +1693,7 @@ declare module 'vscode' { }): Promise; saveNotebook(document: NotebookDocument, cancellation: CancellationToken): Promise; saveNotebookAs(targetResource: Uri, document: NotebookDocument, cancellation: CancellationToken): Promise; - readonly onDidChangeNotebook: Event; + readonly onDidChangeNotebook: Event; backupNotebook(document: NotebookDocument, context: NotebookDocumentBackupContext, cancellation: CancellationToken): Promise; kernel?: NotebookKernel; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 028aad61771a..0e8b5d310b52 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -356,6 +356,7 @@ export interface INotebookEditor extends IEditor { } export interface INotebookCellList { + isDisposed: boolean readonly contextKeyService: IContextKeyService; elementAt(position: number): ICellViewModel | undefined; elementHeight(element: ICellViewModel): number; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts index 9e099bc2c10d..bf7471ed02a8 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts @@ -188,6 +188,10 @@ export class NotebookEditor extends BaseEditor { private _saveEditorViewState(input: IEditorInput | undefined): void { if (this.group && this._widget.value && input instanceof NotebookEditorInput) { + if (this._widget.value.isDisposed) { + return; + } + const state = this._widget.value.getEditorViewState(); this._editorMemento.saveEditorState(this.group, input.resource, state); } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 9af17edbdeea..b7abbb089eb7 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -685,14 +685,13 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor const focus = this._list.getFocus()[0]; if (typeof focus === 'number') { const element = this._notebookViewModel!.viewCells[focus]; - const itemDOM = this._list?.domElementOfElement(element!); - let editorFocused = false; - if (document.activeElement && itemDOM && itemDOM.contains(document.activeElement)) { - editorFocused = true; - } + if (element) { + const itemDOM = this._list?.domElementOfElement(element); + let editorFocused = !!(document.activeElement && itemDOM && itemDOM.contains(document.activeElement)); - state.editorFocused = editorFocused; - state.focus = focus; + state.editorFocused = editorFocused; + state.focus = focus; + } } } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidgetService.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidgetService.ts index 1ae1d6808796..2369eee6a024 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidgetService.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidgetService.ts @@ -92,8 +92,9 @@ class NotebookEditorWidgetService implements INotebookEditorWidgetService { private _disposeWidget(widget: NotebookEditorWidget): void { widget.onWillHide(); - widget.getDomNode().remove(); + const domNode = widget.getDomNode(); widget.dispose(); + domNode.remove(); } private _freeWidget(input: NotebookEditorInput, source: IEditorGroup, target: IEditorGroup): void { diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index eb4a13b0da07..7256429f0f70 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -45,6 +45,12 @@ export class NotebookCellList extends WorkbenchList implements ID private _hiddenRangeIds: string[] = []; private hiddenRangesPrefixSum: PrefixSumComputer | null = null; + private _isDisposed = false; + + get isDisposed() { + return this._isDisposed; + } + constructor( private listUser: string, container: HTMLElement, @@ -162,43 +168,28 @@ export class NotebookCellList extends WorkbenchList implements ID attachViewModel(model: NotebookViewModel) { this._viewModel = model; this._viewModelStore.add(model.onDidChangeViewCells((e) => { - const currentRanges = this._hiddenRangeIds.map(id => this._viewModel!.getTrackedRange(id)).filter(range => range !== null) as ICellRange[]; - const newVisibleViewCells: CellViewModel[] = getVisibleCells(this._viewModel!.viewCells as CellViewModel[], currentRanges); + DOM.scheduleAtNextAnimationFrame(() => { + if (this._isDisposed) { + return; + } - const oldVisibleViewCells: CellViewModel[] = []; - const oldViewCellMapping = new Set(); - for (let i = 0; i < this.length; i++) { - oldVisibleViewCells.push(this.element(i)); - oldViewCellMapping.add(this.element(i).uri.toString()); - } + const currentRanges = this._hiddenRangeIds.map(id => this._viewModel!.getTrackedRange(id)).filter(range => range !== null) as ICellRange[]; + const newVisibleViewCells: CellViewModel[] = getVisibleCells(this._viewModel!.viewCells as CellViewModel[], currentRanges); - const viewDiffs = diff(oldVisibleViewCells, newVisibleViewCells, a => { - return oldViewCellMapping.has(a.uri.toString()); - }); + const oldVisibleViewCells: CellViewModel[] = []; + const oldViewCellMapping = new Set(); + for (let i = 0; i < this.length; i++) { + oldVisibleViewCells.push(this.element(i)); + oldViewCellMapping.add(this.element(i).uri.toString()); + } - if (e.synchronous) { - viewDiffs.reverse().forEach((diff) => { - // remove output in the webview - const hideOutputs: IProcessedOutput[] = []; - const deletedOutputs: IProcessedOutput[] = []; - - for (let i = diff.start; i < diff.start + diff.deleteCount; i++) { - const cell = this.element(i); - if (this._viewModel!.hasCell(cell.handle)) { - hideOutputs.push(...cell?.model.outputs); - } else { - deletedOutputs.push(...cell?.model.outputs); - } - } - - this.splice2(diff.start, diff.deleteCount, diff.toInsert); - - hideOutputs.forEach(output => this._onDidHideOutput.fire(output)); - deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); + const viewDiffs = diff(oldVisibleViewCells, newVisibleViewCells, a => { + return oldViewCellMapping.has(a.uri.toString()); }); - } else { - DOM.scheduleAtNextAnimationFrame(() => { + + if (e.synchronous) { viewDiffs.reverse().forEach((diff) => { + // remove output in the webview const hideOutputs: IProcessedOutput[] = []; const deletedOutputs: IProcessedOutput[] = []; @@ -216,8 +207,29 @@ export class NotebookCellList extends WorkbenchList implements ID hideOutputs.forEach(output => this._onDidHideOutput.fire(output)); deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); }); - }); - } + } else { + DOM.scheduleAtNextAnimationFrame(() => { + viewDiffs.reverse().forEach((diff) => { + const hideOutputs: IProcessedOutput[] = []; + const deletedOutputs: IProcessedOutput[] = []; + + for (let i = diff.start; i < diff.start + diff.deleteCount; i++) { + const cell = this.element(i); + if (this._viewModel!.hasCell(cell.handle)) { + hideOutputs.push(...cell?.model.outputs); + } else { + deletedOutputs.push(...cell?.model.outputs); + } + } + + this.splice2(diff.start, diff.deleteCount, diff.toInsert); + + hideOutputs.forEach(output => this._onDidHideOutput.fire(output)); + deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); + }); + }); + } + }); })); this._viewModelStore.add(model.onDidChangeSelection(() => { @@ -490,7 +502,7 @@ export class NotebookCellList extends WorkbenchList implements ID domElementOfElement(element: ICellViewModel): HTMLElement | null { const index = this._getViewIndexUpperBound(element); - if (index !== undefined) { + if (index !== undefined && index >= 0) { return this.view.domElement(index); } @@ -874,6 +886,7 @@ export class NotebookCellList extends WorkbenchList implements ID } dispose() { + this._isDisposed = true; this._viewModelStore.dispose(); this._localDisposableStore.dispose(); super.dispose(); diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index 128bfdb6fc04..b5dd31db1d8a 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -318,6 +318,10 @@ ${loaderJs} } async initialize(content: string) { + if (!document.body.contains(this.element)) { + throw new Error('Element is already detached from the DOM tree'); + } + this.webview = this._createInset(this.webviewService, content); this.webview.mountTo(this.element); this._register(this.webview); From 5fb0d9192c8afc32d5034a80b2e19c0d5a89c40f Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 12:19:12 -0700 Subject: [PATCH 03/16] render should honor sync flag. --- .../notebook/browser/view/notebookCellList.ts | 80 ++++++++++--------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 7256429f0f70..832dd971d2ef 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -168,28 +168,51 @@ export class NotebookCellList extends WorkbenchList implements ID attachViewModel(model: NotebookViewModel) { this._viewModel = model; this._viewModelStore.add(model.onDidChangeViewCells((e) => { - DOM.scheduleAtNextAnimationFrame(() => { - if (this._isDisposed) { - return; - } + if (this._isDisposed) { + return; + } - const currentRanges = this._hiddenRangeIds.map(id => this._viewModel!.getTrackedRange(id)).filter(range => range !== null) as ICellRange[]; - const newVisibleViewCells: CellViewModel[] = getVisibleCells(this._viewModel!.viewCells as CellViewModel[], currentRanges); + const currentRanges = this._hiddenRangeIds.map(id => this._viewModel!.getTrackedRange(id)).filter(range => range !== null) as ICellRange[]; + const newVisibleViewCells: CellViewModel[] = getVisibleCells(this._viewModel!.viewCells as CellViewModel[], currentRanges); - const oldVisibleViewCells: CellViewModel[] = []; - const oldViewCellMapping = new Set(); - for (let i = 0; i < this.length; i++) { - oldVisibleViewCells.push(this.element(i)); - oldViewCellMapping.add(this.element(i).uri.toString()); - } + const oldVisibleViewCells: CellViewModel[] = []; + const oldViewCellMapping = new Set(); + for (let i = 0; i < this.length; i++) { + oldVisibleViewCells.push(this.element(i)); + oldViewCellMapping.add(this.element(i).uri.toString()); + } - const viewDiffs = diff(oldVisibleViewCells, newVisibleViewCells, a => { - return oldViewCellMapping.has(a.uri.toString()); + const viewDiffs = diff(oldVisibleViewCells, newVisibleViewCells, a => { + return oldViewCellMapping.has(a.uri.toString()); + }); + + if (e.synchronous) { + viewDiffs.reverse().forEach((diff) => { + // remove output in the webview + const hideOutputs: IProcessedOutput[] = []; + const deletedOutputs: IProcessedOutput[] = []; + + for (let i = diff.start; i < diff.start + diff.deleteCount; i++) { + const cell = this.element(i); + if (this._viewModel!.hasCell(cell.handle)) { + hideOutputs.push(...cell?.model.outputs); + } else { + deletedOutputs.push(...cell?.model.outputs); + } + } + + this.splice2(diff.start, diff.deleteCount, diff.toInsert); + + hideOutputs.forEach(output => this._onDidHideOutput.fire(output)); + deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); }); + } else { + DOM.scheduleAtNextAnimationFrame(() => { + if (this._isDisposed) { + return; + } - if (e.synchronous) { viewDiffs.reverse().forEach((diff) => { - // remove output in the webview const hideOutputs: IProcessedOutput[] = []; const deletedOutputs: IProcessedOutput[] = []; @@ -207,29 +230,8 @@ export class NotebookCellList extends WorkbenchList implements ID hideOutputs.forEach(output => this._onDidHideOutput.fire(output)); deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); }); - } else { - DOM.scheduleAtNextAnimationFrame(() => { - viewDiffs.reverse().forEach((diff) => { - const hideOutputs: IProcessedOutput[] = []; - const deletedOutputs: IProcessedOutput[] = []; - - for (let i = diff.start; i < diff.start + diff.deleteCount; i++) { - const cell = this.element(i); - if (this._viewModel!.hasCell(cell.handle)) { - hideOutputs.push(...cell?.model.outputs); - } else { - deletedOutputs.push(...cell?.model.outputs); - } - } - - this.splice2(diff.start, diff.deleteCount, diff.toInsert); - - hideOutputs.forEach(output => this._onDidHideOutput.fire(output)); - deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); - }); - }); - } - }); + }); + } })); this._viewModelStore.add(model.onDidChangeSelection(() => { From 3da356053341faa424f2ba4b8309c8eec632d9c7 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 12:21:51 -0700 Subject: [PATCH 04/16] dispose event listeners. --- .../notebook/browser/notebookEditorWidget.ts | 6 +++--- .../browser/viewModel/notebookViewModel.ts | 4 ++-- .../notebook/common/model/notebookTextModel.ts | 14 +++++++------- .../notebook/test/notebookTextModel.test.ts | 10 +++++----- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index b7abbb089eb7..f77a9689f286 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -105,10 +105,10 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return this._isDisposed; } - private readonly _onDidChangeModel = new Emitter(); + private readonly _onDidChangeModel = this._register(new Emitter()); readonly onDidChangeModel: Event = this._onDidChangeModel.event; - private readonly _onDidFocusEditorWidget = new Emitter(); + private readonly _onDidFocusEditorWidget = this._register(new Emitter()); readonly onDidFocusEditorWidget = this._onDidFocusEditorWidget.event; set viewModel(newModel: NotebookViewModel | undefined) { @@ -129,7 +129,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor } private _activeKernel: INotebookKernelInfo | undefined = undefined; - private readonly _onDidChangeKernel = new Emitter(); + private readonly _onDidChangeKernel = this._register(new Emitter()); readonly onDidChangeKernel: Event = this._onDidChangeKernel.event; get activeKernel() { diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index d19bd1f2e8ea..2d498a3f5265 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -200,7 +200,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return this._notebook.metadata; } - private readonly _onDidChangeViewCells = new Emitter(); + private readonly _onDidChangeViewCells = this._register(new Emitter()); get onDidChangeViewCells(): Event { return this._onDidChangeViewCells.event; } private _lastNotebookEditResource: URI[] = []; @@ -216,7 +216,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return this._layoutInfo; } - private readonly _onDidChangeSelection = new Emitter(); + private readonly _onDidChangeSelection = this._register(new Emitter()); get onDidChangeSelection(): Event { return this._onDidChangeSelection.event; } private _selections: number[] = []; diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index 360a327a663b..a8b7dee12430 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -70,17 +70,17 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel private readonly _onWillDispose: Emitter = this._register(new Emitter()); readonly onWillDispose: Event = this._onWillDispose.event; - private readonly _onDidChangeCells = new Emitter<{ synchronous: boolean, splices: NotebookCellTextModelSplice[] }>(); + private readonly _onDidChangeCells = this._register(new Emitter<{ synchronous: boolean, splices: NotebookCellTextModelSplice[] }>()); get onDidChangeCells() { return this._onDidChangeCells.event; } - private readonly _emitSelections = new Emitter(); + private readonly _emitSelections = this._register(new Emitter()); get emitSelections() { return this._emitSelections.event; } - private _onDidModelChangeProxy = new Emitter(); + private _onDidModelChangeProxy = this._register(new Emitter()); get onDidModelChangeProxy(): Event { return this._onDidModelChangeProxy.event; } - private _onDidSelectionChangeProxy = new Emitter(); + private _onDidSelectionChangeProxy = this._register(new Emitter()); get onDidSelectionChange(): Event { return this._onDidSelectionChangeProxy.event; } - private _onDidChangeContent = new Emitter(); + private _onDidChangeContent = this._register(new Emitter()); onDidChangeContent: Event = this._onDidChangeContent.event; - private _onDidChangeMetadata = new Emitter(); + private _onDidChangeMetadata = this._register(new Emitter()); onDidChangeMetadata: Event = this._onDidChangeMetadata.event; private _mapping: Map = new Map(); private _cellListeners: Map = new Map(); @@ -170,7 +170,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._increaseVersionId(); } - $applyEdit(modelVersionId: number, rawEdits: ICellEditOperation[], emitToExtHost: boolean = true): boolean { + $applyEdit(modelVersionId: number, rawEdits: ICellEditOperation[], emitToExtHost: boolean): boolean { if (modelVersionId !== this._versionId) { return false; } diff --git a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts index 2bad51a304a4..c4e5345e5280 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts @@ -31,7 +31,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, { editType: CellEditType.Insert, index: 3, cells: [new TestCell(viewModel.viewType, 6, ['var f = 6;'], 'javascript', CellKind.Code, [])] }, - ]); + ], true); assert.equal(textModel.cells.length, 6); @@ -56,7 +56,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 6, ['var f = 6;'], 'javascript', CellKind.Code, [])] }, - ]); + ], true); assert.equal(textModel.cells.length, 6); @@ -81,7 +81,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Delete, index: 1, count: 1 }, { editType: CellEditType.Delete, index: 3, count: 1 }, - ]); + ], true); assert.equal(textModel.cells[0].getValue(), 'var a = 1;'); assert.equal(textModel.cells[1].getValue(), 'var c = 3;'); @@ -104,7 +104,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Delete, index: 1, count: 1 }, { editType: CellEditType.Insert, index: 3, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, - ]); + ], true); assert.equal(textModel.cells.length, 4); @@ -129,7 +129,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Delete, index: 1, count: 1 }, { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, - ]); + ], true); assert.equal(textModel.cells.length, 4); assert.equal(textModel.cells[0].getValue(), 'var a = 1;'); From ac9e97aaa5e3c7fee02b6364174bc0ab88c80284 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 13:52:59 -0700 Subject: [PATCH 05/16] delay cells change from revert. --- .../src/notebook.test.ts | 2 +- .../api/browser/mainThreadNotebook.ts | 20 ++++++++++++++----- .../notebook/browser/view/notebookCellList.ts | 4 ++-- .../common/model/notebookTextModel.ts | 4 ++-- .../notebook/test/notebookTextModel.test.ts | 10 +++++----- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index d0f4cfc2d500..47de754f27ed 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -56,7 +56,7 @@ async function splitEditor() { await once; } -suite('API tests', () => { +suite('Notebook API tests', () => { test('document open/close event', async function () { const resource = vscode.Uri.file(join(vscode.workspace.rootPath || '', './first.vsctestnb')); const firstDocumentOpen = getEventOncePromise(vscode.notebook.onDidOpenNotebookDocument); diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index a100bcf462ac..ac04045c4cb0 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as nls from 'vs/nls'; +import * as DOM from 'vs/base/browser/dom'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { MainContext, MainThreadNotebookShape, NotebookExtensionDescription, IExtHostContext, ExtHostNotebookShape, ExtHostContext, INotebookDocumentsAndEditorsDelta, INotebookModelAddedData } from '../common/extHost.protocol'; import { Disposable, IDisposable, combinedDisposable } from 'vs/base/common/lifecycle'; @@ -51,9 +52,18 @@ export class MainThreadNotebookDocument extends Disposable { })); } - async applyEdit(modelVersionId: number, edits: ICellEditOperation[], emitToExtHost: boolean): Promise { + async applyEdit(modelVersionId: number, edits: ICellEditOperation[], emitToExtHost: boolean, synchronous: boolean): Promise { await this.notebookService.transformEditsOutputs(this.textModel, edits); - return this._textModel.$applyEdit(modelVersionId, edits); + if (synchronous) { + return this._textModel.$applyEdit(modelVersionId, edits, emitToExtHost, synchronous); + } else { + return new Promise(resolve => { + this._register(DOM.scheduleAtNextAnimationFrame(() => { + const ret = this._textModel.$applyEdit(modelVersionId, edits, emitToExtHost, true); + resolve(ret); + })); + }); + } } async spliceNotebookCellOutputs(cellHandle: number, splices: NotebookCellOutputsSplice[]) { @@ -533,7 +543,7 @@ export class MainThreadNotebookController implements IMainNotebookController { await mainthreadNotebook.applyEdit(mainthreadNotebook.textModel.versionId, [ { editType: CellEditType.Delete, count: mainthreadNotebook.textModel.cells.length, index: 0 }, { editType: CellEditType.Insert, index: 0, cells: data.cells } - ], true); + ], true, false); } return mainthreadNotebook.textModel; } @@ -553,7 +563,7 @@ export class MainThreadNotebookController implements IMainNotebookController { index: 0, cells: backup.cells || [] } - ], false); + ], false, true); // create document in ext host with cells data await this._mainThreadNotebook.addNotebookDocument({ @@ -630,7 +640,7 @@ export class MainThreadNotebookController implements IMainNotebookController { let mainthreadNotebook = this._mapping.get(URI.from(resource).toString()); if (mainthreadNotebook) { - return await mainthreadNotebook.applyEdit(modelVersionId, edits, true); + return await mainthreadNotebook.applyEdit(modelVersionId, edits, true, true); } return false; diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 832dd971d2ef..0b6fcb09e7e1 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -207,7 +207,7 @@ export class NotebookCellList extends WorkbenchList implements ID deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); }); } else { - DOM.scheduleAtNextAnimationFrame(() => { + this._viewModelStore.add(DOM.scheduleAtNextAnimationFrame(() => { if (this._isDisposed) { return; } @@ -230,7 +230,7 @@ export class NotebookCellList extends WorkbenchList implements ID hideOutputs.forEach(output => this._onDidHideOutput.fire(output)); deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output)); }); - }); + })); } })); diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index a8b7dee12430..f8716f2ee146 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -170,7 +170,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._increaseVersionId(); } - $applyEdit(modelVersionId: number, rawEdits: ICellEditOperation[], emitToExtHost: boolean): boolean { + $applyEdit(modelVersionId: number, rawEdits: ICellEditOperation[], emitToExtHost: boolean, synchronous: boolean): boolean { if (modelVersionId !== this._versionId) { return false; } @@ -259,7 +259,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel emitSelections: this._emitSelectionsDelegate.bind(this) }, undefined, undefined)); - this._onDidChangeCells.fire({ synchronous: true, splices: diffs }); + this._onDidChangeCells.fire({ synchronous: synchronous, splices: diffs }); return true; } diff --git a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts index c4e5345e5280..84f55c863311 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts @@ -31,7 +31,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, { editType: CellEditType.Insert, index: 3, cells: [new TestCell(viewModel.viewType, 6, ['var f = 6;'], 'javascript', CellKind.Code, [])] }, - ], true); + ], true, true); assert.equal(textModel.cells.length, 6); @@ -56,7 +56,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 6, ['var f = 6;'], 'javascript', CellKind.Code, [])] }, - ], true); + ], true, true); assert.equal(textModel.cells.length, 6); @@ -81,7 +81,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Delete, index: 1, count: 1 }, { editType: CellEditType.Delete, index: 3, count: 1 }, - ], true); + ], true, true); assert.equal(textModel.cells[0].getValue(), 'var a = 1;'); assert.equal(textModel.cells[1].getValue(), 'var c = 3;'); @@ -104,7 +104,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Delete, index: 1, count: 1 }, { editType: CellEditType.Insert, index: 3, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, - ], true); + ], true, true); assert.equal(textModel.cells.length, 4); @@ -129,7 +129,7 @@ suite('NotebookTextModel', () => { textModel.$applyEdit(textModel.versionId, [ { editType: CellEditType.Delete, index: 1, count: 1 }, { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, ['var e = 5;'], 'javascript', CellKind.Code, [])] }, - ], true); + ], true, true); assert.equal(textModel.cells.length, 4); assert.equal(textModel.cells[0].getValue(), 'var a = 1;'); From 38776fa77933844ead21a10484b3e7c94443e970 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 17:14:18 -0700 Subject: [PATCH 06/16] track focus state in notebook view model. --- .../notebook/browser/notebookEditorWidget.ts | 4 +++- .../browser/viewModel/notebookViewModel.ts | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index f77a9689f286..d5793d134938 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -217,7 +217,9 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor updateEditorFocus() { // Note - focus going to the webview will fire 'blur', but the webview element will be // a descendent of the notebook editor root. - this._editorFocus?.set(DOM.isAncestor(document.activeElement, this._overlayContainer)); + const focused = DOM.isAncestor(document.activeElement, this._overlayContainer); + this._editorFocus?.set(focused); + this._notebookViewModel?.setFocus(focused); } hasFocus() { diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index 2d498a3f5265..d17b107bd9f0 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -243,6 +243,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD public readonly id: string; private _foldingRanges: FoldingRegions | null = null; private _hiddenRanges: ICellRange[] = []; + private _focused: boolean = false; constructor( public viewType: string, @@ -324,7 +325,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD // text model emit selection change (for example, undo/redo) // we should update the selection handle wisely // TODO, if the editor is note selected, undo/redo should not change the focused element selection - this.selectionHandles = selections; + this.updateSelectionsFromEdits(selections); })); this._register(this.eventDispatcher.onDidChangeLayout((e) => { @@ -352,6 +353,16 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD }); } + setFocus(focused: boolean) { + this._focused = focused; + } + + updateSelectionsFromEdits(selections: number[]) { + if (this._focused) { + this.selectionHandles = selections; + } + } + getFoldingStartIndex(index: number): number { if (!this._foldingRanges) { return -1; @@ -726,7 +737,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD this.deleteCell(index, true, false); }, emitSelections: (selections: number[]) => { - this.selectionHandles = selections; + this.updateSelectionsFromEdits(selections); } } )); @@ -805,7 +816,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return createCellViewModel(this._instantiationService, this, cell); }, emitSelections: (selections: number[]) => { - this.selectionHandles = selections; + this.updateSelectionsFromEdits(selections); } }) ); @@ -859,7 +870,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return createCellViewModel(this._instantiationService, this, cell); }, emitSelections: (selections: number[]) => { - this.selectionHandles = selections; + this.updateSelectionsFromEdits(selections); } }) ); From 7a8b616ff08dced43d3bb2adc1192d61adc4898b Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 18:38:27 -0700 Subject: [PATCH 07/16] update focus state properly --- .../contrib/notebook/browser/notebookEditor.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts index f9332ddbdd1c..0a7f89eb694e 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditor.ts @@ -6,7 +6,7 @@ import * as DOM from 'vs/base/browser/dom'; import { CancellationToken } from 'vs/base/common/cancellation'; import { Emitter, Event } from 'vs/base/common/event'; -import { MutableDisposable, DisposableStore } from 'vs/base/common/lifecycle'; +import { DisposableStore } from 'vs/base/common/lifecycle'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IStorageService } from 'vs/platform/storage/common/storage'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; @@ -30,7 +30,7 @@ export class NotebookEditor extends BaseEditor { static readonly ID: string = 'workbench.editor.notebook'; private readonly _editorMemento: IEditorMemento; - private readonly _groupListener = this._register(new MutableDisposable()); + private readonly _groupListener = this._register(new DisposableStore()); private readonly _widgetDisposableStore: DisposableStore = new DisposableStore(); private _widget: IBorrowValue = { value: undefined }; private _rootElement!: HTMLElement; @@ -49,13 +49,13 @@ export class NotebookEditor extends BaseEditor { @IInstantiationService private readonly instantiationService: IInstantiationService, @IStorageService storageService: IStorageService, @IEditorService private readonly _editorService: IEditorService, - @IEditorGroupsService editorGroupService: IEditorGroupsService, + @IEditorGroupsService private readonly _editorGroupService: IEditorGroupsService, @IEditorDropService private readonly _editorDropService: IEditorDropService, @INotificationService private readonly _notificationService: INotificationService, @INotebookEditorWidgetService private readonly _notebookWidgetService: INotebookEditorWidgetService, ) { super(NotebookEditor.ID, telemetryService, themeService, storageService); - this._editorMemento = this.getEditorMemento(editorGroupService, NOTEBOOK_EDITOR_VIEW_STATE_PREFERENCE_KEY); + this._editorMemento = this.getEditorMemento(_editorGroupService, NOTEBOOK_EDITOR_VIEW_STATE_PREFERENCE_KEY); } set viewModel(newModel: NotebookViewModel | undefined) { @@ -100,7 +100,14 @@ export class NotebookEditor extends BaseEditor { setEditorVisible(visible: boolean, group: IEditorGroup | undefined): void { super.setEditorVisible(visible, group); - this._groupListener.value = group?.onWillCloseEditor(e => this._saveEditorViewState(e.editor)); + if (group) { + this._groupListener.add(group.onWillCloseEditor(e => this._saveEditorViewState(e.editor))); + this._groupListener.add(group.onDidGroupChange(() => { + if (this._editorGroupService.activeGroup !== group) { + this._widget?.value?.updateEditorFocus(); + } + })); + } if (!visible) { this._saveEditorViewState(this.input); From f92962c689c7c0cb761dc64a0f4ef737277a2225 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 19:02:25 -0700 Subject: [PATCH 08/16] revert selections syncing based on focus --- .../contrib/notebook/browser/viewModel/notebookViewModel.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index d17b107bd9f0..6d8d78ee66f7 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -358,9 +358,9 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } updateSelectionsFromEdits(selections: number[]) { - if (this._focused) { - this.selectionHandles = selections; - } + // if (this._focused) { + this.selectionHandles = selections; + // } } getFoldingStartIndex(index: number): number { From 765681974d5f30307a92e9640026454d96b70372 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 19 Jun 2020 19:11:24 -0700 Subject: [PATCH 09/16] :lipstick: --- .../vscode-notebook-tests/src/notebook.test.ts | 12 ++++++++++++ .../notebook/browser/viewModel/notebookViewModel.ts | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index 47de754f27ed..422c886e7774 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -57,6 +57,18 @@ async function splitEditor() { } suite('Notebook API tests', () => { + // test.only('crash', async function () { + // for (let i = 0; i < 200; i++) { + // let resource = vscode.Uri.file(join(vscode.workspace.rootPath || '', './first.vsctestnb')); + // await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); + // await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); + + // resource = vscode.Uri.file(join(vscode.workspace.rootPath || '', './empty.vsctestnb')); + // await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); + // await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); + // } + // }); + test('document open/close event', async function () { const resource = vscode.Uri.file(join(vscode.workspace.rootPath || '', './first.vsctestnb')); const firstDocumentOpen = getEventOncePromise(vscode.notebook.onDidOpenNotebookDocument); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index 6d8d78ee66f7..c8b32efca191 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -245,6 +245,10 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD private _hiddenRanges: ICellRange[] = []; private _focused: boolean = false; + get focused() { + return this._focused; + } + constructor( public viewType: string, private _notebook: NotebookTextModel, From d317dd4cdd9d0d710ee286371529cdf520cdf17b Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 21 Jun 2020 13:19:09 -0700 Subject: [PATCH 10/16] update selections only when focused. --- .../notebook/browser/viewModel/notebookViewModel.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index c8b32efca191..d20832222145 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -243,7 +243,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD public readonly id: string; private _foldingRanges: FoldingRegions | null = null; private _hiddenRanges: ICellRange[] = []; - private _focused: boolean = false; + private _focused: boolean = true; get focused() { return this._focused; @@ -362,9 +362,9 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } updateSelectionsFromEdits(selections: number[]) { - // if (this._focused) { - this.selectionHandles = selections; - // } + if (this._focused) { + this.selectionHandles = selections; + } } getFoldingStartIndex(index: number): number { From 0ac03e433121bbeabbdc4003ca0e48ac44d9b767 Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 21 Jun 2020 13:34:16 -0700 Subject: [PATCH 11/16] avoid events if notebook webview is disposed. --- .../notebook/browser/notebookEditorWidget.ts | 4 +++- .../browser/view/renderers/backLayerWebView.ts | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index d5793d134938..563a11270198 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -1230,6 +1230,9 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor dispose() { this._isDisposed = true; + // dispose webview first + this._webview?.dispose(); + this.notebookService.removeNotebookEditor(this); const keys = Object.keys(this._contributions); for (let i = 0, len = keys.length; i < len; i++) { @@ -1239,7 +1242,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor this._localStore.clear(); this._list?.dispose(); - this._webview?.dispose(); this._overlayContainer.remove(); this.viewModel?.dispose(); diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index b5dd31db1d8a..3f4b36b13b08 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -327,6 +327,10 @@ ${loaderJs} this._register(this.webview); this._register(this.webview.onDidClickLink(link => { + if (this._disposed) { + return; + } + if (!link) { return; } @@ -338,6 +342,10 @@ ${loaderJs} })); this._register(this.webview.onDidReload(() => { + if (this._disposed) { + return; + } + this.preloadsCache.clear(); for (const [output, inset] of this.insetMapping.entries()) { this.updateRendererPreloads(inset.preloads); @@ -346,6 +354,10 @@ ${loaderJs} })); this._register(this.webview.onMessage((data: IMessage) => { + if (this._disposed) { + return; + } + if (data.__vscode_notebook_message) { if (data.type === 'dimension') { let height = data.data.height; @@ -718,6 +730,10 @@ ${loaderJs} } private _sendMessageToWebview(message: ToWebviewMessage) { + if (this._disposed) { + return; + } + this.webview.postMessage(message); } @@ -727,6 +743,7 @@ ${loaderJs} dispose() { this._disposed = true; + this.webview.dispose(); super.dispose(); } } From df3d767b898bb5368c6b35883dd45f65fcbb5309 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 22 Jun 2020 16:30:09 -0700 Subject: [PATCH 12/16] more crash testing --- .../vscode-notebook-tests/src/notebook.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index 422c886e7774..a48052541e93 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -69,6 +69,19 @@ suite('Notebook API tests', () => { // } // }); + // test.only('crash', async function () { + // for (let i = 0; i < 200; i++) { + // let resource = vscode.Uri.file(join(vscode.workspace.rootPath || '', './first.vsctestnb')); + // await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); + // await vscode.commands.executeCommand('workbench.action.files.save'); + // await vscode.commands.executeCommand('workbench.action.closeAllEditors'); + // resource = vscode.Uri.file(join(vscode.workspace.rootPath || '', './empty.vsctestnb')); + // await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); + // await vscode.commands.executeCommand('workbench.action.files.save'); + // await vscode.commands.executeCommand('workbench.action.closeAllEditors'); + // } + // }); + test('document open/close event', async function () { const resource = vscode.Uri.file(join(vscode.workspace.rootPath || '', './first.vsctestnb')); const firstDocumentOpen = getEventOncePromise(vscode.notebook.onDidOpenNotebookDocument); From 5a7a96b52f57dc07580ee908bc2d1437f2b8d8fe Mon Sep 17 00:00:00 2001 From: rebornix Date: Tue, 23 Jun 2020 11:33:34 -0700 Subject: [PATCH 13/16] fix another index out of range. --- .../workbench/contrib/notebook/browser/view/notebookCellList.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 9def8a69c1db..e4490c709c95 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -532,7 +532,7 @@ export class NotebookCellList extends WorkbenchList implements ID updateElementHeight2(element: ICellViewModel, size: number): void { const index = this._getViewIndexUpperBound(element); - if (index === undefined) { + if (index === undefined || index < 0 || index >= this.length) { return; } From e648db1c9f799253e0b287beef9a3f8c395177b2 Mon Sep 17 00:00:00 2001 From: rebornix Date: Tue, 23 Jun 2020 14:01:18 -0700 Subject: [PATCH 14/16] force to use iframe for notebook. --- .../view/renderers/backLayerWebView.ts | 30 +++++++++++++++---- .../contrib/webview/browser/webviewElement.ts | 3 +- .../contrib/webview/browser/webviewService.ts | 2 +- .../electron-browser/webviewService.ts | 2 +- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index 9a5b49d233a9..eaf88e3cdd0f 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -18,7 +18,7 @@ import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookB import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel'; import { CellOutputKind, IProcessedOutput } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; -import { IWebviewService, WebviewElement } from 'vs/workbench/contrib/webview/browser/webview'; +import { WebviewElement, WebviewOptions, WebviewContentOptions, WebviewExtensionDescription } from 'vs/workbench/contrib/webview/browser/webview'; import { asWebviewUri } from 'vs/workbench/contrib/webview/common/webviewUri'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { dirname, joinPath } from 'vs/base/common/resources'; @@ -29,6 +29,9 @@ import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; import { IFileService } from 'vs/platform/files/common/files'; import { VSBuffer } from 'vs/base/common/buffer'; import { getExtensionForMimeType } from 'vs/base/common/mime'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { IFrameWebview } from 'vs/workbench/contrib/webview/browser/webviewElement'; +import { WebviewThemeDataProvider } from 'vs/workbench/contrib/webview/browser/themeing'; export interface WebviewIntialized { __vscode_notebook_message: boolean; @@ -225,12 +228,12 @@ export class BackLayerWebView extends Disposable { private _loaded!: Promise; private _initalized?: Promise; private _disposed = false; + private _webviewThemeDataProvider: WebviewThemeDataProvider; constructor( public notebookEditor: INotebookEditor, public id: string, public documentUri: URI, - @IWebviewService readonly webviewService: IWebviewService, @IOpenerService readonly openerService: IOpenerService, @INotebookService private readonly notebookService: INotebookService, @IEnvironmentService private readonly environmentService: IEnvironmentService, @@ -238,6 +241,7 @@ export class BackLayerWebView extends Disposable { @IWorkbenchEnvironmentService private readonly workbenchEnvironmentService: IWorkbenchEnvironmentService, @IFileDialogService private readonly fileDialogService: IFileDialogService, @IFileService private readonly fileService: IFileService, + @IInstantiationService private readonly instantiationService: IInstantiationService, ) { super(); @@ -247,7 +251,21 @@ export class BackLayerWebView extends Disposable { this.element.style.height = '1400px'; this.element.style.position = 'absolute'; this.element.style.margin = `0px 0 0px ${CELL_MARGIN + CELL_RUN_GUTTER}px`; + + this._webviewThemeDataProvider = this.instantiationService.createInstance(WebviewThemeDataProvider); + this._register(this._webviewThemeDataProvider); } + + createWebviewElement( + id: string, + options: WebviewOptions, + contentOptions: WebviewContentOptions, + extension: WebviewExtensionDescription | undefined, + ): WebviewElement { + return this.instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider, true); + } + + generateContent(outputNodePadding: number, coreDependencies: string, baseUrl: string) { return html` @@ -344,7 +362,7 @@ ${loaderJs} throw new Error('Element is already detached from the DOM tree'); } - this.webview = this._createInset(this.webviewService, content); + this.webview = this._createInset(content); this.webview.mountTo(this.element); this._register(this.webview); @@ -480,13 +498,13 @@ ${loaderJs} await this.openerService.open(newFileUri); } - private _createInset(webviewService: IWebviewService, content: string) { + private _createInset(content: string) { const rootPath = URI.file(path.dirname(getPathFromAmdModule(require, ''))); const workspaceFolders = this.contextService.getWorkspace().folders.map(x => x.uri); this.localResourceRootsCache = [...this.notebookService.getNotebookProviderResourceRoots(), ...workspaceFolders, rootPath]; - const webview = webviewService.createWebviewElement(this.id, { + const webview = this.createWebviewElement(this.id, { enableFindWidget: false, }, { allowMultipleAPIAcquire: true, @@ -767,7 +785,7 @@ ${loaderJs} dispose() { this._disposed = true; - this.webview.dispose(); + this.webview?.dispose(); super.dispose(); } } diff --git a/src/vs/workbench/contrib/webview/browser/webviewElement.ts b/src/vs/workbench/contrib/webview/browser/webviewElement.ts index 86f12969b72a..2f90e1eb35bb 100644 --- a/src/vs/workbench/contrib/webview/browser/webviewElement.ts +++ b/src/vs/workbench/contrib/webview/browser/webviewElement.ts @@ -34,6 +34,7 @@ export class IFrameWebview extends BaseWebview implements Web contentOptions: WebviewContentOptions, extension: WebviewExtensionDescription | undefined, webviewThemeDataProvider: WebviewThemeDataProvider, + private readonly forceUsingExternalEndpoint: boolean, @ITunnelService tunnelService: ITunnelService, @IFileService private readonly fileService: IFileService, @IRequestService private readonly requestService: IRequestService, @@ -91,7 +92,7 @@ export class IFrameWebview extends BaseWebview implements Web } private get useExternalEndpoint(): boolean { - return isWeb || this._configurationService.getValue('webview.experimental.useExternalEndpoint'); + return this.forceUsingExternalEndpoint || isWeb || this._configurationService.getValue('webview.experimental.useExternalEndpoint'); } public mountTo(parent: HTMLElement) { diff --git a/src/vs/workbench/contrib/webview/browser/webviewService.ts b/src/vs/workbench/contrib/webview/browser/webviewService.ts index 688b513948d2..51a8fa3f414d 100644 --- a/src/vs/workbench/contrib/webview/browser/webviewService.ts +++ b/src/vs/workbench/contrib/webview/browser/webviewService.ts @@ -30,7 +30,7 @@ export class WebviewService implements IWebviewService { contentOptions: WebviewContentOptions, extension: WebviewExtensionDescription | undefined, ): WebviewElement { - return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider); + return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider, false); } createWebviewOverlay( diff --git a/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts b/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts index 037ba636611d..7e9fafe3bd4d 100644 --- a/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts +++ b/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts @@ -34,7 +34,7 @@ export class ElectronWebviewService implements IWebviewService { ): WebviewElement { const useExternalEndpoint = this._configService.getValue('webview.experimental.useExternalEndpoint'); if (useExternalEndpoint) { - return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider); + return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider, false); } else { return this._instantiationService.createInstance(ElectronWebviewBasedWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider); } From 0cca8611cb707c524951a1207668f930c63ef9e1 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 24 Jun 2020 09:56:52 -0700 Subject: [PATCH 15/16] Revert "force to use iframe for notebook." This reverts commit e648db1c9f799253e0b287beef9a3f8c395177b2. --- .../view/renderers/backLayerWebView.ts | 30 ++++--------------- .../contrib/webview/browser/webviewElement.ts | 3 +- .../contrib/webview/browser/webviewService.ts | 2 +- .../electron-browser/webviewService.ts | 2 +- 4 files changed, 9 insertions(+), 28 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index eaf88e3cdd0f..9a5b49d233a9 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -18,7 +18,7 @@ import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookB import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel'; import { CellOutputKind, IProcessedOutput } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; -import { WebviewElement, WebviewOptions, WebviewContentOptions, WebviewExtensionDescription } from 'vs/workbench/contrib/webview/browser/webview'; +import { IWebviewService, WebviewElement } from 'vs/workbench/contrib/webview/browser/webview'; import { asWebviewUri } from 'vs/workbench/contrib/webview/common/webviewUri'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { dirname, joinPath } from 'vs/base/common/resources'; @@ -29,9 +29,6 @@ import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; import { IFileService } from 'vs/platform/files/common/files'; import { VSBuffer } from 'vs/base/common/buffer'; import { getExtensionForMimeType } from 'vs/base/common/mime'; -import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { IFrameWebview } from 'vs/workbench/contrib/webview/browser/webviewElement'; -import { WebviewThemeDataProvider } from 'vs/workbench/contrib/webview/browser/themeing'; export interface WebviewIntialized { __vscode_notebook_message: boolean; @@ -228,12 +225,12 @@ export class BackLayerWebView extends Disposable { private _loaded!: Promise; private _initalized?: Promise; private _disposed = false; - private _webviewThemeDataProvider: WebviewThemeDataProvider; constructor( public notebookEditor: INotebookEditor, public id: string, public documentUri: URI, + @IWebviewService readonly webviewService: IWebviewService, @IOpenerService readonly openerService: IOpenerService, @INotebookService private readonly notebookService: INotebookService, @IEnvironmentService private readonly environmentService: IEnvironmentService, @@ -241,7 +238,6 @@ export class BackLayerWebView extends Disposable { @IWorkbenchEnvironmentService private readonly workbenchEnvironmentService: IWorkbenchEnvironmentService, @IFileDialogService private readonly fileDialogService: IFileDialogService, @IFileService private readonly fileService: IFileService, - @IInstantiationService private readonly instantiationService: IInstantiationService, ) { super(); @@ -251,21 +247,7 @@ export class BackLayerWebView extends Disposable { this.element.style.height = '1400px'; this.element.style.position = 'absolute'; this.element.style.margin = `0px 0 0px ${CELL_MARGIN + CELL_RUN_GUTTER}px`; - - this._webviewThemeDataProvider = this.instantiationService.createInstance(WebviewThemeDataProvider); - this._register(this._webviewThemeDataProvider); } - - createWebviewElement( - id: string, - options: WebviewOptions, - contentOptions: WebviewContentOptions, - extension: WebviewExtensionDescription | undefined, - ): WebviewElement { - return this.instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider, true); - } - - generateContent(outputNodePadding: number, coreDependencies: string, baseUrl: string) { return html` @@ -362,7 +344,7 @@ ${loaderJs} throw new Error('Element is already detached from the DOM tree'); } - this.webview = this._createInset(content); + this.webview = this._createInset(this.webviewService, content); this.webview.mountTo(this.element); this._register(this.webview); @@ -498,13 +480,13 @@ ${loaderJs} await this.openerService.open(newFileUri); } - private _createInset(content: string) { + private _createInset(webviewService: IWebviewService, content: string) { const rootPath = URI.file(path.dirname(getPathFromAmdModule(require, ''))); const workspaceFolders = this.contextService.getWorkspace().folders.map(x => x.uri); this.localResourceRootsCache = [...this.notebookService.getNotebookProviderResourceRoots(), ...workspaceFolders, rootPath]; - const webview = this.createWebviewElement(this.id, { + const webview = webviewService.createWebviewElement(this.id, { enableFindWidget: false, }, { allowMultipleAPIAcquire: true, @@ -785,7 +767,7 @@ ${loaderJs} dispose() { this._disposed = true; - this.webview?.dispose(); + this.webview.dispose(); super.dispose(); } } diff --git a/src/vs/workbench/contrib/webview/browser/webviewElement.ts b/src/vs/workbench/contrib/webview/browser/webviewElement.ts index 2f90e1eb35bb..86f12969b72a 100644 --- a/src/vs/workbench/contrib/webview/browser/webviewElement.ts +++ b/src/vs/workbench/contrib/webview/browser/webviewElement.ts @@ -34,7 +34,6 @@ export class IFrameWebview extends BaseWebview implements Web contentOptions: WebviewContentOptions, extension: WebviewExtensionDescription | undefined, webviewThemeDataProvider: WebviewThemeDataProvider, - private readonly forceUsingExternalEndpoint: boolean, @ITunnelService tunnelService: ITunnelService, @IFileService private readonly fileService: IFileService, @IRequestService private readonly requestService: IRequestService, @@ -92,7 +91,7 @@ export class IFrameWebview extends BaseWebview implements Web } private get useExternalEndpoint(): boolean { - return this.forceUsingExternalEndpoint || isWeb || this._configurationService.getValue('webview.experimental.useExternalEndpoint'); + return isWeb || this._configurationService.getValue('webview.experimental.useExternalEndpoint'); } public mountTo(parent: HTMLElement) { diff --git a/src/vs/workbench/contrib/webview/browser/webviewService.ts b/src/vs/workbench/contrib/webview/browser/webviewService.ts index 51a8fa3f414d..688b513948d2 100644 --- a/src/vs/workbench/contrib/webview/browser/webviewService.ts +++ b/src/vs/workbench/contrib/webview/browser/webviewService.ts @@ -30,7 +30,7 @@ export class WebviewService implements IWebviewService { contentOptions: WebviewContentOptions, extension: WebviewExtensionDescription | undefined, ): WebviewElement { - return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider, false); + return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider); } createWebviewOverlay( diff --git a/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts b/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts index 7e9fafe3bd4d..037ba636611d 100644 --- a/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts +++ b/src/vs/workbench/contrib/webview/electron-browser/webviewService.ts @@ -34,7 +34,7 @@ export class ElectronWebviewService implements IWebviewService { ): WebviewElement { const useExternalEndpoint = this._configService.getValue('webview.experimental.useExternalEndpoint'); if (useExternalEndpoint) { - return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider, false); + return this._instantiationService.createInstance(IFrameWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider); } else { return this._instantiationService.createInstance(ElectronWebviewBasedWebview, id, options, contentOptions, extension, this._webviewThemeDataProvider); } From 8a08b757f8a0a4b3aeca3b0c1b9cb05357471ca4 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 25 Jun 2020 15:03:06 -0700 Subject: [PATCH 16/16] fix #100992. --- .../browser/view/output/transforms/richTransform.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts index 4cfc80e8ef11..360dce577041 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts @@ -61,7 +61,12 @@ class RichRenderer implements IOutputTransformContribution { let mimeTypesMessage = mimeTypes.join(', '); - contentNode.innerText = `No renderer could be found for output. It has the following MIME types: ${mimeTypesMessage}`; + if (preferredMimeType) { + contentNode.innerText = `No renderer could be found for MIME type: ${preferredMimeType}`; + } else { + contentNode.innerText = `No renderer could be found for output. It has the following MIME types: ${mimeTypesMessage}`; + } + container.appendChild(contentNode); return {