From 7707feab17b297d62e4cd38d81530d2727d7ac29 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 3 Mar 2021 15:17:56 -0800 Subject: [PATCH 01/25] accept focus from the list view. --- .../notebook/browser/notebookEditorWidget.ts | 1 + .../notebook/browser/view/notebookCellList.ts | 20 +----------- .../viewModel/cellSelectionCollection.ts | 4 +++ .../browser/viewModel/notebookViewModel.ts | 8 +++-- .../contrib/notebook/common/notebookCommon.ts | 5 +-- .../notebook/test/notebookSelection.test.ts | 32 ++++++++++++++++--- .../notebook/test/notebookTextModel.test.ts | 4 +-- 7 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 270abab7746..2b0eec28be3 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -914,6 +914,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor } else if (this._list.length > 0) { this.viewModel?.updateSelectionsState({ kind: SelectionStateType.Index, + focus: { start: 0, end: 1 }, selections: [{ start: 0, end: 1 }] }); } diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 67381a6fdba..689b734cc73 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -521,7 +521,7 @@ export class NotebookCellList extends WorkbenchList implements ID if (!selectionsLeft.length && this._viewModel!.viewCells.length) { // after splice, the selected cells are deleted - this._viewModel!.updateSelectionsState({ kind: SelectionStateType.Index, selections: [{ start: 0, end: 1 }] }); + this._viewModel!.updateSelectionsState({ kind: SelectionStateType.Index, focus: { start: 0, end: 1 }, selections: [{ start: 0, end: 1 }] }); } } @@ -609,30 +609,12 @@ export class NotebookCellList extends WorkbenchList implements ID focusNext(n: number | undefined, loop: boolean | undefined, browserEvent?: UIEvent, filter?: (element: CellViewModel) => boolean): void { this._focusNextPreviousDelegate.onFocusNext(() => { super.focusNext(n, loop, browserEvent, filter); - const focus = this.getFocus(); - if (focus.length) { - const focusedElementHandle = this.element(focus[0]).handle; - this._viewModel?.updateSelectionsState({ - kind: SelectionStateType.Handle, - primary: focusedElementHandle, - selections: [focusedElementHandle] - }, 'view'); - } }); } focusPrevious(n: number | undefined, loop: boolean | undefined, browserEvent?: UIEvent, filter?: (element: CellViewModel) => boolean): void { this._focusNextPreviousDelegate.onFocusPrevious(() => { super.focusPrevious(n, loop, browserEvent, filter); - const focus = this.getFocus(); - if (focus.length) { - const focusedElementHandle = this.element(focus[0]).handle; - this._viewModel?.updateSelectionsState({ - kind: SelectionStateType.Handle, - primary: focusedElementHandle, - selections: [focusedElementHandle] - }, 'view'); - } }); } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts index 9d2d5e1d98a..218a87bfb98 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts @@ -42,6 +42,10 @@ export class NotebookCellSelectionCollection extends Disposable { return this._selections[0]; } + get focus(): ICellRange { + return this._primary ?? { start: 0, end: 0 }; + } + setState(primary: ICellRange | null, selections: ICellRange[], forceEventEmit: boolean, source: 'view' | 'model') { if (primary !== null) { const primaryRange = primary; diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index b9cf9f594ec..71714acb6d3 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -353,6 +353,10 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD }); } + getFocus() { + return this._selectionCollection.focus; + } + getSelection() { return this._selectionCollection.selection; } @@ -736,7 +740,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD cells: [] }], synchronous, - { kind: SelectionStateType.Index, selections: this.getSelections() }, + { kind: SelectionStateType.Index, focus: this.getFocus(), selections: this.getSelections() }, () => ({ kind: SelectionStateType.Handle, primary: endPrimarySelection, selections: endSelections }), undefined, pushUndoStop @@ -764,7 +768,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD length, newIdx } - ], synchronous, { kind: SelectionStateType.Index, selections: this.getSelections() }, () => ({ kind: SelectionStateType.Index, selections: [{ start: newIdx, end: newIdx + 1 }] }), undefined); + ], synchronous, { kind: SelectionStateType.Index, focus: this.getFocus(), selections: this.getSelections() }, () => ({ kind: SelectionStateType.Index, focus: { start: newIdx, end: newIdx + 1 }, selections: [{ start: newIdx, end: newIdx + 1 }] }), undefined); return true; } diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 96a7811054c..8e616a0e031 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -310,10 +310,7 @@ export interface ISelectionHandleState { export interface ISelectionIndexState { kind: SelectionStateType.Index; - - /** - * [primarySelection, ...secondarySelections] - */ + focus: ICellRange; selections: ICellRange[]; } diff --git a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts index 4e77f4b4ad9..2efa5281196 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts @@ -83,6 +83,29 @@ suite('NotebookCellList focus/selection', () => { }); }); + test('notebook cell list setFocus', function () { + withTestNotebook( + instantiationService, + [ + ['var a = 1;', 'javascript', CellKind.Code, [], {}], + ['var b = 2;', 'javascript', CellKind.Code, [], {}] + ], + (editor, viewModel) => { + const cellList = createNotebookCellList(instantiationService); + cellList.attachViewModel(viewModel); + + assert.strictEqual(cellList.length, 2); + cellList.setFocus([0]); + assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + + cellList.setFocus([1]); + assert.deepStrictEqual(viewModel.getFocus(), { start: 1, end: 2 }); + + cellList.setSelection([1]); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 1, end: 2 }]); + }); + }); + test('notebook cell list focus/selection with folding regions', function () { withTestNotebook( instantiationService, @@ -113,15 +136,14 @@ suite('NotebookCellList focus/selection', () => { cellList.focusNext(1, false); // focus next should skip the folded items - assert.deepStrictEqual(viewModel.getSelection(), { start: 2, end: 3 }); - assert.deepStrictEqual(viewModel.getSelections(), [{ start: 2, end: 3 }]); + assert.deepStrictEqual(viewModel.getFocus(), { start: 2, end: 3 }); // unfold updateFoldingStateAtIndex(foldingModel, 2, false); viewModel.updateFoldingRanges(foldingModel.regions); cellList.setHiddenAreas(viewModel.getHiddenRanges(), true); assert.strictEqual(cellList.length, 4); - assert.deepStrictEqual(viewModel.getSelection(), { start: 2, end: 3 }); + assert.deepStrictEqual(viewModel.getFocus(), { start: 2, end: 3 }); }); }); @@ -153,7 +175,7 @@ suite('NotebookCellList focus/selection', () => { ['var b = 1;', 'javascript', CellKind.Code, [], {}] ], (editor, viewModel) => { - viewModel.updateSelectionsState({ kind: SelectionStateType.Index, selections: [{ start: 1, end: 2 }, { start: -1, end: 0 }] }); + viewModel.updateSelectionsState({ kind: SelectionStateType.Index, focus: { start: 1, end: 2 }, selections: [{ start: 1, end: 2 }, { start: -1, end: 0 }] }); assert.deepStrictEqual(viewModel.getSelections(), [{ start: 1, end: 2 }]); }); }); @@ -166,7 +188,7 @@ suite('NotebookCellList focus/selection', () => { ['var b = 1;', 'javascript', CellKind.Code, [], {}] ], (editor, viewModel) => { - viewModel.updateSelectionsState({ kind: SelectionStateType.Index, selections: [{ start: 1, end: 2 }] }); + viewModel.updateSelectionsState({ kind: SelectionStateType.Index, focus: { start: 1, end: 2 }, selections: [{ start: 1, end: 2 }] }); viewModel.deleteCell(1, true, false); assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); }); diff --git a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts index 86bf2faf9c8..1f4a4979c1e 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts @@ -279,7 +279,7 @@ suite('NotebookTextModel', () => { textModel.applyEdits([ { editType: CellEditType.Replace, index: 1, count: 1, cells: [] }, { editType: CellEditType.Replace, index: 1, count: 0, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, - ], true, undefined, () => ({ kind: SelectionStateType.Index, selections: [{ start: 0, end: 1 }] }), undefined); + ], true, undefined, () => ({ kind: SelectionStateType.Index, focus: { start: 0, end: 1 }, selections: [{ start: 0, end: 1 }] }), undefined); assert.equal(textModel.cells.length, 4); assert.equal(textModel.cells[0].getValue(), 'var a = 1;'); @@ -318,7 +318,7 @@ suite('NotebookTextModel', () => { editType: CellEditType.Metadata, metadata: { editable: false }, } - ], true, undefined, () => ({ kind: SelectionStateType.Index, selections: [{ start: 0, end: 1 }] }), undefined); + ], true, undefined, () => ({ kind: SelectionStateType.Index, focus: { start: 0, end: 1 }, selections: [{ start: 0, end: 1 }] }), undefined); assert.notEqual(changeEvent, undefined); assert.equal(changeEvent!.rawEvents.length, 2); From d3086eaa8c591787ac9921f7f0baad2b3117059c Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 3 Mar 2021 16:59:05 -0800 Subject: [PATCH 02/25] expect commands to set selection and focus for us properly. --- .../contrib/outline/notebookOutline.ts | 8 ++-- .../notebook/browser/notebookBrowser.ts | 2 +- .../notebook/browser/notebookEditorWidget.ts | 4 +- .../notebook/browser/view/notebookCellList.ts | 2 +- .../viewModel/cellSelectionCollection.ts | 35 +++------------ .../browser/viewModel/notebookViewModel.ts | 16 +++---- .../notebook/test/notebookSelection.test.ts | 43 +++++++------------ .../notebook/test/testNotebookEditor.ts | 2 +- 8 files changed, 36 insertions(+), 76 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts index 1d789cf6d59..979399fdf6d 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts @@ -390,8 +390,8 @@ export class NotebookCellOutline implements IOutline { includeCodeCells = this._configurationService.getValue('notebook.breadcrumbs.showCodeCells'); } - const selectedCellIndex = viewModel.getSelection().start; - const selected = viewModel.getCellByIndex(selectedCellIndex)?.handle; + const focusedCellIndex = viewModel.getFocus().start; + const focused = viewModel.getCellByIndex(focusedCellIndex)?.handle; const entries: OutlineEntry[] = []; for (let i = 0; i < viewModel.viewCells.length; i++) { @@ -434,7 +434,7 @@ export class NotebookCellOutline implements IOutline { entries.push(new OutlineEntry(entries.length, 7, cell, preview, isMarkdown ? Codicon.markdown : Codicon.code)); } - if (cell.handle === selected) { + if (cell.handle === focused) { this._activeEntry = entries[entries.length - 1]; } @@ -518,7 +518,7 @@ export class NotebookCellOutline implements IOutline { const { viewModel } = this._editor; if (viewModel) { - const cell = viewModel.getCellByIndex(viewModel.getSelection().start); + const cell = viewModel.getCellByIndex(viewModel.getFocus().start); if (cell) { for (let entry of this._entries) { newActive = entry.find(cell, []); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index de4f972fb02..5d88b91a197 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -319,7 +319,7 @@ export interface INotebookEditorCreationOptions { export interface IActiveNotebookEditor extends INotebookEditor { viewModel: NotebookViewModel; // selection is never undefined when the editor is attached to a document. - getSelection(): ICellRange; + getFocus(): ICellRange; } export const NOTEBOOK_EDITOR_ID = 'workbench.editor.notebook'; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 2b0eec28be3..3064ec58115 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -294,8 +294,8 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return this.viewModel?.getSelections() ?? []; } - getSelection() { - return this.viewModel?.getSelection(); + getFocus() { + return this.viewModel?.getFocus() ?? { start: 0, end: 0 }; } getSelectionViewModels(): ICellViewModel[] { diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 689b734cc73..08f23876839 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -372,7 +372,7 @@ export class NotebookCellList extends WorkbenchList implements ID // convert model selections to view selections const viewSelections = cellRangesToIndexes(model.getSelections()).map(index => model.getCellByIndex(index)).filter(cell => !!cell).map(cell => this._getViewIndexUpperBound(cell!)); this.setSelection(viewSelections, undefined, true); - const primary = cellRangesToIndexes([model.getSelection()]).map(index => model.getCellByIndex(index)).filter(cell => !!cell).map(cell => this._getViewIndexUpperBound(cell!)); + const primary = cellRangesToIndexes([model.getFocus()]).map(index => model.getCellByIndex(index)).filter(cell => !!cell).map(cell => this._getViewIndexUpperBound(cell!)); if (primary.length) { this.setFocus(primary, undefined, true); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts index 218a87bfb98..587cd6ecce4 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection.ts @@ -32,7 +32,7 @@ export class NotebookCellSelectionCollection extends Disposable { private _primary: ICellRange | null = null; - private _selections: ICellRange[] = [{ start: 0, end: 0 }]; + private _selections: ICellRange[] = []; get selections(): ICellRange[] { return this._selections; @@ -47,35 +47,12 @@ export class NotebookCellSelectionCollection extends Disposable { } setState(primary: ICellRange | null, selections: ICellRange[], forceEventEmit: boolean, source: 'view' | 'model') { - if (primary !== null) { - const primaryRange = primary; - // TODO@rebornix deal with overlap - const newSelections = [primaryRange, ...selections.filter(selection => !(selection.start === primaryRange.start && selection.end === primaryRange.end)).sort((a, b) => a.start - b.start)]; + const changed = primary !== this._primary || !rangesEqual(this._selections, selections); - const changed = primary !== this._primary || !rangesEqual(this._selections, newSelections); - this._primary = primary; - this._selections = newSelections; - - if (!this._selections.length) { - this._selections.push({ start: 0, end: 0 }); - } - - if (changed || forceEventEmit) { - this._onDidChangeSelection.fire(source); - } - } else { - const changed = primary !== this._primary || !rangesEqual(this._selections, selections); - - this._primary = primary; - this._selections = selections; - - if (!this._selections.length) { - this._selections.push({ start: 0, end: 0 }); - } - - if (changed || forceEventEmit) { - this._onDidChangeSelection.fire(source); - } + this._primary = primary; + this._selections = selections; + if (changed || forceEventEmit) { + this._onDidChangeSelection.fire(source); } } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index 71714acb6d3..dcaf033f2ee 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -357,10 +357,6 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return this._selectionCollection.focus; } - getSelection() { - return this._selectionCollection.selection; - } - getSelections() { return this._selectionCollection.selections; } @@ -719,14 +715,14 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } deleteCell(index: number, synchronous: boolean, pushUndoStop: boolean = true) { - const primarySelectionIndex = this.getSelection()?.start ?? null; + const focusSelectionIndex = this.getFocus()?.start ?? null; let endPrimarySelection: number | null = null; - if (index === primarySelectionIndex) { - if (primarySelectionIndex < this.length - 1) { - endPrimarySelection = this._viewCells[primarySelectionIndex + 1].handle; - } else if (primarySelectionIndex === this.length - 1 && this.length > 1) { - endPrimarySelection = this._viewCells[primarySelectionIndex - 1].handle; + if (index === focusSelectionIndex) { + if (focusSelectionIndex < this.length - 1) { + endPrimarySelection = this._viewCells[focusSelectionIndex + 1].handle; + } else if (focusSelectionIndex === this.length - 1 && this.length > 1) { + endPrimarySelection = this._viewCells[focusSelectionIndex - 1].handle; } } diff --git a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts index 2efa5281196..5461f5490cf 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts @@ -10,28 +10,12 @@ import { CellKind, SelectionStateType } from 'vs/workbench/contrib/notebook/comm import { createNotebookCellList, setupInstantiationService, withTestNotebook } from 'vs/workbench/contrib/notebook/test/testNotebookEditor'; suite('NotebookSelection', () => { - test('selection is never empty', function () { + test('focus is never empty', function () { const selectionCollection = new NotebookCellSelectionCollection(); - assert.deepStrictEqual(selectionCollection.selections, [{ start: 0, end: 0 }]); + assert.deepStrictEqual(selectionCollection.focus, { start: 0, end: 0 }); selectionCollection.setState(null, [], true, 'model'); - assert.deepStrictEqual(selectionCollection.selections, [{ start: 0, end: 0 }]); - }); - - test('selections[0] is primary selection', function () { - const selectionCollection = new NotebookCellSelectionCollection(); - selectionCollection.setState(null, [{ start: 0, end: 1 }, { start: 3, end: 5 }], true, 'model'); - assert.deepStrictEqual(selectionCollection.selection, { start: 0, end: 1 }); - assert.deepStrictEqual(selectionCollection.selections, [{ start: 0, end: 1 }, { start: 3, end: 5 }]); - - selectionCollection.setState({ start: 0, end: 1 }, [{ start: 3, end: 5 }], true, 'model'); - assert.deepStrictEqual(selectionCollection.selection, { start: 0, end: 1 }); - assert.deepStrictEqual(selectionCollection.selections, [{ start: 0, end: 1 }, { start: 3, end: 5 }]); - - selectionCollection.setState({ start: 0, end: 1 }, [], true, 'model'); - assert.deepStrictEqual(selectionCollection.selection, { start: 0, end: 1 }); - assert.deepStrictEqual(selectionCollection.selections, [{ start: 0, end: 1 }]); - + assert.deepStrictEqual(selectionCollection.focus, { start: 0, end: 0 }); }); }); @@ -51,10 +35,10 @@ suite('NotebookCellList focus/selection', () => { assert.strictEqual(cellList.length, 2); cellList.setFocus([0]); - assert.deepStrictEqual(viewModel.getSelection(), { start: 0, end: 1 }); + assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); cellList.setFocus([1]); - assert.deepStrictEqual(viewModel.getSelection(), { start: 1, end: 2 }); + assert.deepStrictEqual(viewModel.getFocus(), { start: 1, end: 2 }); cellList.detachViewModel(); }); }); @@ -73,13 +57,11 @@ suite('NotebookCellList focus/selection', () => { assert.strictEqual(cellList.length, 2); cellList.setSelection([0]); // the only selection is also the focus - assert.deepStrictEqual(viewModel.getSelection(), { start: 0, end: 1 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); // set selection does not modify focus cellList.setSelection([1]); - assert.deepStrictEqual(viewModel.getSelection(), { start: 0, end: 1 }); - // `getSelections()` now returns all focus/selection ranges - assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }, { start: 1, end: 2 }]); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 1, end: 2 }]); }); }); @@ -123,6 +105,8 @@ suite('NotebookCellList focus/selection', () => { const cellList = createNotebookCellList(instantiationService); cellList.attachViewModel(viewModel); assert.strictEqual(cellList.length, 5); + assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); cellList.setFocus([0]); updateFoldingStateAtIndex(foldingModel, 0, true); @@ -131,12 +115,14 @@ suite('NotebookCellList focus/selection', () => { cellList.setHiddenAreas(viewModel.getHiddenRanges(), true); assert.strictEqual(cellList.length, 3); - // currently, focus on a folded cell will only select the cell itself, excluding its "inner" cells - assert.deepStrictEqual(viewModel.getSelection(), { start: 0, end: 1 }); + // currently, focus on a folded cell will only focus the cell itself, excluding its "inner" cells + assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); cellList.focusNext(1, false); // focus next should skip the folded items assert.deepStrictEqual(viewModel.getFocus(), { start: 2, end: 3 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); // unfold updateFoldingStateAtIndex(foldingModel, 2, false); @@ -190,7 +176,8 @@ suite('NotebookCellList focus/selection', () => { (editor, viewModel) => { viewModel.updateSelectionsState({ kind: SelectionStateType.Index, focus: { start: 1, end: 2 }, selections: [{ start: 1, end: 2 }] }); viewModel.deleteCell(1, true, false); - assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); + assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(viewModel.getSelections(), []); }); }); }); diff --git a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts index 43296a32a9c..7b5e85c3602 100644 --- a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts @@ -68,7 +68,7 @@ export class TestNotebookEditor implements INotebookEditor { constructor(readonly viewModel: NotebookViewModel) { } - getSelection(): ICellRange | undefined { + getFocus(): ICellRange | undefined { throw new Error('Method not implemented.'); } getSelections(): ICellRange[] { From 2d5e9cae4e085b3bbb4613401ce0452fc1849f33 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 5 Mar 2021 20:15:13 -0800 Subject: [PATCH 03/25] :lipstick: --- .../notebook/browser/contrib/clipboard/notebookClipboard.ts | 3 ++- src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts | 2 -- .../contrib/notebook/browser/view/renderers/codeCell.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) 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 39f7ff2face..9cba4d3ca3f 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts @@ -182,9 +182,10 @@ class NotebookClipboardContribution extends Disposable { ? firstSelectIndex : viewModel.notebookDocument.cells.length - 1; - viewModel.notebookDocument.applyEdits(edits, true, { kind: SelectionStateType.Index, selections: viewModel.getSelections() }, () => { + viewModel.notebookDocument.applyEdits(edits, true, { kind: SelectionStateType.Index, focus: viewModel.getFocus(), selections: viewModel.getSelections() }, () => { return { kind: SelectionStateType.Index, + focus: { start: newFocusedCellIndex, end: newFocusedCellIndex + 1 }, selections: [{ start: newFocusedCellIndex, end: newFocusedCellIndex + 1 }] }; }, undefined, true); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 5d88b91a197..a7feefb555d 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -318,7 +318,6 @@ export interface INotebookEditorCreationOptions { export interface IActiveNotebookEditor extends INotebookEditor { viewModel: NotebookViewModel; - // selection is never undefined when the editor is attached to a document. getFocus(): ICellRange; } @@ -331,7 +330,6 @@ export interface INotebookEditor extends ICommonNotebookEditor { // from the old IEditor readonly onDidChangeVisibleRanges: Event; readonly onDidChangeSelection: Event; - getSelection(): ICellRange | undefined; getSelections(): ICellRange[]; visibleRanges: ICellRange[]; textModel?: NotebookTextModel; 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 e7f2d1ca093..34334bb72db 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts @@ -76,7 +76,7 @@ export class CodeCell extends Disposable { }); const updateForFocusMode = () => { - if (this.notebookEditor.getSelection().start !== this.notebookEditor.viewModel.getCellIndex(viewCell)) { + if (this.notebookEditor.getFocus().start !== this.notebookEditor.viewModel.getCellIndex(viewCell)) { return; } From a0c8d41f9ed316c4996502f1c58ec79323e9e330 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 5 Mar 2021 12:40:07 -0800 Subject: [PATCH 04/25] add notebook navigation commands --- .../notebook/browser/contrib/coreActions.ts | 55 ++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index 5d03dc12837..e9fca3ca358 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -23,7 +23,7 @@ import { IQuickInputService, IQuickPickItem, QuickPickInput } from 'vs/platform/ import { CATEGORIES } from 'vs/workbench/common/actions'; import { BaseCellRenderTemplate, CellEditState, CellFocusMode, EXECUTE_CELL_COMMAND_ID, EXPAND_CELL_CONTENT_COMMAND_ID, getNotebookEditorFromEditorPane, IActiveNotebookEditor, ICellViewModel, NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_EDITOR_FOCUSED, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_INPUT_COLLAPSED, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_OUTPUT_COLLAPSED, NOTEBOOK_CELL_TYPE, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_EXECUTING_NOTEBOOK, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_EDITOR_RUNNABLE, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_OUTPUT_FOCUSED } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; -import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellRunState, NOTEBOOK_EDITOR_CURSOR_BEGIN_END, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellRunState, NOTEBOOK_EDITOR_CURSOR_BEGIN_END, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, SelectionStateType, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; @@ -1075,6 +1075,58 @@ async function copyCell(context: INotebookCellActionContext, direction: 'up' | ' } } +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: 'notebook.cell.focusDown', + title: localize('notebookActions.focusDown', "Notebook Cell Focus Down"), + icon: icons.moveUpIcon, + keybinding: { + primary: KeyCode.DownArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + const viewModel = context.notebookEditor.viewModel; + const primaryIndex = viewModel.getSelection().start; + const nextIndex = Math.min(primaryIndex + 1, viewModel.length - 1); + viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + selections: [{ start: nextIndex, end: nextIndex + 1 }] + }); + } +}); + +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: 'notebook.cell.focusUp', + title: localize('notebookActions.focusUp', "Notebook Cell Focus Up"), + icon: icons.moveDownIcon, + keybinding: { + primary: KeyCode.UpArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + const viewModel = context.notebookEditor.viewModel; + const primaryIndex = viewModel.getSelection().start; + const nextIndex = Math.max(primaryIndex - 1, 0); + viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + selections: [{ start: nextIndex, end: nextIndex + 1 }] + }); + } +}); + registerAction2(class extends NotebookCellAction { constructor() { super( @@ -1091,7 +1143,6 @@ registerAction2(class extends NotebookCellAction { } async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - return moveCell(context, 'up'); } }); From 3f56118e7bbfda2a30abe6c929ca8e80573020f0 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 8 Mar 2021 11:18:21 -0800 Subject: [PATCH 05/25] move cell up --- src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index e9fca3ca358..8e28e241700 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -1143,6 +1143,7 @@ registerAction2(class extends NotebookCellAction { } async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + return moveCell(context, 'up'); } }); From 1761ecec3bd8ff5fb28dc5e457b071667e228a2d Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 8 Mar 2021 11:50:41 -0800 Subject: [PATCH 06/25] reduce selections. --- .../browser/viewModel/notebookViewModel.ts | 8 ++++---- .../contrib/notebook/common/notebookCommon.ts | 19 +++++++++++++++++++ .../notebook/test/notebookCommon.test.ts | 8 ++++++++ 3 files changed, 31 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 dcaf033f2ee..ecde86d487a 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -24,7 +24,7 @@ import { NotebookEventDispatcher, NotebookMetadataChangedEvent } from 'vs/workbe import { CellFoldingState, EditorFoldingStateDelegate } from 'vs/workbench/contrib/notebook/browser/contrib/fold/foldingModel'; import { MarkdownCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { CellKind, NotebookCellMetadata, INotebookSearchOptions, ICellRange, NotebookCellsChangeType, ICell, NotebookCellTextModelSplice, CellEditType, IOutputDto, SelectionStateType, ISelectionState, cellIndexesToRanges, cellRangesToIndexes } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, NotebookCellMetadata, INotebookSearchOptions, ICellRange, NotebookCellsChangeType, ICell, NotebookCellTextModelSplice, CellEditType, IOutputDto, SelectionStateType, ISelectionState, cellIndexesToRanges, cellRangesToIndexes, reduceRanges } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { FoldingRegions } from 'vs/editor/contrib/folding/foldingRanges'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { MarkdownRenderer } from 'vs/editor/browser/core/markdownRenderer'; @@ -396,13 +396,13 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD const selections = cellIndexesToRanges(state.selections.map(sel => this.getCellIndexByHandle(sel))) .map(range => this.validateRange(range)) .filter(range => range !== null) as ICellRange[]; - this._selectionCollection.setState(primarySelection, selections, true, source); + this._selectionCollection.setState(primarySelection, reduceRanges(selections), true, source); } else { - const primarySelection = this.validateRange(state.selections[0]); + const primarySelection = this.validateRange(state.focus); const selections = state.selections .map(range => this.validateRange(range)) .filter(range => range !== null) as ICellRange[]; - this._selectionCollection.setState(primarySelection, selections, true, source); + this._selectionCollection.setState(primarySelection, reduceRanges(selections), true, source); } } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 8e616a0e031..88250228816 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -821,3 +821,22 @@ export function cellRangesToIndexes(ranges: ICellRange[]) { return indexes; } + +export function reduceRanges(ranges: ICellRange[]) { + const sorted = ranges.sort((a, b) => a.start - b.start); + const first = sorted[0]; + + if (!first) { + return []; + } + + return sorted.reduce((prev: ICellRange[], curr) => { + const last = prev[prev.length - 1]; + if (last.end >= curr.start) { + last.end = Math.max(last.end, curr.end); + } else { + prev.push(curr); + } + return prev; + }, [first] as ICellRange[]); +} diff --git a/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts b/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts index bc9f5874744..1169d7258c7 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts @@ -8,6 +8,7 @@ import { NOTEBOOK_DISPLAY_ORDER, sortMimeTypes, CellKind, diff, CellUri, cellRan import { TestCell, setupInstantiationService } from 'vs/workbench/contrib/notebook/test/testNotebookEditor'; import { URI } from 'vs/base/common/uri'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; +import { reduceCellRanges } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; suite('NotebookCommon', () => { const instantiationService = setupInstantiationService(); @@ -314,4 +315,11 @@ suite('CellRange', function () { assert.deepStrictEqual(cellIndexesToRanges([0, 1, 2]), [{ start: 0, end: 3 }]); assert.deepStrictEqual(cellIndexesToRanges([0, 1, 3]), [{ start: 0, end: 2 }, { start: 3, end: 4 }]); }); + + test('Reduce ranges', function () { + assert.deepStrictEqual(reduceCellRanges([{ start: 0, end: 1 }, { start: 1, end: 2 }]), [{ start: 0, end: 2 }]); + assert.deepStrictEqual(reduceCellRanges([{ start: 0, end: 2 }, { start: 1, end: 3 }]), [{ start: 0, end: 3 }]); + assert.deepStrictEqual(reduceCellRanges([{ start: 1, end: 3 }, { start: 0, end: 2 }]), [{ start: 0, end: 3 }]); + assert.deepStrictEqual(reduceCellRanges([{ start: 0, end: 2 }, { start: 4, end: 5 }]), [{ start: 0, end: 2 }, { start: 4, end: 5 }]); + }); }); From 9b1700a82912815e7692893a9676f4f306ab871f Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 8 Mar 2021 12:54:23 -0800 Subject: [PATCH 07/25] example of notebook specific navigation commands. --- .../notebook/browser/contrib/coreActions.ts | 98 ++++++++++++++++++- 1 file changed, 94 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index 8e28e241700..94495178dff 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -1092,12 +1092,28 @@ registerAction2(class extends NotebookCellAction { async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { const viewModel = context.notebookEditor.viewModel; - const primaryIndex = viewModel.getSelection().start; + const primaryIndex = viewModel.getFocus().start; const nextIndex = Math.min(primaryIndex + 1, viewModel.length - 1); + + const focus = { start: nextIndex, end: nextIndex + 1 }; + const existingSelections = viewModel.getSelections(); + let newSelections = []; + + if (existingSelections.length && existingSelections[0].start === primaryIndex && existingSelections[0].end === primaryIndex + 1) { + // focus == selection + newSelections = [focus]; + } else { + // no-op as we allow you to move focus once you are in multi select mode + newSelections = existingSelections; + } + viewModel.updateSelectionsState({ kind: SelectionStateType.Index, - selections: [{ start: nextIndex, end: nextIndex + 1 }] + focus: focus, + selections: newSelections }); + + context.notebookEditor.revealCellRangeInView(focus); } }); @@ -1118,12 +1134,86 @@ registerAction2(class extends NotebookCellAction { async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { const viewModel = context.notebookEditor.viewModel; - const primaryIndex = viewModel.getSelection().start; + const primaryIndex = viewModel.getFocus().start; const nextIndex = Math.max(primaryIndex - 1, 0); + const focus = { start: nextIndex, end: nextIndex + 1 }; + const existingSelections = viewModel.getSelections(); + let newSelections = []; + + if (existingSelections.length === 1 && existingSelections[0].start === primaryIndex && existingSelections[0].end === primaryIndex + 1) { + // focus == selection + newSelections = [focus]; + } else { + // no-op as we allow you to move focus once you are in multi select mode + newSelections = existingSelections; + } + viewModel.updateSelectionsState({ kind: SelectionStateType.Index, - selections: [{ start: nextIndex, end: nextIndex + 1 }] + focus: focus, + selections: newSelections }); + + context.notebookEditor.revealCellRangeInView(focus); + } +}); + +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: 'notebook.cell.expandSelectionDown', + title: localize('notebookActions.expandSelectionDown', "Notebook Cell Expand Selection Down"), + icon: icons.moveUpIcon, + keybinding: { + primary: KeyMod.Shift | KeyCode.DownArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + const viewModel = context.notebookEditor.viewModel; + const primaryIndex = viewModel.getFocus().start; + const nextIndex = Math.min(primaryIndex + 1, viewModel.length - 1); + const focus = { start: nextIndex, end: nextIndex + 1 }; + + viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + focus: focus, + selections: [focus, ...viewModel.getSelections()] + }); + context.notebookEditor.revealCellRangeInView(focus); + } +}); + +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: 'notebook.cell.expandSelectionUp', + title: localize('notebookActions.expandSelectionUp', "Notebook Expand Selection Up"), + icon: icons.moveDownIcon, + keybinding: { + primary: KeyMod.Shift | KeyCode.UpArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + const viewModel = context.notebookEditor.viewModel; + const primaryIndex = viewModel.getFocus().start; + const nextIndex = Math.max(primaryIndex - 1, 0); + const focus = { start: nextIndex, end: nextIndex + 1 }; + viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + focus: focus, + selections: [focus, ...viewModel.getSelections()] + }); + context.notebookEditor.revealCellRangeInView(focus); } }); From 61301530c6944b31180b14e748940ff416056555 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 8 Mar 2021 14:50:48 -0800 Subject: [PATCH 08/25] revealView should scroll a little to reveal items below viewport. --- .../notebook/browser/notebookBrowser.ts | 3 +- .../notebook/browser/view/notebookCellList.ts | 33 +++++++++++++------ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index a7feefb555d..1ee17e33f97 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -746,7 +746,8 @@ export enum CellRevealType { export enum CellRevealPosition { Top, - Center + Center, + Bottom } export enum CellEditState { diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 08f23876839..ed9eaca5c23 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -680,7 +680,7 @@ export class NotebookCellList extends WorkbenchList implements ID return; } - const endIndex = this._getViewIndexUpperBound2(range.end); + const endIndex = this._getViewIndexUpperBound2(range.end - 1); const scrollTop = this.getViewScrollTop(); const wrapperBottom = this.getViewScrollBottom(); @@ -1056,18 +1056,31 @@ export class NotebookCellList extends WorkbenchList implements ID } } - // first render - const viewItemOffset = revealPosition === CellRevealPosition.Top ? elementTop : (elementTop - this.view.renderHeight / 2); - this.view.setScrollTop(viewItemOffset); - - // second scroll as markdown cell is dynamic - const newElementTop = this.view.elementTop(viewIndex); - const newViewItemOffset = revealPosition === CellRevealPosition.Top ? newElementTop : (newElementTop - this.view.renderHeight / 2); - this.view.setScrollTop(newViewItemOffset); + switch (revealPosition) { + case CellRevealPosition.Top: + this.view.setScrollTop(elementTop); + this.view.setScrollTop(this.view.elementTop(viewIndex)); + break; + case CellRevealPosition.Center: + this.view.setScrollTop(elementTop - this.view.renderHeight / 2); + this.view.setScrollTop(this.view.elementTop(viewIndex) - this.view.renderHeight / 2); + break; + case CellRevealPosition.Bottom: + this.view.setScrollTop(elementBottom - this.view.renderHeight); + this.view.setScrollTop(this.view.elementTop(viewIndex) + this.view.elementHeight(viewIndex) - this.view.renderHeight); + break; + default: + break; + } } private _revealInView(viewIndex: number) { - this._revealInternal(viewIndex, true, CellRevealPosition.Top); + const firstIndex = this.view.firstVisibleIndex; + if (viewIndex < firstIndex) { + this._revealInternal(viewIndex, true, CellRevealPosition.Top); + } else { + this._revealInternal(viewIndex, true, CellRevealPosition.Bottom); + } } private _revealInCenter(viewIndex: number) { From a97990c97130f6a51e92344ccadb4a493b4043c2 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 13:38:23 -0800 Subject: [PATCH 09/25] sort indexes in cellIndexesToRanges --- src/vs/workbench/contrib/notebook/common/notebookCommon.ts | 1 + src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index abbd2519eb7..8952279114a 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -789,6 +789,7 @@ export interface INotebookDecorationRenderOptions { export function cellIndexesToRanges(indexes: number[]) { + indexes.sort(); const first = indexes.shift(); if (first === undefined) { diff --git a/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts b/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts index 1169d7258c7..b1845714f25 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts @@ -314,6 +314,10 @@ suite('CellRange', function () { assert.deepStrictEqual(cellIndexesToRanges([0, 1]), [{ start: 0, end: 2 }]); assert.deepStrictEqual(cellIndexesToRanges([0, 1, 2]), [{ start: 0, end: 3 }]); assert.deepStrictEqual(cellIndexesToRanges([0, 1, 3]), [{ start: 0, end: 2 }, { start: 3, end: 4 }]); + + assert.deepStrictEqual(cellIndexesToRanges([1, 0]), [{ start: 0, end: 2 }]); + assert.deepStrictEqual(cellIndexesToRanges([1, 2, 0]), [{ start: 0, end: 3 }]); + assert.deepStrictEqual(cellIndexesToRanges([3, 1, 0]), [{ start: 0, end: 2 }, { start: 3, end: 4 }]); }); test('Reduce ranges', function () { From 46fd12b0a9149c15ba885ce42cb4f9cba8713237 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 14:06:40 -0800 Subject: [PATCH 10/25] align selection and focus if they were both empty --- src/vs/workbench/browser/actions/listCommands.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/browser/actions/listCommands.ts b/src/vs/workbench/browser/actions/listCommands.ts index fdb4c8880f4..a160f2db6a5 100644 --- a/src/vs/workbench/browser/actions/listCommands.ts +++ b/src/vs/workbench/browser/actions/listCommands.ts @@ -40,7 +40,10 @@ async function updateFocus(widget: WorkbenchListWidget, updateFocusFn: (widget: const newFocus = widget.getFocus(); - if (selection.length !== 1 || !equals(focus, selection) || equals(focus, newFocus)) { + if ( + (selection.length !== 1 || !equals(focus, selection) || equals(focus, newFocus)) + && !(focus.length === 0 && selection.length === 0) + ) { return; } From 63d6e0c008afe1f14275655d34066365d0b1f256 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 14:07:08 -0800 Subject: [PATCH 11/25] adopt selectionNavigation. --- .../notebook/browser/contrib/coreActions.ts | 144 +----------------- .../notebook/browser/notebookEditorWidget.ts | 1 + 2 files changed, 2 insertions(+), 143 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index 67c6f8dc05a..3ee68e379b5 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -23,7 +23,7 @@ import { IQuickInputService, IQuickPickItem, QuickPickInput } from 'vs/platform/ import { CATEGORIES } from 'vs/workbench/common/actions'; import { BaseCellRenderTemplate, CellEditState, CellFocusMode, EXECUTE_CELL_COMMAND_ID, EXPAND_CELL_CONTENT_COMMAND_ID, getNotebookEditorFromEditorPane, IActiveNotebookEditor, ICellViewModel, NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_EDITOR_FOCUSED, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_INPUT_COLLAPSED, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_OUTPUT_COLLAPSED, NOTEBOOK_CELL_RUN_STATE, NOTEBOOK_CELL_TYPE, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_EXECUTING_NOTEBOOK, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_OUTPUT_FOCUSED } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; -import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellRunState, NOTEBOOK_EDITOR_CURSOR_BEGIN_END, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, SelectionStateType, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellRunState, NOTEBOOK_EDITOR_CURSOR_BEGIN_END, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; @@ -1058,148 +1058,6 @@ async function copyCell(context: INotebookCellActionContext, direction: 'up' | ' } } -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: 'notebook.cell.focusDown', - title: localize('notebookActions.focusDown', "Notebook Cell Focus Down"), - icon: icons.moveUpIcon, - keybinding: { - primary: KeyCode.DownArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - const viewModel = context.notebookEditor.viewModel; - const primaryIndex = viewModel.getFocus().start; - const nextIndex = Math.min(primaryIndex + 1, viewModel.length - 1); - - const focus = { start: nextIndex, end: nextIndex + 1 }; - const existingSelections = viewModel.getSelections(); - let newSelections = []; - - if (existingSelections.length && existingSelections[0].start === primaryIndex && existingSelections[0].end === primaryIndex + 1) { - // focus == selection - newSelections = [focus]; - } else { - // no-op as we allow you to move focus once you are in multi select mode - newSelections = existingSelections; - } - - viewModel.updateSelectionsState({ - kind: SelectionStateType.Index, - focus: focus, - selections: newSelections - }); - - context.notebookEditor.revealCellRangeInView(focus); - } -}); - -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: 'notebook.cell.focusUp', - title: localize('notebookActions.focusUp', "Notebook Cell Focus Up"), - icon: icons.moveDownIcon, - keybinding: { - primary: KeyCode.UpArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - const viewModel = context.notebookEditor.viewModel; - const primaryIndex = viewModel.getFocus().start; - const nextIndex = Math.max(primaryIndex - 1, 0); - const focus = { start: nextIndex, end: nextIndex + 1 }; - const existingSelections = viewModel.getSelections(); - let newSelections = []; - - if (existingSelections.length === 1 && existingSelections[0].start === primaryIndex && existingSelections[0].end === primaryIndex + 1) { - // focus == selection - newSelections = [focus]; - } else { - // no-op as we allow you to move focus once you are in multi select mode - newSelections = existingSelections; - } - - viewModel.updateSelectionsState({ - kind: SelectionStateType.Index, - focus: focus, - selections: newSelections - }); - - context.notebookEditor.revealCellRangeInView(focus); - } -}); - -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: 'notebook.cell.expandSelectionDown', - title: localize('notebookActions.expandSelectionDown', "Notebook Cell Expand Selection Down"), - icon: icons.moveUpIcon, - keybinding: { - primary: KeyMod.Shift | KeyCode.DownArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - const viewModel = context.notebookEditor.viewModel; - const primaryIndex = viewModel.getFocus().start; - const nextIndex = Math.min(primaryIndex + 1, viewModel.length - 1); - const focus = { start: nextIndex, end: nextIndex + 1 }; - - viewModel.updateSelectionsState({ - kind: SelectionStateType.Index, - focus: focus, - selections: [focus, ...viewModel.getSelections()] - }); - context.notebookEditor.revealCellRangeInView(focus); - } -}); - -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: 'notebook.cell.expandSelectionUp', - title: localize('notebookActions.expandSelectionUp', "Notebook Expand Selection Up"), - icon: icons.moveDownIcon, - keybinding: { - primary: KeyMod.Shift | KeyCode.UpArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - const viewModel = context.notebookEditor.viewModel; - const primaryIndex = viewModel.getFocus().start; - const nextIndex = Math.max(primaryIndex - 1, 0); - const focus = { start: nextIndex, end: nextIndex + 1 }; - viewModel.updateSelectionsState({ - kind: SelectionStateType.Index, - focus: focus, - selections: [focus, ...viewModel.getSelections()] - }); - context.notebookEditor.revealCellRangeInView(focus); - } -}); - registerAction2(class extends NotebookCellAction { constructor() { super( diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 7cd0bcaf5f4..6deece89246 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -399,6 +399,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor keyboardSupport: false, mouseSupport: true, multipleSelectionSupport: true, + selectionNavigation: true, enableKeyboardNavigation: true, additionalScrollHeight: 0, transformOptimization: false, //(isMacintosh && isNative) || getTitleBarStyle(this.configurationService, this.environmentService) === 'native', From b4ebf03316c1f8bd3574311c89c1f1d5f67cc755 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 16:24:04 -0800 Subject: [PATCH 12/25] selection tests. --- .../notebook/test/notebookSelection.test.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts index 5461f5490cf..830a3299c3e 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts @@ -88,6 +88,43 @@ suite('NotebookCellList focus/selection', () => { }); }); + + test('notebook cell list focus/selection from UI', function () { + withTestNotebook( + instantiationService, + [ + ['# header a', 'markdown', CellKind.Markdown, [], {}], + ['var b = 1;', 'javascript', CellKind.Code, [], {}], + ['# header b', 'markdown', CellKind.Markdown, [], {}], + ['var b = 2;', 'javascript', CellKind.Code, [], {}], + ['# header c', 'markdown', CellKind.Markdown, [], {}] + ], + (editor, viewModel) => { + const cellList = createNotebookCellList(instantiationService); + cellList.attachViewModel(viewModel); + assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); + + // arrow down, move both focus and selections + cellList.setFocus([1], new KeyboardEvent('keydown'), undefined); + cellList.setSelection([1], new KeyboardEvent('keydown'), undefined); + assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); + + // shift+arrow down, expands selection + cellList.setFocus([2], new KeyboardEvent('keydown'), undefined); + cellList.setSelection([1, 2]); + assert.deepStrictEqual(viewModel.getFocus(), { start: 2, end: 3 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 1, end: 3 }]); + + // arrow down, will move focus but not expand selection + cellList.setFocus([3], new KeyboardEvent('keydown'), undefined); + assert.deepStrictEqual(viewModel.getFocus(), { start: 3, end: 4 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 1, end: 3 }]); + }); + }); + + test('notebook cell list focus/selection with folding regions', function () { withTestNotebook( instantiationService, From 5e2f573d1dd77eaa903eeeb4896c4e067b63b55d Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 19:25:16 -0800 Subject: [PATCH 13/25] cut multi cells w/ folding. --- .../contrib/clipboard/notebookClipboard.ts | 61 +++++++++++++++++-- .../notebook/browser/notebookBrowser.ts | 20 ++++++ .../notebook/browser/notebookEditorWidget.ts | 43 +++++++++++++ .../notebook/browser/view/notebookCellList.ts | 19 +++++- .../notebook/test/notebookSelection.test.ts | 28 +++++++++ .../notebook/test/testNotebookEditor.ts | 9 +++ 6 files changed, 173 insertions(+), 7 deletions(-) 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 dfe8ee42636..f40699b5b6e 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts @@ -8,13 +8,13 @@ import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions as WorkbenchExtensions, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; -import { getNotebookEditorFromEditorPane, ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { getNotebookEditorFromEditorPane, ICellViewModel, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import * as UUID from 'vs/base/common/uuid'; import { CopyAction, CutAction, PasteAction } from 'vs/editor/contrib/clipboard/clipboard'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; -import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; +import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { CellEditType, ICellEditOperation, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, cellRangesToIndexes, ICellEditOperation, ICellRange, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; class NotebookClipboardContribution extends Disposable { @@ -46,6 +46,10 @@ class NotebookClipboardContribution extends Disposable { return false; } + if (!editor.hasModel()) { + return false; + } + if (editor.hasOutputTextSelection()) { document.execCommand('copy'); return true; @@ -53,7 +57,8 @@ class NotebookClipboardContribution extends Disposable { const clipboardService = accessor.get(IClipboardService); const notebookService = accessor.get(INotebookService); - const selectedCells = editor.getSelectionViewModels(); + const selectionRanges = this._getModelRangesFromViewRanges(editor, editor.viewModel, editor.viewModel.getSelections()); + const selectedCells = this._cellRangeToViewCells(editor.viewModel, selectionRanges); if (!selectedCells.length) { return false; @@ -167,7 +172,8 @@ class NotebookClipboardContribution extends Disposable { const clipboardService = accessor.get(IClipboardService); const notebookService = accessor.get(INotebookService); - const selectedCells = editor.getSelectionViewModels(); + const selectionRanges = this._getModelRangesFromViewRanges(editor, viewModel, viewModel.getSelections()); + const selectedCells = this._cellRangeToViewCells(viewModel, selectionRanges); if (!selectedCells.length) { return false; @@ -181,7 +187,7 @@ class NotebookClipboardContribution extends Disposable { ? firstSelectIndex : viewModel.notebookDocument.cells.length - 1; - viewModel.notebookDocument.applyEdits(edits, true, { kind: SelectionStateType.Index, focus: viewModel.getFocus(), selections: viewModel.getSelections() }, () => { + viewModel.notebookDocument.applyEdits(edits, true, { kind: SelectionStateType.Index, focus: viewModel.getFocus(), selections: selectionRanges }, () => { return { kind: SelectionStateType.Index, focus: { start: newFocusedCellIndex, end: newFocusedCellIndex + 1 }, @@ -194,6 +200,49 @@ class NotebookClipboardContribution extends Disposable { }); } } + + private _cellRangeToViewCells(viewModel: NotebookViewModel, ranges: ICellRange[]) { + const cells: ICellViewModel[] = []; + ranges.forEach(range => { + cells.push(...viewModel.viewCells.slice(range.start, range.end)); + }); + + return cells; + } + + /** + * It will include hidden cells + * @param editor + * @param viewModel + * @param ranges + * @returns + */ + private _getModelRangesFromViewRanges(editor: INotebookEditor, viewModel: NotebookViewModel, ranges: ICellRange[]) { + // assuming ranges are sorted and no overlap + const indexes = cellRangesToIndexes(ranges); + let modelRanges: ICellRange[] = []; + indexes.forEach(index => { + const viewCell = viewModel.viewCells[index]; + + if (!viewCell) { + return; + } + + const viewIndex = editor.getViewIndex(viewCell); + if (viewIndex < 0) { + return; + } + + const nextViewIndex = viewIndex + 1; + const range = editor.getCellRangeFromViewRange(viewIndex, nextViewIndex); + + if (range) { + modelRanges.push(range); + } + }); + + return modelRanges; + } } const workbenchContributionsRegistry = Registry.as(WorkbenchExtensions.Workbench); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 64d35c01954..6d28f727ac5 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -565,6 +565,23 @@ export interface INotebookEditor extends ICommonNotebookEditor { */ revealRangeInCenterIfOutsideViewportAsync(cell: ICellViewModel, range: Range): Promise; + /** + * Get the view index of a cell + */ + getViewIndex(cell: ICellViewModel): number; + + /** + * @param startIndex Inclusive + * @param endIndex Exclusive + */ + getCellRangeFromViewRange(startIndex: number, endIndex: number): ICellRange | undefined; + + /** + * @param startIndex Inclusive + * @param endIndex Exclusive + */ + getCellsFromViewRange(startIndex: number, endIndex: number): ICellViewModel[]; + /** * Set hidden areas on cell text models. */ @@ -634,6 +651,9 @@ export interface INotebookCellList { attachViewModel(viewModel: NotebookViewModel): void; clear(): void; getViewIndex(cell: ICellViewModel): number | undefined; + getViewIndex2(modelIndex: number): number | undefined; + getModelIndex(cell: CellViewModel): number | undefined; + getModelIndex2(viewIndex: number): number | undefined; focusElement(element: ICellViewModel): void; selectElement(element: ICellViewModel): void; getFocusedElements(): ICellViewModel[]; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 6deece89246..b154f63c1ec 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -1160,6 +1160,49 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return this._list.revealElementRangeInCenterIfOutsideViewportAsync(cell, range); } + getViewIndex(cell: ICellViewModel): number { + if (!this._list) { + return -1; + } + return this._list.getViewIndex(cell) ?? -1; + } + + getCellRangeFromViewRange(startIndex: number, endIndex: number): ICellRange | undefined { + if (!this.viewModel) { + return undefined; + } + + const modelIndex = this._list.getModelIndex2(startIndex); + if (modelIndex === undefined) { + throw new Error(`startIndex ${startIndex} out of boundary`); + } + + if (endIndex >= this._list.length) { + // it's the end + const endModelIndex = this.viewModel.length; + return { start: modelIndex, end: endModelIndex }; + } else { + const endModelIndex = this._list.getModelIndex2(endIndex); + if (endModelIndex === undefined) { + throw new Error(`endIndex ${endIndex} out of boundary`); + } + return { start: modelIndex, end: endModelIndex }; + } + } + + getCellsFromViewRange(startIndex: number, endIndex: number): ICellViewModel[] { + if (!this.viewModel) { + return []; + } + + const range = this.getCellRangeFromViewRange(startIndex, endIndex); + if (!range) { + return []; + } + + return this.viewModel.viewCells.slice(range.start, range.end); + } + setCellEditorSelection(cell: ICellViewModel, range: Range): void { this._list.setCellSelection(cell, range); } diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 4c17c0f92fa..12e7f661e02 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -490,8 +490,26 @@ export class NotebookCellList extends WorkbenchList implements ID } } + getModelIndex(cell: CellViewModel): number | undefined { + const viewIndex = this.indexOf(cell); + return this.getModelIndex2(viewIndex); + } + + getModelIndex2(viewIndex: number): number | undefined { + if (!this.hiddenRangesPrefixSum) { + return viewIndex; + } + + const modelIndex = this.hiddenRangesPrefixSum.getAccumulatedValue(viewIndex - 1); + return modelIndex; + } + getViewIndex(cell: ICellViewModel) { const modelIndex = this._viewModel!.getCellIndex(cell); + return this.getViewIndex2(modelIndex); + } + + getViewIndex2(modelIndex: number): number | undefined { if (!this.hiddenRangesPrefixSum) { return modelIndex; } @@ -509,7 +527,6 @@ export class NotebookCellList extends WorkbenchList implements ID } } - private _getViewIndexUpperBound(cell: ICellViewModel): number { if (!this._viewModel) { return -1; diff --git a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts index 830a3299c3e..8f75e166c70 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts @@ -170,6 +170,34 @@ suite('NotebookCellList focus/selection', () => { }); }); + test('notebook cell list view/model api', function () { + withTestNotebook( + instantiationService, + [ + ['# header a', 'markdown', CellKind.Markdown, [], {}], + ['var b = 1;', 'javascript', CellKind.Code, [], {}], + ['# header b', 'markdown', CellKind.Markdown, [], {}], + ['var b = 2;', 'javascript', CellKind.Code, [], {}], + ['# header c', 'markdown', CellKind.Markdown, [], {}] + ], + (editor, viewModel) => { + const foldingModel = new FoldingModel(); + foldingModel.attachViewModel(viewModel); + + const cellList = createNotebookCellList(instantiationService); + cellList.attachViewModel(viewModel); + + updateFoldingStateAtIndex(foldingModel, 0, true); + updateFoldingStateAtIndex(foldingModel, 2, true); + + assert.deepStrictEqual(cellList.getModelIndex2(-1), undefined); + assert.deepStrictEqual(cellList.getModelIndex2(0), 0); + assert.deepStrictEqual(cellList.getModelIndex2(1), 2); + assert.deepStrictEqual(cellList.getModelIndex2(2), 4); + }); + }); + + test('notebook validate range', () => { withTestNotebook( instantiationService, diff --git a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts index 7cf2272b749..2c64ee3bb28 100644 --- a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts @@ -67,6 +67,15 @@ export class TestNotebookEditor implements INotebookEditor { creationOptions: INotebookEditorCreationOptions = { isEmbedded: false }; constructor(readonly viewModel: NotebookViewModel) { } + getCellRangeFromViewRange(startIndex: number, endIndex: number): ICellRange | undefined { + throw new Error('Method not implemented.'); + } + getViewIndex(cell: ICellViewModel): number { + throw new Error('Method not implemented.'); + } + getCellsFromViewRange(startIndex: number, endIndex: number): ICellViewModel[] { + throw new Error('Method not implemented.'); + } getFocus(): ICellRange | undefined { throw new Error('Method not implemented.'); From 3dff030ea14f29f87d633e21159449633ceff0df Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 19:33:40 -0800 Subject: [PATCH 14/25] delete folded cell would not change other folding states. --- .../browser/contrib/clipboard/notebookClipboard.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 f40699b5b6e..69639b89742 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts @@ -180,9 +180,14 @@ class NotebookClipboardContribution extends Disposable { } clipboardService.writeText(selectedCells.map(cell => cell.getText()).join('\n')); - const selectionIndexes = selectedCells.map(cell => [cell, viewModel.getCellIndex(cell)] as [ICellViewModel, number]).sort((a, b) => b[1] - a[1]); - const edits: ICellEditOperation[] = selectionIndexes.map(value => ({ editType: CellEditType.Replace, index: value[1], count: 1, cells: [] })); - const firstSelectIndex = selectionIndexes.sort((a, b) => a[1] - b[1])[0][1]; + const edits: ICellEditOperation[] = selectionRanges.map(range => ({ editType: CellEditType.Replace, index: range.start, count: range.end - range.start, cells: [] })); + const firstSelectIndex = selectionRanges[0].start; + + /** + * If we have cells, 0, 1, 2, 3, 4, 5, 6 + * and cells 1, 2 are selected, and then we delete cells 1 and 2 + * the new focused cell should still be at index 1 + */ const newFocusedCellIndex = firstSelectIndex < viewModel.notebookDocument.cells.length ? firstSelectIndex : viewModel.notebookDocument.cells.length - 1; From 4f5824ea28bfb8f0674d08032f2199b1cd62bfa4 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 21:38:01 -0800 Subject: [PATCH 15/25] enable async tests. --- .../notebook/test/notebookSelection.test.ts | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts index 8f75e166c70..a6e90e44116 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts @@ -22,8 +22,8 @@ suite('NotebookSelection', () => { suite('NotebookCellList focus/selection', () => { const instantiationService = setupInstantiationService(); - test('notebook cell list setFocus', function () { - withTestNotebook( + test('notebook cell list setFocus', async function () { + await withTestNotebook( instantiationService, [ ['var a = 1;', 'javascript', CellKind.Code, [], {}], @@ -43,8 +43,8 @@ suite('NotebookCellList focus/selection', () => { }); }); - test('notebook cell list setSelections', function () { - withTestNotebook( + test('notebook cell list setSelections', async function () { + await withTestNotebook( instantiationService, [ ['var a = 1;', 'javascript', CellKind.Code, [], {}], @@ -65,8 +65,8 @@ suite('NotebookCellList focus/selection', () => { }); }); - test('notebook cell list setFocus', function () { - withTestNotebook( + test('notebook cell list setFocus', async function () { + await withTestNotebook( instantiationService, [ ['var a = 1;', 'javascript', CellKind.Code, [], {}], @@ -89,8 +89,8 @@ suite('NotebookCellList focus/selection', () => { }); - test('notebook cell list focus/selection from UI', function () { - withTestNotebook( + test('notebook cell list focus/selection from UI', async function () { + await withTestNotebook( instantiationService, [ ['# header a', 'markdown', CellKind.Markdown, [], {}], @@ -108,8 +108,8 @@ suite('NotebookCellList focus/selection', () => { // arrow down, move both focus and selections cellList.setFocus([1], new KeyboardEvent('keydown'), undefined); cellList.setSelection([1], new KeyboardEvent('keydown'), undefined); - assert.deepStrictEqual(viewModel.getFocus(), { start: 0, end: 1 }); - assert.deepStrictEqual(viewModel.getSelections(), [{ start: 0, end: 1 }]); + assert.deepStrictEqual(viewModel.getFocus(), { start: 1, end: 2 }); + assert.deepStrictEqual(viewModel.getSelections(), [{ start: 1, end: 2 }]); // shift+arrow down, expands selection cellList.setFocus([2], new KeyboardEvent('keydown'), undefined); @@ -125,8 +125,8 @@ suite('NotebookCellList focus/selection', () => { }); - test('notebook cell list focus/selection with folding regions', function () { - withTestNotebook( + test('notebook cell list focus/selection with folding regions', async function () { + await withTestNotebook( instantiationService, [ ['# header a', 'markdown', CellKind.Markdown, [], {}], @@ -198,8 +198,8 @@ suite('NotebookCellList focus/selection', () => { }); - test('notebook validate range', () => { - withTestNotebook( + test('notebook validate range', async () => { + await withTestNotebook( instantiationService, [ ['# header a', 'markdown', CellKind.Markdown, [], {}], @@ -218,8 +218,8 @@ suite('NotebookCellList focus/selection', () => { }); }); - test('notebook updateSelectionState', function () { - withTestNotebook( + test('notebook updateSelectionState', async function () { + await withTestNotebook( instantiationService, [ ['# header a', 'markdown', CellKind.Markdown, [], {}], @@ -231,8 +231,8 @@ suite('NotebookCellList focus/selection', () => { }); }); - test('notebook cell selection w/ cell deletion', function () { - withTestNotebook( + test('notebook cell selection w/ cell deletion', async function () { + await withTestNotebook( instantiationService, [ ['# header a', 'markdown', CellKind.Markdown, [], {}], From fe9466a6e1d4af2b2d9d64fdf14a95694108f3e5 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 21:40:50 -0800 Subject: [PATCH 16/25] false negative hidden ranges test --- .../contrib/notebook/test/notebookSelection.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts index a6e90e44116..d000909648a 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts @@ -170,8 +170,8 @@ suite('NotebookCellList focus/selection', () => { }); }); - test('notebook cell list view/model api', function () { - withTestNotebook( + test('notebook cell list getModelIndex', async function () { + await withTestNotebook( instantiationService, [ ['# header a', 'markdown', CellKind.Markdown, [], {}], @@ -189,8 +189,10 @@ suite('NotebookCellList focus/selection', () => { updateFoldingStateAtIndex(foldingModel, 0, true); updateFoldingStateAtIndex(foldingModel, 2, true); + viewModel.updateFoldingRanges(foldingModel.regions); + cellList.setHiddenAreas(viewModel.getHiddenRanges(), true); - assert.deepStrictEqual(cellList.getModelIndex2(-1), undefined); + assert.deepStrictEqual(cellList.getModelIndex2(-1), 0); assert.deepStrictEqual(cellList.getModelIndex2(0), 0); assert.deepStrictEqual(cellList.getModelIndex2(1), 2); assert.deepStrictEqual(cellList.getModelIndex2(2), 4); From ebc6eade48381f60e6e3f3155e061ee689484f13 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 22:07:44 -0800 Subject: [PATCH 17/25] update prefix sum always when edits happen --- .../notebook/browser/view/notebookCellList.ts | 18 ++++-- .../notebook/test/notebookSelection.test.ts | 60 ++++++++++++++++++- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 12e7f661e02..345e5e8e6bd 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -404,6 +404,8 @@ export class NotebookCellList extends WorkbenchList implements ID } if (!hasDifference) { + // they call 'setHiddenAreas' for a reason, even if the ranges are still the same, it's possible that the hiddenRangeSum is not update to date + this._updateHiddenRangePrefixSum(newRanges); return false; } } @@ -414,6 +416,16 @@ export class NotebookCellList extends WorkbenchList implements ID this._hiddenRangeIds = hiddenAreaIds; // set hidden ranges prefix sum + this._updateHiddenRangePrefixSum(newRanges); + + if (triggerViewUpdate) { + this.updateHiddenAreasInView(oldRanges, newRanges); + } + + return true; + } + + private _updateHiddenRangePrefixSum(newRanges: ICellRange[]) { let start = 0; let index = 0; const ret: number[] = []; @@ -438,12 +450,6 @@ export class NotebookCellList extends WorkbenchList implements ID } this.hiddenRangesPrefixSum = new PrefixSumComputer(values); - - if (triggerViewUpdate) { - this.updateHiddenAreasInView(oldRanges, newRanges); - } - - return true; } /** diff --git a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts index d000909648a..c7dd1c62c1d 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookSelection.test.ts @@ -4,10 +4,11 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; +import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { FoldingModel, updateFoldingStateAtIndex } from 'vs/workbench/contrib/notebook/browser/contrib/fold/foldingModel'; import { NotebookCellSelectionCollection } from 'vs/workbench/contrib/notebook/browser/viewModel/cellSelectionCollection'; -import { CellKind, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { createNotebookCellList, setupInstantiationService, withTestNotebook } from 'vs/workbench/contrib/notebook/test/testNotebookEditor'; +import { CellEditType, CellKind, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { createNotebookCellList, setupInstantiationService, TestCell, withTestNotebook } from 'vs/workbench/contrib/notebook/test/testNotebookEditor'; suite('NotebookSelection', () => { test('focus is never empty', function () { @@ -21,6 +22,7 @@ suite('NotebookSelection', () => { suite('NotebookCellList focus/selection', () => { const instantiationService = setupInstantiationService(); + const textModelService = instantiationService.get(ITextModelService); test('notebook cell list setFocus', async function () { await withTestNotebook( @@ -170,6 +172,60 @@ suite('NotebookCellList focus/selection', () => { }); }); + test('notebook cell list focus/selection with folding regions and applyEdits', async function () { + await withTestNotebook( + instantiationService, + [ + ['# header a', 'markdown', CellKind.Markdown, [], {}], + ['var b = 1;', 'javascript', CellKind.Code, [], {}], + ['# header b', 'markdown', CellKind.Markdown, [], {}], + ['var b = 2;', 'javascript', CellKind.Code, [], {}], + ['var c = 3', 'javascript', CellKind.Markdown, [], {}], + ['# header d', 'markdown', CellKind.Markdown, [], {}], + ['var e = 4;', 'javascript', CellKind.Code, [], {}], + ], + (editor, viewModel) => { + const foldingModel = new FoldingModel(); + foldingModel.attachViewModel(viewModel); + + const cellList = createNotebookCellList(instantiationService); + cellList.attachViewModel(viewModel); + cellList.setFocus([0]); + cellList.setSelection([0]); + + updateFoldingStateAtIndex(foldingModel, 0, true); + updateFoldingStateAtIndex(foldingModel, 2, true); + viewModel.updateFoldingRanges(foldingModel.regions); + cellList.setHiddenAreas(viewModel.getHiddenRanges(), true); + assert.strictEqual(cellList.getModelIndex2(0), 0); + assert.strictEqual(cellList.getModelIndex2(1), 2); + + viewModel.notebookDocument.applyEdits([{ + editType: CellEditType.Replace, index: 0, count: 2, cells: [] + }], true, undefined, () => undefined, undefined, false); + viewModel.updateFoldingRanges(foldingModel.regions); + cellList.setHiddenAreas(viewModel.getHiddenRanges(), true); + + assert.strictEqual(cellList.getModelIndex2(0), 0); + assert.strictEqual(cellList.getModelIndex2(1), 3); + + // mimic undo + viewModel.notebookDocument.applyEdits([{ + editType: CellEditType.Replace, index: 0, count: 0, cells: [ + new TestCell(viewModel.viewType, 7, '# header f', 'markdown', CellKind.Code, [], textModelService), + new TestCell(viewModel.viewType, 8, 'var g = 5;', 'javascript', CellKind.Code, [], textModelService) + ] + }], true, undefined, () => undefined, undefined, false); + viewModel.updateFoldingRanges(foldingModel.regions); + cellList.setHiddenAreas(viewModel.getHiddenRanges(), true); + assert.strictEqual(cellList.getModelIndex2(0), 0); + assert.strictEqual(cellList.getModelIndex2(1), 1); + assert.strictEqual(cellList.getModelIndex2(2), 2); + + + }); + }); + test('notebook cell list getModelIndex', async function () { await withTestNotebook( instantiationService, From 1dce5d35536d3bcedd0369c8399655c9f16e28e4 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 11 Mar 2021 22:11:04 -0800 Subject: [PATCH 18/25] replaceCells as a single edit. --- .../contrib/clipboard/notebookClipboard.ts | 4 +-- .../contrib/notebook/common/model/cellEdit.ts | 25 ++++++------------- .../common/model/notebookTextModel.ts | 22 ++++++++++++++++ .../contrib/notebook/common/notebookCommon.ts | 4 +++ 4 files changed, 35 insertions(+), 20 deletions(-) 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 69639b89742..bb2dfd3be0c 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts @@ -14,7 +14,7 @@ import { CopyAction, CutAction, PasteAction } from 'vs/editor/contrib/clipboard/ import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { CellEditType, cellRangesToIndexes, ICellEditOperation, ICellRange, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, cellRangesToIndexes, ICellEditOperation, ICellRange, reduceRanges, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; class NotebookClipboardContribution extends Disposable { @@ -246,7 +246,7 @@ class NotebookClipboardContribution extends Disposable { } }); - return modelRanges; + return reduceRanges(modelRanges); } } diff --git a/src/vs/workbench/contrib/notebook/common/model/cellEdit.ts b/src/vs/workbench/contrib/notebook/common/model/cellEdit.ts index 52d2a01a081..5bfc431dc34 100644 --- a/src/vs/workbench/contrib/notebook/common/model/cellEdit.ts +++ b/src/vs/workbench/contrib/notebook/common/model/cellEdit.ts @@ -14,6 +14,7 @@ import { ISelectionState, NotebookCellMetadata } from 'vs/workbench/contrib/note export interface ITextCellEditingDelegate { insertCell?(index: number, cell: NotebookCellTextModel, endSelections?: ISelectionState): void; deleteCell?(index: number, endSelections?: ISelectionState): void; + replaceCell?(index: number, count: number, cells: NotebookCellTextModel[], endSelections?: ISelectionState): void; moveCell?(fromIndex: number, length: number, toIndex: number, beforeSelections: ISelectionState | undefined, endSelections?: ISelectionState): void; updateCellMetadata?(index: number, newMetadata: NotebookCellMetadata): void; } @@ -63,34 +64,22 @@ export class SpliceCellsEdit implements IResourceUndoRedoElement { } undo(): void { - if (!this.editingDelegate.deleteCell || !this.editingDelegate.insertCell) { - throw new Error('Notebook Insert/Delete Cell not implemented for Undo/Redo'); + if (!this.editingDelegate.replaceCell) { + throw new Error('Notebook Replace Cell not implemented for Undo/Redo'); } this.diffs.forEach(diff => { - for (let i = 0; i < diff[2].length; i++) { - this.editingDelegate.deleteCell!(diff[0], this.beforeHandles); - } - - diff[1].reverse().forEach(cell => { - this.editingDelegate.insertCell!(diff[0], cell, this.beforeHandles); - }); + this.editingDelegate.replaceCell!(diff[0], diff[2].length, diff[1], this.beforeHandles); }); } redo(): void { - if (!this.editingDelegate.deleteCell || !this.editingDelegate.insertCell) { - throw new Error('Notebook Insert/Delete Cell not implemented for Undo/Redo'); + if (!this.editingDelegate.replaceCell) { + throw new Error('Notebook Replace 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], this.endHandles); - } - - diff[2].reverse().forEach(cell => { - this.editingDelegate.insertCell!(diff[0], cell, this.endHandles); - }); + this.editingDelegate.replaceCell!(diff[0], diff[1].length, diff[2], this.endHandles); }); } } diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index 0246e87346e..3c4d8eb1c8d 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -414,6 +414,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._operationManager.pushEditOperation(new SpliceCellsEdit(this.uri, undoDiff, { insertCell: (index, cell, endSelections) => { this._insertNewCell(index, [cell], true, endSelections); }, deleteCell: (index, endSelections) => { this._removeCell(index, 1, true, endSelections); }, + replaceCell: (index, count, cells, endSelections) => { this._replaceNewCells(index, count, cells, true, endSelections); }, }, undefined, undefined), undefined, undefined); } @@ -505,6 +506,27 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._eventEmitter.emit({ kind: NotebookCellsChangeType.ModelChange, changes: [[index, count, []]], transient: false }, synchronous, endSelections); } + private _replaceNewCells(index: number, count: number, cells: NotebookCellTextModel[], synchronous: boolean, endSelections: ISelectionState | undefined) { + for (let i = index; i < index + count; i++) { + const cell = this._cells[i]; + this._cellListeners.get(cell.handle)?.dispose(); + this._cellListeners.delete(cell.handle); + } + + for (let i = 0; i < cells.length; i++) { + this._mapping.set(cells[i].handle, cells[i]); + const dirtyStateListener = cells[i].onDidChangeContent(() => { + this._eventEmitter.emit({ kind: NotebookCellsChangeType.ChangeCellContent, transient: false }, true); + }); + + this._cellListeners.set(cells[i].handle, dirtyStateListener); + } + + this._cells.splice(index, count, ...cells); + this._eventEmitter.emit({ kind: NotebookCellsChangeType.ModelChange, changes: [[index, count, cells]], transient: false }, synchronous, endSelections); + + } + private _isCellMetadataChanged(a: NotebookCellMetadata, b: NotebookCellMetadata) { const keys = new Set([...Object.keys(a || {}), ...Object.keys(b || {})]); for (let key of keys) { diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 8952279114a..68c44efb4ca 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -818,6 +818,10 @@ export function cellRangesToIndexes(ranges: ICellRange[]) { return indexes; } +/** + * todo@rebornix notebookBrowser.reduceCellRanges + * @returns + */ export function reduceRanges(ranges: ICellRange[]) { const sorted = ranges.sort((a, b) => a.start - b.start); const first = sorted[0]; From e545c0750b8d1e9689d0e1de78a415964bb7e668 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 12 Mar 2021 08:05:22 -0800 Subject: [PATCH 19/25] copy paste with hidden cells. --- .../contrib/clipboard/notebookClipboard.ts | 42 ++----------------- .../notebook/browser/notebookBrowser.ts | 27 ++++++++++++ 2 files changed, 31 insertions(+), 38 deletions(-) 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 bb2dfd3be0c..6ee9c3c6e7c 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts @@ -8,13 +8,13 @@ import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions as WorkbenchExtensions, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; -import { getNotebookEditorFromEditorPane, ICellViewModel, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { expandCellRangesWithHiddenCells, getNotebookEditorFromEditorPane, ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import * as UUID from 'vs/base/common/uuid'; import { CopyAction, CutAction, PasteAction } from 'vs/editor/contrib/clipboard/clipboard'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { CellEditType, cellRangesToIndexes, ICellEditOperation, ICellRange, reduceRanges, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, ICellEditOperation, ICellRange, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; class NotebookClipboardContribution extends Disposable { @@ -57,7 +57,7 @@ class NotebookClipboardContribution extends Disposable { const clipboardService = accessor.get(IClipboardService); const notebookService = accessor.get(INotebookService); - const selectionRanges = this._getModelRangesFromViewRanges(editor, editor.viewModel, editor.viewModel.getSelections()); + const selectionRanges = expandCellRangesWithHiddenCells(editor, editor.viewModel, editor.viewModel.getSelections()); const selectedCells = this._cellRangeToViewCells(editor.viewModel, selectionRanges); if (!selectedCells.length) { @@ -172,7 +172,7 @@ class NotebookClipboardContribution extends Disposable { const clipboardService = accessor.get(IClipboardService); const notebookService = accessor.get(INotebookService); - const selectionRanges = this._getModelRangesFromViewRanges(editor, viewModel, viewModel.getSelections()); + const selectionRanges = expandCellRangesWithHiddenCells(editor, viewModel, viewModel.getSelections()); const selectedCells = this._cellRangeToViewCells(viewModel, selectionRanges); if (!selectedCells.length) { @@ -214,40 +214,6 @@ class NotebookClipboardContribution extends Disposable { return cells; } - - /** - * It will include hidden cells - * @param editor - * @param viewModel - * @param ranges - * @returns - */ - private _getModelRangesFromViewRanges(editor: INotebookEditor, viewModel: NotebookViewModel, ranges: ICellRange[]) { - // assuming ranges are sorted and no overlap - const indexes = cellRangesToIndexes(ranges); - let modelRanges: ICellRange[] = []; - indexes.forEach(index => { - const viewCell = viewModel.viewCells[index]; - - if (!viewCell) { - return; - } - - const viewIndex = editor.getViewIndex(viewCell); - if (viewIndex < 0) { - return; - } - - const nextViewIndex = viewIndex + 1; - const range = editor.getCellRangeFromViewRange(viewIndex, nextViewIndex); - - if (range) { - modelRanges.push(range); - } - }); - - return reduceRanges(modelRanges); - } } const workbenchContributionsRegistry = Registry.as(WorkbenchExtensions.Workbench); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 6d28f727ac5..530907f558c 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -896,3 +896,30 @@ export function updateEditorTopPadding(top: number) { export function getEditorTopPadding() { return EDITOR_TOP_PADDING; } + +export function expandCellRangesWithHiddenCells(editor: INotebookEditor, viewModel: NotebookViewModel, ranges: ICellRange[]) { + // assuming ranges are sorted and no overlap + const indexes = cellRangesToIndexes(ranges); + let modelRanges: ICellRange[] = []; + indexes.forEach(index => { + const viewCell = viewModel.viewCells[index]; + + if (!viewCell) { + return; + } + + const viewIndex = editor.getViewIndex(viewCell); + if (viewIndex < 0) { + return; + } + + const nextViewIndex = viewIndex + 1; + const range = editor.getCellRangeFromViewRange(viewIndex, nextViewIndex); + + if (range) { + modelRanges.push(range); + } + }); + + return reduceRanges(modelRanges); +} From 458904ed7a3c5b542eee95cba5cf5015f89ea84c Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 12 Mar 2021 08:06:45 -0800 Subject: [PATCH 20/25] move cells with multi select --- .../contrib/cellOperations/cellOperations.ts | 136 ++++++++++++++++++ .../notebook/browser/contrib/coreActions.ts | 55 +------ .../notebook/browser/notebook.contribution.ts | 1 + .../notebook/browser/notebookBrowser.ts | 3 +- .../contrib/notebook/common/notebookCommon.ts | 10 ++ 5 files changed, 150 insertions(+), 55 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts b/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts new file mode 100644 index 00000000000..fba975d0773 --- /dev/null +++ b/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts @@ -0,0 +1,136 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; +import { localize } from 'vs/nls'; +import { registerAction2 } from 'vs/platform/actions/common/actions'; +import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; +import { InputFocusedContext } from 'vs/platform/contextkey/common/contextkeys'; +import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; +import { INotebookCellActionContext, NotebookCellAction } from 'vs/workbench/contrib/notebook/browser/contrib/coreActions'; +import { expandCellRangesWithHiddenCells, NOTEBOOK_EDITOR_FOCUSED } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; +import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; +import { CellEditType, cellRangeContains, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; + +const MOVE_CELL_UP_COMMAND_ID = 'notebook.cell.moveUp'; +const MOVE_CELL_DOWN_COMMAND_ID = 'notebook.cell.moveDown'; + +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: MOVE_CELL_UP_COMMAND_ID, + title: localize('notebookActions.moveCellUp', "Move Cell Up"), + icon: icons.moveUpIcon, + keybinding: { + primary: KeyMod.Alt | KeyCode.UpArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + return moveCellRange(context, 'up'); + } +}); + +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: MOVE_CELL_DOWN_COMMAND_ID, + title: localize('notebookActions.moveCellDown', "Move Cell Down"), + icon: icons.moveDownIcon, + keybinding: { + primary: KeyMod.Alt | KeyCode.DownArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + return moveCellRange(context, 'down'); + } +}); + + +async function moveCellRange(context: INotebookCellActionContext, direction: 'up' | 'down'): Promise { + const viewModel = context.notebookEditor.viewModel; + if (!viewModel) { + return; + } + + if (!viewModel.metadata.editable) { + return; + } + + const selections = context.notebookEditor.getSelections(); + const modelRanges = expandCellRangesWithHiddenCells(context.notebookEditor, context.notebookEditor.viewModel!, selections); + const range = modelRanges[0]; + if (!range || range.start === range.end) { + return; + } + + if (direction === 'up') { + if (range.start === 0) { + return; + } + + const indexAbove = range.start - 1; + const finalSelection = { start: range.start - 1, end: range.end - 1 }; + const focus = context.notebookEditor.getFocus(); + const newFocus = cellRangeContains(range, focus) ? { start: focus.start - 1, end: focus.end - 1 } : { start: range.start - 1, end: range.start }; + viewModel.notebookDocument.applyEdits([ + { + editType: CellEditType.Move, + index: indexAbove, + length: 1, + newIdx: range.end - 1 + }], + true, + { + kind: SelectionStateType.Index, + focus: viewModel.getFocus(), + selections: viewModel.getSelections() + }, + () => ({ kind: SelectionStateType.Index, focus: newFocus, selections: [finalSelection] }), + undefined + ); + const focusRange = viewModel.getSelections()[0] ?? viewModel.getFocus(); + context.notebookEditor.revealCellRangeInView(focusRange); + } else { + if (range.end >= viewModel.length) { + return; + } + + const indexBelow = range.end; + const finalSelection = { start: range.start + 1, end: range.end + 1 }; + const focus = context.notebookEditor.getFocus(); + const newFocus = cellRangeContains(range, focus) ? { start: focus.start + 1, end: focus.end + 1 } : { start: range.start + 1, end: range.start + 2 }; + + viewModel.notebookDocument.applyEdits([ + { + editType: CellEditType.Move, + index: indexBelow, + length: 1, + newIdx: range.start + }], + true, + { + kind: SelectionStateType.Index, + focus: viewModel.getFocus(), + selections: viewModel.getSelections() + }, + () => ({ kind: SelectionStateType.Index, focus: newFocus, selections: [finalSelection] }), + undefined + ); + + const focusRange = viewModel.getSelections()[0] ?? viewModel.getFocus(); + context.notebookEditor.revealCellRangeInView(focusRange); + } +} diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index 3ee68e379b5..e37321c7c85 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -59,8 +59,6 @@ const EDIT_CELL_COMMAND_ID = 'notebook.cell.edit'; const QUIT_EDIT_CELL_COMMAND_ID = 'notebook.cell.quitEdit'; const DELETE_CELL_COMMAND_ID = 'notebook.cell.delete'; -const MOVE_CELL_UP_COMMAND_ID = 'notebook.cell.moveUp'; -const MOVE_CELL_DOWN_COMMAND_ID = 'notebook.cell.moveDown'; const COPY_CELL_COMMAND_ID = 'notebook.cell.copy'; const CUT_CELL_COMMAND_ID = 'notebook.cell.cut'; const PASTE_CELL_COMMAND_ID = 'notebook.cell.paste'; @@ -223,7 +221,7 @@ abstract class NotebookAction extends Action2 { } } -abstract class NotebookCellAction extends NotebookAction { +export abstract class NotebookCellAction extends NotebookAction { protected isCellActionContext(context?: unknown): context is INotebookCellActionContext { return !!context && !!(context as INotebookCellActionContext).notebookEditor && !!(context as INotebookCellActionContext).cell; } @@ -1038,17 +1036,6 @@ registerAction2(class extends NotebookCellAction { } }); -async function moveCell(context: INotebookCellActionContext, direction: 'up' | 'down'): Promise { - const result = direction === 'up' ? - await context.notebookEditor.moveCellUp(context.cell) : - await context.notebookEditor.moveCellDown(context.cell); - - if (result) { - // move cell command only works when the cell container has focus - context.notebookEditor.focusNotebookCell(result, 'container'); - } -} - async function copyCell(context: INotebookCellActionContext, direction: 'up' | 'down'): Promise { const text = context.cell.getText(); const newCellDirection = direction === 'up' ? 'above' : 'below'; @@ -1058,46 +1045,6 @@ async function copyCell(context: INotebookCellActionContext, direction: 'up' | ' } } -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: MOVE_CELL_UP_COMMAND_ID, - title: localize('notebookActions.moveCellUp', "Move Cell Up"), - icon: icons.moveUpIcon, - keybinding: { - primary: KeyMod.Alt | KeyCode.UpArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - return moveCell(context, 'up'); - } -}); - -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: MOVE_CELL_DOWN_COMMAND_ID, - title: localize('notebookActions.moveCellDown', "Move Cell Down"), - icon: icons.moveDownIcon, - keybinding: { - primary: KeyMod.Alt | KeyCode.DownArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - return moveCell(context, 'down'); - } -}); - registerAction2(class extends NotebookCellAction { constructor() { super( diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index f6eb9e6b126..70517294775 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -66,6 +66,7 @@ import 'vs/workbench/contrib/notebook/browser/contrib/marker/markerProvider'; import 'vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline'; import 'vs/workbench/contrib/notebook/browser/contrib/status/editorStatus'; import 'vs/workbench/contrib/notebook/browser/contrib/undoRedo/notebookUndoRedo'; +import 'vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations'; // Diff Editor Contribution diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 530907f558c..8a33cce9890 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -22,7 +22,7 @@ import { OutputRenderer } from 'vs/workbench/contrib/notebook/browser/view/outpu import { RunStateRenderer, TimerRenderer } from 'vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer'; import { CellViewModel, IModelDecorationsChangeAccessor, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { CellKind, NotebookCellMetadata, NotebookDocumentMetadata, INotebookKernel, ICellRange, IOrderedMimeType, INotebookRendererInfo, ICellOutput, IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, NotebookCellMetadata, NotebookDocumentMetadata, INotebookKernel, ICellRange, IOrderedMimeType, INotebookRendererInfo, ICellOutput, IOutputItemDto, cellRangesToIndexes, reduceRanges } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { Webview } from 'vs/workbench/contrib/webview/browser/webview'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { IMenu } from 'vs/platform/actions/common/actions'; @@ -231,6 +231,7 @@ export interface ICellViewModel extends IGenericCellViewModel { readonly model: NotebookCellTextModel; readonly id: string; readonly textBuffer: IReadonlyTextBuffer; + readonly layoutInfo: { totalHeight: number; }; dragging: boolean; handle: number; uri: URI; diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 68c44efb4ca..950626ea9f2 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -840,3 +840,13 @@ export function reduceRanges(ranges: ICellRange[]) { return prev; }, [first] as ICellRange[]); } + +/** + * todo@rebornix test and sort + * @param range + * @param other + * @returns + */ +export function cellRangeContains(range: ICellRange, other: ICellRange): boolean { + return other.start >= range.start && other.end <= range.end; +} From 34a537bb732356847843628b53d95b6093d34ea1 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 12 Mar 2021 10:27:27 -0800 Subject: [PATCH 21/25] sort numbers. --- src/vs/workbench/contrib/notebook/common/notebookCommon.ts | 2 +- src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 950626ea9f2..20d063da235 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -789,7 +789,7 @@ export interface INotebookDecorationRenderOptions { export function cellIndexesToRanges(indexes: number[]) { - indexes.sort(); + indexes.sort((a, b) => a - b); const first = indexes.shift(); if (first === undefined) { diff --git a/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts b/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts index b1845714f25..5abb1ffffd9 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookCommon.test.ts @@ -318,6 +318,9 @@ suite('CellRange', function () { assert.deepStrictEqual(cellIndexesToRanges([1, 0]), [{ start: 0, end: 2 }]); assert.deepStrictEqual(cellIndexesToRanges([1, 2, 0]), [{ start: 0, end: 3 }]); assert.deepStrictEqual(cellIndexesToRanges([3, 1, 0]), [{ start: 0, end: 2 }, { start: 3, end: 4 }]); + + assert.deepStrictEqual(cellIndexesToRanges([9, 10]), [{ start: 9, end: 11 }]); + assert.deepStrictEqual(cellIndexesToRanges([10, 9]), [{ start: 9, end: 11 }]); }); test('Reduce ranges', function () { From ad7983834afe94d5e64d74104e90cd336d2524ff Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 12 Mar 2021 10:29:28 -0800 Subject: [PATCH 22/25] extract clone cell. --- .../contrib/clipboard/notebookClipboard.ts | 31 ++----------------- .../common/model/notebookCellTextModel.ts | 25 +++++++++++++++ 2 files changed, 28 insertions(+), 28 deletions(-) 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 6ee9c3c6e7c..1c8864a87a2 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/clipboard/notebookClipboard.ts @@ -9,11 +9,10 @@ import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions as WorkbenchExtensions, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { expandCellRangesWithHiddenCells, getNotebookEditorFromEditorPane, ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import * as UUID from 'vs/base/common/uuid'; import { CopyAction, CutAction, PasteAction } from 'vs/editor/contrib/clipboard/clipboard'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; -import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; +import { cloneNotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { CellEditType, ICellEditOperation, ICellRange, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; @@ -96,35 +95,11 @@ class NotebookClipboardContribution extends Disposable { return false; } - const cloneMetadata = (cell: NotebookCellTextModel) => { - return { - editable: cell.metadata?.editable, - breakpointMargin: cell.metadata?.breakpointMargin, - hasExecutionOrder: cell.metadata?.hasExecutionOrder, - inputCollapsed: cell.metadata?.inputCollapsed, - outputCollapsed: cell.metadata?.outputCollapsed, - custom: cell.metadata?.custom - }; - }; - - const cloneCell = (cell: NotebookCellTextModel) => { - return { - source: cell.getValue(), - language: cell.language, - cellKind: cell.cellKind, - outputs: cell.outputs.map(output => ({ - outputs: output.outputs, - /* paste should generate new outputId */ outputId: UUID.generateUuid() - })), - metadata: cloneMetadata(cell) - }; - }; - if (activeCell) { const currCellIndex = viewModel.getCellIndex(activeCell); let topPastedCell: CellViewModel | undefined = undefined; - pasteCells.items.reverse().map(cell => cloneCell(cell)).forEach(pasteCell => { + pasteCells.items.reverse().map(cell => cloneNotebookCellTextModel(cell)).forEach(pasteCell => { const newIdx = typeof currCellIndex === 'number' ? currCellIndex + 1 : 0; topPastedCell = viewModel.createCell(newIdx, pasteCell.source, pasteCell.language, pasteCell.cellKind, pasteCell.metadata, pasteCell.outputs, true); }); @@ -138,7 +113,7 @@ class NotebookClipboardContribution extends Disposable { } let topPastedCell: CellViewModel | undefined = undefined; - pasteCells.items.reverse().map(cell => cloneCell(cell)).forEach(pasteCell => { + pasteCells.items.reverse().map(cell => cloneNotebookCellTextModel(cell)).forEach(pasteCell => { topPastedCell = viewModel.createCell(0, pasteCell.source, pasteCell.language, pasteCell.cellKind, pasteCell.metadata, pasteCell.outputs, true); }); diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts index 08eb5a6a5c4..562043609ea 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts @@ -7,6 +7,7 @@ import { Emitter, Event } from 'vs/base/common/event'; import { ICell, NotebookCellOutputsSplice, CellKind, NotebookCellMetadata, NotebookDocumentMetadata, TransientOptions, IOutputDto, ICellOutput } 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 UUID from 'vs/base/common/uuid'; import * as model from 'vs/editor/common/model'; import { Range } from 'vs/editor/common/core/range'; import { Disposable } from 'vs/base/common/lifecycle'; @@ -179,3 +180,27 @@ export class NotebookCellTextModel extends Disposable implements ICell { super.dispose(); } } + +export function cloneMetadata(cell: NotebookCellTextModel) { + return { + editable: cell.metadata?.editable, + breakpointMargin: cell.metadata?.breakpointMargin, + hasExecutionOrder: cell.metadata?.hasExecutionOrder, + inputCollapsed: cell.metadata?.inputCollapsed, + outputCollapsed: cell.metadata?.outputCollapsed, + custom: cell.metadata?.custom + }; +} + +export function cloneNotebookCellTextModel(cell: NotebookCellTextModel) { + return { + source: cell.getValue(), + language: cell.language, + cellKind: cell.cellKind, + outputs: cell.outputs.map(output => ({ + outputs: output.outputs, + /* paste should generate new outputId */ outputId: UUID.generateUuid() + })), + metadata: cloneMetadata(cell) + }; +} From 5af4ccb87bee6aabacf64fdb3024f01f58c72fd7 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 12 Mar 2021 10:30:00 -0800 Subject: [PATCH 23/25] copy cells with multi select. --- .../contrib/cellOperations/cellOperations.ts | 110 +++++++++++++++++- .../notebook/browser/contrib/coreActions.ts | 49 -------- 2 files changed, 109 insertions(+), 50 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts b/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts index fba975d0773..714b2eb4c0a 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/cellOperations/cellOperations.ts @@ -13,10 +13,13 @@ import { INotebookCellActionContext, NotebookCellAction } from 'vs/workbench/con import { expandCellRangesWithHiddenCells, NOTEBOOK_EDITOR_FOCUSED } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; -import { CellEditType, cellRangeContains, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, cellRangeContains, cellRangesToIndexes, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { cloneNotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; const MOVE_CELL_UP_COMMAND_ID = 'notebook.cell.moveUp'; const MOVE_CELL_DOWN_COMMAND_ID = 'notebook.cell.moveDown'; +const COPY_CELL_UP_COMMAND_ID = 'notebook.cell.copyUp'; +const COPY_CELL_DOWN_COMMAND_ID = 'notebook.cell.copyDown'; registerAction2(class extends NotebookCellAction { constructor() { @@ -134,3 +137,108 @@ async function moveCellRange(context: INotebookCellActionContext, direction: 'up context.notebookEditor.revealCellRangeInView(focusRange); } } + +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: COPY_CELL_UP_COMMAND_ID, + title: localize('notebookActions.copyCellUp', "Copy Cell Up"), + keybinding: { + primary: KeyMod.Alt | KeyMod.Shift | KeyCode.UpArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + return copyCellRange(context, 'up'); + } +}); + +registerAction2(class extends NotebookCellAction { + constructor() { + super( + { + id: COPY_CELL_DOWN_COMMAND_ID, + title: localize('notebookActions.copyCellDown', "Copy Cell Down"), + keybinding: { + primary: KeyMod.Alt | KeyMod.Shift | KeyCode.DownArrow, + when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), + weight: KeybindingWeight.WorkbenchContrib + } + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { + return copyCellRange(context, 'down'); + } +}); + +async function copyCellRange(context: INotebookCellActionContext, direction: 'up' | 'down'): Promise { + const viewModel = context.notebookEditor.viewModel; + if (!viewModel) { + return; + } + + if (!viewModel.metadata.editable) { + return; + } + + const selections = context.notebookEditor.getSelections(); + const modelRanges = expandCellRangesWithHiddenCells(context.notebookEditor, context.notebookEditor.viewModel!, selections); + const range = modelRanges[0]; + if (!range || range.start === range.end) { + return; + } + + if (direction === 'up') { + // insert up, without changing focus and selections + const focus = viewModel.getFocus(); + const selections = viewModel.getSelections(); + viewModel.notebookDocument.applyEdits([ + { + editType: CellEditType.Replace, + index: range.end, + count: 0, + cells: cellRangesToIndexes([range]).map(index => cloneNotebookCellTextModel(viewModel.viewCells[index].model)) + }], + true, + { + kind: SelectionStateType.Index, + focus: focus, + selections: selections + }, + () => ({ kind: SelectionStateType.Index, focus: focus, selections: selections }), + undefined + ); + } else { + // insert down, move selections + const focus = viewModel.getFocus(); + const selections = viewModel.getSelections(); + const newCells = cellRangesToIndexes([range]).map(index => cloneNotebookCellTextModel(viewModel.viewCells[index].model)); + const countDelta = newCells.length; + const newFocus = { start: focus.start + countDelta, end: focus.end + countDelta }; + const newSelections = [{ start: range.start + countDelta, end: range.end + countDelta }]; + viewModel.notebookDocument.applyEdits([ + { + editType: CellEditType.Replace, + index: range.end, + count: 0, + cells: cellRangesToIndexes([range]).map(index => cloneNotebookCellTextModel(viewModel.viewCells[index].model)) + }], + true, + { + kind: SelectionStateType.Index, + focus: focus, + selections: selections + }, + () => ({ kind: SelectionStateType.Index, focus: newFocus, selections: newSelections }), + undefined + ); + + const focusRange = viewModel.getSelections()[0] ?? viewModel.getFocus(); + context.notebookEditor.revealCellRangeInView(focusRange); + } +} diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index e37321c7c85..b33fcb33b14 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -63,8 +63,6 @@ const COPY_CELL_COMMAND_ID = 'notebook.cell.copy'; const CUT_CELL_COMMAND_ID = 'notebook.cell.cut'; const PASTE_CELL_COMMAND_ID = 'notebook.cell.paste'; const PASTE_CELL_ABOVE_COMMAND_ID = 'notebook.cell.pasteAbove'; -const COPY_CELL_UP_COMMAND_ID = 'notebook.cell.copyUp'; -const COPY_CELL_DOWN_COMMAND_ID = 'notebook.cell.copyDown'; const SPLIT_CELL_COMMAND_ID = 'notebook.cell.split'; const JOIN_CELL_ABOVE_COMMAND_ID = 'notebook.cell.joinAbove'; const JOIN_CELL_BELOW_COMMAND_ID = 'notebook.cell.joinBelow'; @@ -1036,15 +1034,6 @@ registerAction2(class extends NotebookCellAction { } }); -async function copyCell(context: INotebookCellActionContext, direction: 'up' | 'down'): Promise { - const text = context.cell.getText(); - const newCellDirection = direction === 'up' ? 'above' : 'below'; - const newCell = context.notebookEditor.insertNotebookCell(context.cell, context.cell.cellKind, newCellDirection, text); - if (newCell) { - context.notebookEditor.focusNotebookCell(newCell, 'container'); - } -} - registerAction2(class extends NotebookCellAction { constructor() { super( @@ -1234,44 +1223,6 @@ registerAction2(class extends NotebookCellAction { } }); -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: COPY_CELL_UP_COMMAND_ID, - title: localize('notebookActions.copyCellUp', "Copy Cell Up"), - keybinding: { - primary: KeyMod.Alt | KeyMod.Shift | KeyCode.UpArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - return copyCell(context, 'up'); - } -}); - -registerAction2(class extends NotebookCellAction { - constructor() { - super( - { - id: COPY_CELL_DOWN_COMMAND_ID, - title: localize('notebookActions.copyCellDown', "Copy Cell Down"), - keybinding: { - primary: KeyMod.Alt | KeyMod.Shift | KeyCode.DownArrow, - when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, InputFocusedContext.toNegated()), - weight: KeybindingWeight.WorkbenchContrib - } - }); - } - - async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext) { - return copyCell(context, 'down'); - } -}); - registerAction2(class extends NotebookCellAction { constructor() { super({ From c1f954ce48be43c19ad6c3f1dfa49ead032b2fcc Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 12 Mar 2021 10:43:41 -0800 Subject: [PATCH 24/25] optimize scroll view from bottom. --- .../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 345e5e8e6bd..730330465aa 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -682,7 +682,7 @@ export class NotebookCellList extends WorkbenchList implements ID const endElementHeight = this.view.elementHeight(endIndex); if (endElementTop >= wrapperBottom) { - return this._revealInternal(startIndex, false, CellRevealPosition.Top); + return this._revealInternal(endIndex, false, CellRevealPosition.Bottom); } if (endElementTop < wrapperBottom) { From eac163c061119e1cc80bde5dd94b6508185b4cc1 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 12 Mar 2021 10:56:46 -0800 Subject: [PATCH 25/25] fix tests. --- .../src/singlefolder-tests/notebook.test.ts | 41 +------------------ 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts index 517033df83e..01c682f5678 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts @@ -286,28 +286,9 @@ suite('Notebook API tests', function () { ] }); - const secondCell = vscode.window.activeNotebookEditor!.document.cells[1]; - const moveCellEvent = asPromise(vscode.notebook.onDidChangeNotebookCells); await vscode.commands.executeCommand('notebook.cell.moveUp'); - const moveCellEventRet = await moveCellEvent; - assert.deepStrictEqual(moveCellEventRet, { - document: vscode.window.activeNotebookEditor!.document, - changes: [ - { - start: 1, - deletedCount: 1, - deletedItems: [secondCell], - items: [] - }, - { - start: 0, - deletedCount: 0, - deletedItems: [], - items: [vscode.window.activeNotebookEditor?.document.cells[0]] - } - ] - }); + await moveCellEvent; const cellOutputChange = asPromise(vscode.notebook.onDidChangeCellOutputs); await vscode.commands.executeCommand('notebook.cell.execute'); @@ -350,25 +331,7 @@ suite('Notebook API tests', function () { assert.strictEqual(vscode.window.activeNotebookEditor!.document.cells.indexOf(activeCell!), 0); const moveChange = asPromise(vscode.notebook.onDidChangeNotebookCells); await vscode.commands.executeCommand('notebook.cell.moveDown'); - const ret = await moveChange; - assert.deepStrictEqual(ret, { - document: vscode.window.activeNotebookEditor?.document, - changes: [ - { - start: 0, - deletedCount: 1, - deletedItems: [activeCell], - items: [] - }, - { - start: 1, - deletedCount: 0, - deletedItems: [], - items: [activeCell] - } - ] - }); - + await moveChange; await saveAllEditors(); await closeAllEditors();