From 5e75d958ecff8191d69ddf337acee4e11e2c5cee Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 11 Jun 2021 14:52:34 -0700 Subject: [PATCH] remove nbvm.setSelections. --- .../test/cellOperations.test.ts | 42 ++++++++++++------- .../contrib/clipboard/notebookClipboard.ts | 2 +- .../notebook/browser/notebookBrowser.ts | 17 ++++---- .../notebook/browser/notebookEditorWidget.ts | 30 ++++++++++++- .../browser/view/renderers/codeCell.ts | 4 +- .../browser/view/renderers/markdownCell.ts | 2 +- .../browser/viewModel/notebookViewModel.ts | 4 -- .../notebook/test/testNotebookEditor.ts | 16 ++++++- 8 files changed, 83 insertions(+), 34 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/test/cellOperations.test.ts b/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/test/cellOperations.test.ts index 095ab2e39ff..8396257a6fc 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/test/cellOperations.test.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/test/cellOperations.test.ts @@ -282,7 +282,8 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 0, end: 1 }, [{ start: 0, end: 1 }]); + editor.setFocus({ start: 0, end: 1 }); + editor.setSelections([{ start: 0, end: 1 }]); runDeleteAction(viewModel, viewModel.cellAt(0)!); assert.strictEqual(viewModel.length, 2); }); @@ -297,7 +298,8 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 0, end: 1 }, [{ start: 0, end: 2 }]); + editor.setFocus({ start: 0, end: 1 }); + editor.setSelections([{ start: 0, end: 2 }]); runDeleteAction(viewModel, viewModel.cellAt(0)!); assert.strictEqual(viewModel.length, 1); }); @@ -313,7 +315,8 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 0, end: 1 }, [{ start: 2, end: 4 }]); + editor.setFocus({ start: 0, end: 1 }); + editor.setSelections([{ start: 2, end: 4 }]); runDeleteAction(viewModel, viewModel.cellAt(0)!); assert.strictEqual(viewModel.length, 3); }); @@ -328,7 +331,8 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 0, end: 1 }, [{ start: 0, end: 1 }]); + editor.setFocus({ start: 0, end: 1 }); + editor.setSelections([{ start: 0, end: 1 }]); runDeleteAction(viewModel, viewModel.cellAt(2)!); assert.strictEqual(viewModel.length, 2); assert.strictEqual(viewModel.cellAt(0)?.getText(), 'var a = 1;'); @@ -347,10 +351,11 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 0, end: 1 }, [{ start: 0, end: 1 }, { start: 3, end: 5 }]); + editor.setFocus({ start: 0, end: 1 }); + editor.setSelections([{ start: 0, end: 1 }, { start: 3, end: 5 }]); runDeleteAction(viewModel, viewModel.cellAt(1)!); assert.strictEqual(viewModel.length, 4); - assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(editor.getFocus(), { start: 0, end: 1 }); assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }, { start: 2, end: 4 }]); }); }); @@ -366,10 +371,11 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 0, end: 1 }, [{ start: 2, end: 3 }]); + editor.setFocus({ start: 0, end: 1 }); + editor.setSelections([{ start: 2, end: 3 }]); runDeleteAction(viewModel, viewModel.cellAt(0)!); assert.strictEqual(viewModel.length, 4); - assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(editor.getFocus(), { start: 0, end: 1 }); assert.deepStrictEqual(viewModel.getSelections(), [{ start: 1, end: 2 }]); }); }); @@ -385,10 +391,11 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 2, end: 3 }, [{ start: 3, end: 5 }]); + editor.setFocus({ start: 2, end: 3 }); + editor.setSelections([{ start: 3, end: 5 }]); runDeleteAction(viewModel, viewModel.cellAt(0)!); assert.strictEqual(viewModel.length, 4); - assert.deepStrictEqual(viewModel.getFocus(), { start: 1, end: 2 }); + assert.deepStrictEqual(editor.getFocus(), { start: 1, end: 2 }); assert.deepStrictEqual(viewModel.getSelections(), [{ start: 2, end: 4 }]); }); }); @@ -403,10 +410,11 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 2, end: 3 }, [{ start: 2, end: 3 }]); + editor.setFocus({ start: 2, end: 3 }); + editor.setSelections([{ start: 2, end: 3 }]); runDeleteAction(viewModel, viewModel.cellAt(2)!); assert.strictEqual(viewModel.length, 2); - assert.deepStrictEqual(viewModel.getFocus(), { start: 1, end: 2 }); + assert.deepStrictEqual(editor.getFocus(), { start: 1, end: 2 }); }); }); @@ -420,10 +428,11 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 0, end: 1 }, [{ start: 0, end: 1 }, { start: 3, end: 4 }]); + editor.setFocus({ start: 0, end: 1 }); + editor.setSelections([{ start: 0, end: 1 }, { start: 3, end: 4 }]); runDeleteAction(viewModel, viewModel.cellAt(0)!); assert.strictEqual(viewModel.length, 2); - assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(editor.getFocus(), { start: 0, end: 1 }); }); }); @@ -438,10 +447,11 @@ suite('CellOperations', () => { ], async (editor) => { const viewModel = editor.viewModel; - viewModel.setSelections({ start: 1, end: 2 }, [{ start: 1, end: 2 }, { start: 3, end: 5 }]); + editor.setFocus({ start: 1, end: 2 }); + editor.setSelections([{ start: 1, end: 2 }, { start: 3, end: 5 }]); runDeleteAction(viewModel, viewModel.cellAt(1)!); assert.strictEqual(viewModel.length, 2); - assert.deepStrictEqual(viewModel.getFocus(), { start: 1, end: 2 }); + assert.deepStrictEqual(editor.getFocus(), { start: 1, end: 2 }); }); }); }); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts index c1e818abea5..488f63f12e3 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts @@ -31,7 +31,7 @@ import { Webview } from 'vs/workbench/contrib/webview/browser/webview'; function getFocusedWebviewDelegate(accessor: ServicesAccessor): Webview | undefined { const editorService = accessor.get(IEditorService); const editor = getNotebookEditorFromEditorPane(editorService.activeEditorPane); - if (!editor?.hasFocus()) { + if (!editor?.hasEditorFocus()) { return; } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 8378b0a5090..ae13143c082 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -183,9 +183,9 @@ export interface ICommonNotebookEditor { scheduleOutputHeightAck(cellInfo: ICommonCellInfo, outputId: string, height: number): void; updateMarkupCellHeight(cellId: string, height: number, isInit: boolean): void; setMarkupCellEditState(cellId: string, editState: CellEditState): void; - didStartDragMarkupCell(cellId: string, event: { dragOffsetY: number }): void; - didDragMarkupCell(cellId: string, event: { dragOffsetY: number }): void; - didDropMarkupCell(cellId: string, event: { dragOffsetY: number, ctrlKey: boolean, altKey: boolean }): void; + didStartDragMarkupCell(cellId: string, event: { dragOffsetY: number; }): void; + didDragMarkupCell(cellId: string, event: { dragOffsetY: number; }): void; + didDropMarkupCell(cellId: string, event: { dragOffsetY: number, ctrlKey: boolean, altKey: boolean; }): void; didEndDragMarkupCell(cellId: string): void; } @@ -357,10 +357,13 @@ export interface INotebookEditor extends ICommonNotebookEditor { readonly onDidChangeVisibleRanges: Event; readonly onDidChangeSelection: Event; getSelections(): ICellRange[]; + setSelections(selections: ICellRange[]): void; + getFocus(): ICellRange; + setFocus(focus: ICellRange): void; visibleRanges: ICellRange[]; textModel?: NotebookTextModel; getId(): string; - hasFocus(): boolean; + hasEditorFocus(): boolean; isEmbedded: boolean; @@ -396,7 +399,7 @@ export interface INotebookEditor extends ICommonNotebookEditor { */ focus(): void; - hasFocus(): boolean; + hasEditorFocus(): boolean; hasWebviewFocus(): boolean; hasOutputTextSelection(): boolean; @@ -461,12 +464,12 @@ export interface INotebookEditor extends ICommonNotebookEditor { /** * Execute the given notebook cells */ - executeNotebookCells(cells?: Iterable): Promise + executeNotebookCells(cells?: Iterable): Promise; /** * Cancel the given notebook cells */ - cancelNotebookCells(cells?: Iterable): Promise + cancelNotebookCells(cells?: Iterable): Promise; /** * Get current active cell diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 040ab437824..778d884b9dc 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -456,10 +456,36 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return this.viewModel?.getSelections() ?? []; } + setSelections(selections: ICellRange[]) { + if (!this.hasModel()) { + return; + } + + const focus = this.viewModel.getFocus(); + this.viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + focus: focus, + selections: selections + }); + } + getFocus() { return this.viewModel?.getFocus() ?? { start: 0, end: 0 }; } + setFocus(focus: ICellRange) { + if (!this.hasModel()) { + return; + } + + const selections = this.viewModel.getSelections(); + this.viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + focus: focus, + selections: selections + }); + } + getSelectionViewModels(): ICellViewModel[] { if (!this.viewModel) { return []; @@ -1603,7 +1629,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor this.viewModel?.setEditorFocus(focused); } - hasFocus() { + hasEditorFocus() { return this._editorFocus.get() || false; } @@ -1612,7 +1638,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor } hasOutputTextSelection() { - if (!this.hasFocus()) { + if (!this.hasEditorFocus()) { return false; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts index 1c6b5bf6f83..a507556d087 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts @@ -56,7 +56,7 @@ export class CodeCell extends Disposable { if (model && templateData.editor) { templateData.editor.setModel(model); viewCell.attachTextEditor(templateData.editor); - if (notebookEditor.getActiveCell() === viewCell && viewCell.focusMode === CellFocusMode.Editor && this.notebookEditor.hasFocus()) { + if (notebookEditor.getActiveCell() === viewCell && viewCell.focusMode === CellFocusMode.Editor && this.notebookEditor.hasEditorFocus()) { templateData.editor?.focus(); } @@ -65,7 +65,7 @@ export class CodeCell extends Disposable { this.onCellHeightChange(realContentHeight); } - if (this.notebookEditor.getActiveCell() === this.viewCell && viewCell.focusMode === CellFocusMode.Editor && this.notebookEditor.hasFocus()) { + if (this.notebookEditor.getActiveCell() === this.viewCell && viewCell.focusMode === CellFocusMode.Editor && this.notebookEditor.hasEditorFocus()) { templateData.editor?.focus(); } } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/markdownCell.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/markdownCell.ts index 11ec00cba56..420a80441fb 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/markdownCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/markdownCell.ts @@ -382,7 +382,7 @@ export class StatefulMarkdownCell extends Disposable { } private focusEditorIfNeeded() { - if (this.viewCell.focusMode === CellFocusMode.Editor && this.notebookEditor.hasFocus()) { + if (this.viewCell.focusMode === CellFocusMode.Editor && this.notebookEditor.hasEditorFocus()) { this.editor?.focus(); } } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index 455da3848ce..ee9423ffb95 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -403,10 +403,6 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } } - setSelections(focus: ICellRange, selections: ICellRange[]) { - this.updateSelectionsState({ kind: SelectionStateType.Index, focus, selections }, 'model'); - } - // selection change from list view's `setFocus` and `setSelection` should always use `source: view` to prevent events breaking the list view focus/selection change transaction updateSelectionsState(state: ISelectionState, source: 'view' | 'model' = 'model') { if (this._focused) { diff --git a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts index 727a3f2bfcc..3dfe5b3e06e 100644 --- a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts @@ -21,7 +21,7 @@ import { NotebookEventDispatcher } from 'vs/workbench/contrib/notebook/browser/v import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; -import { CellKind, CellUri, INotebookDiffEditorModel, INotebookEditorModel, IOutputDto, IResolvedNotebookEditorModel, NotebookCellMetadata, } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, CellUri, INotebookDiffEditorModel, INotebookEditorModel, IOutputDto, IResolvedNotebookEditorModel, NotebookCellMetadata, SelectionStateType, } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { TextModelResolverService } from 'vs/workbench/services/textmodelResolver/common/textModelResolverService'; import { IModelService } from 'vs/editor/common/services/modelService'; @@ -201,6 +201,20 @@ function _createTestNotebookEditor(instantiationService: TestInstantiationServic } override getFocus() { return viewModel.getFocus(); } override getSelections() { return viewModel.getSelections(); } + override setFocus(focus: ICellRange) { + viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + focus: focus, + selections: viewModel.getSelections() + }); + } + override setSelections(selections: ICellRange[]) { + viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + focus: viewModel.getFocus(), + selections: selections + }); + } override getViewIndex(cell: ICellViewModel) { return listViewInfoAccessor.getViewIndex(cell); } override getCellRangeFromViewRange(startIndex: number, endIndex: number) { return listViewInfoAccessor.getCellRangeFromViewRange(startIndex, endIndex); } override revealCellRangeInView() { }