From 412f45fa80a35f9dd6f5f1336e0415e099ab43c8 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 29 Mar 2021 16:22:34 -0700 Subject: [PATCH 01/25] re #111663. --- .../browser/diff/diffNestedCellViewModel.ts | 11 +++ .../notebook/browser/notebookBrowser.ts | 2 + .../view/renderers/backLayerWebView.ts | 34 +++++++++ .../browser/view/renderers/cellRenderer.ts | 9 +++ .../browser/view/renderers/webviewPreloads.ts | 76 ++++++++++++++++++- .../browser/viewModel/codeCellViewModel.ts | 10 +++ .../viewModel/markdownCellViewModel.ts | 9 +++ 7 files changed, 150 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffNestedCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffNestedCellViewModel.ts index bd9f8d2ad2a..84ef80fbea3 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffNestedCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffNestedCellViewModel.ts @@ -50,6 +50,17 @@ export class DiffNestedCellViewModel extends Disposable implements IDiffNestedCe this._hoveringOutput = v; this._onDidChangeState.fire({ outputIsHoveredChanged: true }); } + + private _focusOnOutput: boolean = false; + public get outputIsFocused(): boolean { + return this._focusOnOutput; + } + + public set outputIsFocused(v: boolean) { + this._focusOnOutput = v; + this._onDidChangeState.fire({ outputIsFocusedChanged: true }); + } + private _outputViewModels: ICellOutputViewModel[]; get outputsViewModels() { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 43ca7cb09a2..d2f359fc961 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -129,6 +129,7 @@ export interface IGenericCellViewModel { uri: URI; metadata: NotebookCellMetadata | undefined; outputIsHovered: boolean; + outputIsFocused: boolean; outputsViewModels: ICellOutputViewModel[]; getOutputOffset(index: number): number; updateOutputHeight(index: number, height: number): void; @@ -811,6 +812,7 @@ export interface CellViewModelStateChangeEvent { readonly foldingStateChanged?: boolean; readonly contentChanged?: boolean; readonly outputIsHoveredChanged?: boolean; + readonly outputIsFocusedChanged?: boolean; readonly cellIsHoveredChanged?: boolean; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index a1613cc48a6..c6f000bcf45 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -52,6 +52,16 @@ export interface IMouseLeaveMessage extends BaseToWebviewMessage { id: string; } +export interface IOutputFocusMessage extends BaseToWebviewMessage { + type: 'outputFocus'; + id: string; +} + +export interface IOutputBlurMessage extends BaseToWebviewMessage { + type: 'outputBlur'; + id: string; +} + export interface IWheelMessage extends BaseToWebviewMessage { type: 'did-scroll-wheel'; payload: any; @@ -292,6 +302,8 @@ export type FromWebviewMessage = | IDimensionMessage | IMouseEnterMessage | IMouseLeaveMessage + | IOutputFocusMessage + | IOutputBlurMessage | IWheelMessage | IScrollAckMessage | IBlurOutputMessage @@ -843,6 +855,28 @@ var requirejs = (function() { } break; } + case 'outputFocus': + { + const resolvedResult = this.resolveOutputId(data.id); + if (resolvedResult) { + const latestCell = this.notebookEditor.getCellByInfo(resolvedResult.cellInfo); + if (latestCell) { + latestCell.outputIsFocused = true; + } + } + break; + } + case 'outputBlur': + { + const resolvedResult = this.resolveOutputId(data.id); + if (resolvedResult) { + const latestCell = this.notebookEditor.getCellByInfo(resolvedResult.cellInfo); + if (latestCell) { + latestCell.outputIsFocused = false; + } + } + break; + } case 'scroll-ack': { // const date = new Date(); diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts index b71911dd14e..7c0dff0eebd 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts @@ -953,6 +953,10 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende templateData.container.classList.toggle('cell-output-hover', element.outputIsHovered); } + private updateForFocus(element: CodeCellViewModel, templateData: CodeCellRenderTemplate): void { + templateData.container.classList.toggle('cell-output-focus', element.outputIsFocused); + } + private updateForLayout(element: CodeCellViewModel, templateData: CodeCellRenderTemplate): void { templateData.focusIndicatorLeft.style.height = `${element.layoutInfo.indicatorHeight}px`; templateData.focusIndicatorRight.style.height = `${element.layoutInfo.indicatorHeight}px`; @@ -1028,6 +1032,7 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende templateData.cellRunState.clear(); this.updateForMetadata(element, templateData, cellEditorOptions); this.updateForHover(element, templateData); + this.updateForFocus(element, templateData); elementDisposables.add(element.onDidChangeState((e) => { if (e.metadataChanged) { this.updateForMetadata(element, templateData, cellEditorOptions); @@ -1036,6 +1041,10 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende if (e.outputIsHoveredChanged) { this.updateForHover(element, templateData); } + + if (e.outputIsFocusedChanged) { + this.updateForFocus(element, templateData); + } })); elementDisposables.add(this.notebookEditor.viewModel.notebookDocument.onDidChangeContent(e => { if (e.rawEvents.find(event => event.kind === NotebookCellsChangeType.ChangeDocumentMetadata)) { diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index a55e23e2f52..23c09246486 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -6,7 +6,7 @@ import type { Event } from 'vs/base/common/event'; import type { IDisposable } from 'vs/base/common/lifecycle'; import { RenderOutputType } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { FromWebviewMessage, IBlurOutputMessage, ICellDropMessage, ICellDragMessage, ICellDragStartMessage, IClickedDataUrlMessage, ICustomRendererMessage, IDimensionMessage, IClickMarkdownPreviewMessage, IMouseEnterMarkdownPreviewMessage, IMouseEnterMessage, IMouseLeaveMarkdownPreviewMessage, IMouseLeaveMessage, IToggleMarkdownPreviewMessage, IWheelMessage, ToWebviewMessage, ICellDragEndMessage } from 'vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView'; +import { FromWebviewMessage, IBlurOutputMessage, ICellDropMessage, ICellDragMessage, ICellDragStartMessage, IClickedDataUrlMessage, ICustomRendererMessage, IDimensionMessage, IClickMarkdownPreviewMessage, IMouseEnterMarkdownPreviewMessage, IMouseEnterMessage, IMouseLeaveMarkdownPreviewMessage, IMouseLeaveMessage, IToggleMarkdownPreviewMessage, IWheelMessage, ToWebviewMessage, ICellDragEndMessage, IOutputFocusMessage, IOutputBlurMessage } from 'vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView'; // !! IMPORTANT !! everything must be in-line within the webviewPreloads // function. Imports are not allowed. This is stringifies and injected into @@ -259,6 +259,75 @@ function webviewPreloads() { }); } + function isAncestor(testChild: Node | null, testAncestor: Node | null): boolean { + while (testChild) { + if (testChild === testAncestor) { + return true; + } + testChild = testChild.parentNode; + } + + return false; + } + + class FocusTracker { + private _outputId: string; + private _hasFocus: boolean = false; + private _loosingFocus: boolean = false; + private _element: HTMLElement | Window; + constructor(element: HTMLElement | Window, outputId: string) { + this._element = element; + this._outputId = outputId; + this._hasFocus = isAncestor(document.activeElement, element); + this._loosingFocus = false; + + element.addEventListener('focus', this._onFocus.bind(this), true); + element.addEventListener('blur', this._onBlur.bind(this), true); + } + + private _onFocus() { + this._loosingFocus = false; + if (!this._hasFocus) { + this._hasFocus = true; + postNotebookMessage('outputFocus', { + id: this._outputId, + }); + } + } + + private _onBlur() { + if (this._hasFocus) { + this._loosingFocus = true; + window.setTimeout(() => { + if (this._loosingFocus) { + this._loosingFocus = false; + this._hasFocus = false; + postNotebookMessage('outputBlur', { + id: this._outputId, + }); + } + }, 0); + } + } + + dispose() { + if (this._element) { + this._element.removeEventListener('focus', this._onFocus, true); + this._element.removeEventListener('blur', this._onBlur, true); + } + } + } + + const focusTrackers = new Map(); + + function addFocusTracker(element: HTMLElement, outputId: string): void { + if (focusTrackers.has(outputId)) { + focusTrackers.get(outputId)?.dispose(); + } + + focusTrackers.set(outputId, new FocusTracker(element, outputId)); + } + const dontEmit = Symbol('dontEmit'); function createEmitter(listenerChange: (listeners: Set>) => void = () => undefined): EmitterLike { @@ -513,6 +582,7 @@ function webviewPreloads() { outputNode.id = outputId; addMouseoverListeners(outputNode, outputId); + addFocusTracker(outputNode, outputId); const content = data.content; if (content.type === RenderOutputType.Html) { const trustedHtml = ttPolicy?.createHTML(content.htmlContent) ?? content.htmlContent; @@ -617,6 +687,10 @@ function webviewPreloads() { ob.disconnect(); }); outputObservers.clear(); + focusTrackers.forEach(ft => { + ft.dispose(); + }); + focusTrackers.clear(); break; case 'clearOutput': const output = document.getElementById(event.data.outputId); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts index fafb2684b18..c7ef3ec8526 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts @@ -50,6 +50,16 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod this._onDidChangeState.fire({ outputIsHoveredChanged: true }); } + private _focusOnOutput: boolean = false; + public get outputIsFocused(): boolean { + return this._focusOnOutput; + } + + public set outputIsFocused(v: boolean) { + this._focusOnOutput = v; + this._onDidChangeState.fire({ outputIsFocusedChanged: true }); + } + private _outputMinHeight: number = 0; get outputMinHeight() { diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts index 4e5ef4c99df..8ddbdb57a54 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel.ts @@ -73,6 +73,15 @@ export class MarkdownCellViewModel extends BaseCellViewModel implements ICellVie this._hoveringOutput = v; } + private _focusOnOutput: boolean = false; + public get outputIsFocused(): boolean { + return this._focusOnOutput; + } + + public set outputIsFocused(v: boolean) { + this._focusOnOutput = v; + } + private _hoveringCell = false; public get cellIsHovered(): boolean { return this._hoveringCell; From 2a1f20afc7c2a52a74806c88c21fc4e333c36363 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 29 Mar 2021 17:24:12 -0700 Subject: [PATCH 02/25] fix #115432. --- .../notebook/browser/notebookEditorWidget.ts | 15 ++----- .../browser/viewModel/notebookViewModel.ts | 13 ++++++ .../notebook/test/notebookViewModel.test.ts | 40 +++++++++++++++++++ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 1117b83ee55..aaba6423f68 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -1772,21 +1772,12 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return new Promise(resolve => { r = resolve; }); } - private _nearestCodeCellIndex(index: number /* exclusive */, direction: 'above' | 'below') { + private _nearestCodeCellIndex(index: number /* exclusive */) { if (!this.viewModel) { return -1; } - const nearest = this.viewModel.viewCells.slice(0, index).reverse().findIndex(cell => cell.cellKind === CellKind.Code); - if (nearest > -1) { - return index - nearest - 1; - } else { - const nearestCellTheOtherDirection = this.viewModel.viewCells.slice(index + 1).findIndex(cell => cell.cellKind === CellKind.Code); - if (nearestCellTheOtherDirection > -1) { - return index + nearestCellTheOtherDirection; - } - return -1; - } + return this.viewModel.nearestCodeCellIndex(index); } insertNotebookCell(cell: ICellViewModel | undefined, type: CellKind, direction: 'above' | 'below' = 'above', initialText: string = '', ui: boolean = false): CellViewModel | null { @@ -1807,7 +1798,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor if (cell?.cellKind === CellKind.Code) { language = cell.language; } else if (cell?.cellKind === CellKind.Markdown) { - const nearestCodeCellIndex = this._nearestCodeCellIndex(index, direction); + const nearestCodeCellIndex = this._nearestCodeCellIndex(index); if (nearestCodeCellIndex > -1) { language = this.viewModel.viewCells[nearestCodeCellIndex].language; } else { diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index 96ed9e159bc..01a4327ea2d 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -697,6 +697,19 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD return result; } + nearestCodeCellIndex(index: number /* exclusive */) { + const nearest = this.viewCells.slice(0, index).reverse().findIndex(cell => cell.cellKind === CellKind.Code); + if (nearest > -1) { + return index - nearest - 1; + } else { + const nearestCellTheOtherDirection = this.viewCells.slice(index + 1).findIndex(cell => cell.cellKind === CellKind.Code); + if (nearestCellTheOtherDirection > -1) { + return index + 1 + nearestCellTheOtherDirection; + } + return -1; + } + } + createCell(index: number, source: string, language: string, type: CellKind, metadata: NotebookCellMetadata | undefined, outputs: IOutputDto[], synchronous: boolean, pushUndoStop: boolean = true, previouslyPrimary: number | null = null, previouslyFocused: ICellViewModel[] = []): CellViewModel { const beforeSelections = previouslyFocused.map(e => e.handle); const endSelections: ISelectionState = { kind: SelectionStateType.Index, focus: { start: index, end: index + 1 }, selections: [{ start: index, end: index + 1 }] }; diff --git a/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts index 17bee916c0e..f1b89403602 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts @@ -395,3 +395,43 @@ suite('NotebookViewModel Decorations', () => { }); }); + +suite('NotebookViewModel API', () => { + test('#115432, get nearest code cell', async function () { + await withTestNotebook( + [ + ['# header a', 'markdown', CellKind.Markdown, [], {}], + ['var b = 1;', 'javascript', CellKind.Code, [], {}], + ['# header b', 'markdown', CellKind.Markdown, [], {}], + ['b = 2;', 'python', CellKind.Code, [], {}], + ['var c = 3', 'javascript', CellKind.Code, [], {}], + ['# header d', 'markdown', CellKind.Markdown, [], {}], + ['var e = 4;', 'TypeScript', CellKind.Code, [], {}], + ['# header f', 'markdown', CellKind.Markdown, [], {}] + ], + (editor) => { + const viewModel = editor.viewModel; + assert.strictEqual(viewModel.nearestCodeCellIndex(0), 1); + // find the nearest code cell from above + assert.strictEqual(viewModel.nearestCodeCellIndex(2), 1); + assert.strictEqual(viewModel.nearestCodeCellIndex(4), 3); + assert.strictEqual(viewModel.nearestCodeCellIndex(5), 4); + assert.strictEqual(viewModel.nearestCodeCellIndex(6), 4); + } + ); + }); + + test('#108464, get nearest code cell', async function () { + await withTestNotebook( + [ + ['# header a', 'markdown', CellKind.Markdown, [], {}], + ['var b = 1;', 'javascript', CellKind.Code, [], {}], + ['# header b', 'markdown', CellKind.Markdown, [], {}] + ], + (editor) => { + const viewModel = editor.viewModel; + assert.strictEqual(viewModel.nearestCodeCellIndex(2), 1); + } + ); + }); +}); From f908389c0b15066caccc1572c041e9dbcb37fd7f Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 29 Mar 2021 20:11:22 -0700 Subject: [PATCH 03/25] fix #117670. --- .../notebook/browser/notebookEditorWidget.ts | 3 ++ .../browser/view/renderers/webviewPreloads.ts | 33 ++++++++++++++----- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index aaba6423f68..053dd8e23cb 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -2240,6 +2240,9 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor const cell = this.viewModel?.viewCells.find(vc => vc.handle === cellInfo.cellHandle); if (cell && cell instanceof CodeCellViewModel) { const outputIndex = cell.outputsViewModels.indexOf(output); + if (isInit && outputHeight !== 0) { + cell.outputMinHeight = 0; + } cell.updateOutputHeight(outputIndex, outputHeight); this.layoutNotebookCell(cell, cell.layoutInfo.totalHeight); } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index 23c09246486..1edfc76e59c 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -629,14 +629,31 @@ function webviewPreloads() { resizeObserve(outputNode, outputId, true); - postNotebookMessage('dimension', { - id: outputId, - isOutput: true, - init: true, - data: { - height: outputNode.clientHeight - } - }); + const clientHeight = outputNode.clientHeight; + const cps = document.defaultView!.getComputedStyle(outputNode); + if (clientHeight !== 0 && cps.padding === '0px') { + // we set padding to zero if the output height is zero (then we can have a zero-height output DOM node) + // thus we need to ensure the padding is accounted when updating the init height of the output + postNotebookMessage('dimension', { + id: outputId, + isOutput: true, + init: true, + data: { + height: clientHeight + __outputNodePadding__ * 2 + } + }); + + outputNode.style.padding = `${__outputNodePadding__}px ${__outputNodePadding__}px ${__outputNodePadding__}px ${output ? __outputNodeLeftPadding__ : __leftMargin__}px`; + } else { + postNotebookMessage('dimension', { + id: outputId, + isOutput: true, + init: true, + data: { + height: outputNode.clientHeight + } + }); + } // don't hide until after this step so that the height is right cellOutputContainer.style.display = data.initiallyHidden ? 'none' : 'block'; From 5cf75096a6042cc0adee4ea176c5d2b3788fd513 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 30 Mar 2021 11:22:12 +0200 Subject: [PATCH 04/25] :lipstick: --- .../contrib/notebook/common/notebookEditorInput.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorInput.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorInput.ts index ad3ad32ade1..fd316656cd9 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookEditorInput.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookEditorInput.ts @@ -122,11 +122,7 @@ export class NotebookEditorInput extends EditorInput { return `${pattern.include} (exclude: ${pattern.exclude})`; }).join(', '); - throw new Error(`File name ${target} is not supported by ${provider.providerDisplayName}. - -Please make sure the file name matches following patterns: -${patterns} -`); + throw new Error(`File name ${target} is not supported by ${provider.providerDisplayName}.\n\nPlease make sure the file name matches following patterns:\n${patterns}`); } return await this._editorModelReference.object.saveAs(target); @@ -191,6 +187,4 @@ ${patterns} } return false; } - - } From 29a1cfddcd2207d2bd581b5101d82921d172dcce Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 30 Mar 2021 11:50:20 +0200 Subject: [PATCH 05/25] make sure dirty state of notebooks from simple content provider is reflected in extension host, add integration test --- extensions/vscode-api-tests/package.json | 9 ++++ .../notebook.document.test.ts | 52 +++++++++++++++++-- .../api/browser/mainThreadNotebook.ts | 27 +++------- .../notebookEditorModelResolverService.ts | 3 +- .../notebookEditorModelResolverServiceImpl.ts | 23 +++++++- 5 files changed, 87 insertions(+), 27 deletions(-) diff --git a/extensions/vscode-api-tests/package.json b/extensions/vscode-api-tests/package.json index 0abdff1acbe..bce3f10fa67 100644 --- a/extensions/vscode-api-tests/package.json +++ b/extensions/vscode-api-tests/package.json @@ -135,6 +135,15 @@ "filenamePattern": "**/*.nbdtest" } ] + }, + { + "viewType": "notebook.nbdserializer", + "displayName": "notebook.nbdserializer", + "selector": [ + { + "filenamePattern": "**/*.nbdserializer" + } + ] } ] }, diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts index b611a83953e..79697de48d8 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts @@ -9,7 +9,19 @@ import * as utils from '../utils'; suite('Notebook Document', function () { - const contentProvider = new class implements vscode.NotebookContentProvider { + const simpleContentProvider = new class implements vscode.NotebookSerializer { + dataToNotebook(_data: Uint8Array): vscode.NotebookData | Thenable { + return new vscode.NotebookData( + [new vscode.NotebookCellData(vscode.NotebookCellKind.Code, '// SIMPLE', 'javascript')], + new vscode.NotebookDocumentMetadata() + ); + } + notebookToData(_data: vscode.NotebookData): Uint8Array | Thenable { + return new Uint8Array(); + } + }; + + const complexContentProvider = new class implements vscode.NotebookContentProvider { async openNotebook(uri: vscode.Uri, _openContext: vscode.NotebookDocumentOpenContext): Promise { return new vscode.NotebookData( [new vscode.NotebookCellData(vscode.NotebookCellKind.Code, uri.toString(), 'javascript')], @@ -42,13 +54,17 @@ suite('Notebook Document', function () { }); suiteSetup(function () { - disposables.push(vscode.notebook.registerNotebookContentProvider('notebook.nbdtest', contentProvider)); + disposables.push(vscode.notebook.registerNotebookContentProvider('notebook.nbdtest', complexContentProvider)); + disposables.push(vscode.notebook.registerNotebookSerializer('notebook.nbdserializer', simpleContentProvider)); }); test('cannot register sample provider multiple times', function () { assert.throws(() => { - vscode.notebook.registerNotebookContentProvider('notebook.nbdtest', contentProvider); + vscode.notebook.registerNotebookContentProvider('notebook.nbdtest', complexContentProvider); }); + // assert.throws(() => { + // vscode.notebook.registerNotebookSerializer('notebook.nbdserializer', simpleContentProvider); + // }); }); test('cannot open unknown types', async function () { @@ -385,4 +401,34 @@ suite('Notebook Document', function () { assert.deepStrictEqual(document.cells[0].outputs[1].metadata, { outputType: 'stream', streamName: 'stderr' }); assert.deepStrictEqual(document.cells[0].outputs[1].outputs[0].metadata, { outputType: 'stream', streamName: 'stderr' }); }); + + test('dirty state - complex', async function () { + const resource = await utils.createRandomFile(undefined, undefined, '.nbdtest'); + const document = await vscode.notebook.openNotebookDocument(resource); + assert.strictEqual(document.isDirty, false); + + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookCells(document.uri, 0, document.getCells().length, []); + assert.ok(await vscode.workspace.applyEdit(edit)); + + assert.strictEqual(document.isDirty, true); + + await document.save(); + assert.strictEqual(document.isDirty, false); + }); + + test('dirty state - serializer', async function () { + const resource = await utils.createRandomFile(undefined, undefined, '.nbdserializer'); + const document = await vscode.notebook.openNotebookDocument(resource); + assert.strictEqual(document.isDirty, false); + + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookCells(document.uri, 0, document.getCells().length, []); + assert.ok(await vscode.workspace.applyEdit(edit)); + + assert.strictEqual(document.isDirty, true); + + await document.save(); + assert.strictEqual(document.isDirty, false); + }); }); diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index ef913f0cf42..8b994acf80e 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -10,8 +10,6 @@ import { Emitter } from 'vs/base/common/event'; import { IRelativePattern } from 'vs/base/common/glob'; import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; import { ResourceMap } from 'vs/base/common/map'; -import { Schemas } from 'vs/base/common/network'; -import { isEqual } from 'vs/base/common/resources'; import { URI, UriComponents } from 'vs/base/common/uri'; import { EditorActivation, EditorOverride } from 'vs/platform/editor/common/editor'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -29,7 +27,6 @@ import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebo import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity'; -import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookCellStatusBarEntryDto, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, INotebookModelAddedData, MainContext, MainThreadNotebookShape, NotebookEditorRevealType, NotebookExtensionDescription } from '../common/extHost.protocol'; class NotebookAndEditorState { @@ -119,7 +116,6 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { constructor( extHostContext: IExtHostContext, @IInstantiationService private readonly _instantiationService: IInstantiationService, - @IWorkingCopyService private readonly _workingCopyService: IWorkingCopyService, @INotebookService private readonly _notebookService: INotebookService, @INotebookEditorService private readonly _notebookEditorService: INotebookEditorService, @IEditorService private readonly _editorService: IEditorService, @@ -176,21 +172,13 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { private _registerListeners(): void { - // forward changes to dirty state - // todo@rebornix todo@mjbvz this seem way too complicated... is there an easy way to - // the actual resource from a working copy? - this._disposables.add(this._workingCopyService.onDidChangeDirty(e => { - if (e.resource.scheme !== Schemas.vscodeNotebook) { - return; - } - for (const notebook of this._notebookService.getNotebookTextModels()) { - if (isEqual(notebook.uri.with({ scheme: Schemas.vscodeNotebook }), e.resource)) { - this._proxy.$acceptDirtyStateChanged(notebook.uri, e.isDirty()); - break; - } - } - })); + this._notebookEditorModelResolverService.onDidChangeDirty(model => { + this._proxy.$acceptDirtyStateChanged(model.resource, model.isDirty); + }); + this._disposables.add(this._notebookEditorModelResolverService.onDidSaveNotebook(e => { + this._proxy.$acceptModelSaved(e); + })); this._disposables.add(this._editorService.onDidActiveEditorChange(e => { this._updateState(); @@ -334,9 +322,6 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { this._proxy.$acceptNotebookActiveKernelChange(e); })); - this._disposables.add(this._notebookEditorModelResolverService.onDidSaveNotebook(e => { - this._proxy.$acceptModelSaved(e); - })); const notebookEditor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane); this._updateState(notebookEditor); diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts index 8d433169c1d..52c13ae3031 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts @@ -14,7 +14,8 @@ export const INotebookEditorModelResolverService = createDecorator; + readonly onDidSaveNotebook: Event; + readonly onDidChangeDirty: Event<{ resource: URI, isDirty: boolean }>; resolve(resource: URI, viewType?: string): Promise>; } diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts index 71a006f6a81..1daf24a0f39 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts @@ -7,7 +7,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { URI } from 'vs/base/common/uri'; import { CellUri, IResolvedNotebookEditorModel } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ComplexNotebookEditorModel, NotebookFileWorkingCopyModel, NotebookFileWorkingCopyModelFactory, SimpleNotebookEditorModel } from 'vs/workbench/contrib/notebook/common/notebookEditorModel'; -import { combinedDisposable, DisposableStore, IDisposable, IReference, ReferenceCollection } from 'vs/base/common/lifecycle'; +import { combinedDisposable, DisposableStore, dispose, IDisposable, IReference, ReferenceCollection } from 'vs/base/common/lifecycle'; import { ComplexNotebookProviderInfo, INotebookService, SimpleNotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookService'; import { ILogService } from 'vs/platform/log/common/log'; import { Emitter, Event } from 'vs/base/common/event'; @@ -25,6 +25,9 @@ class NotebookModelReferenceCollection extends ReferenceCollection(); readonly onDidSaveNotebook: Event = this._onDidSaveNotebook.event; + private readonly _onDidChangeDirty = new Emitter<{ resource: URI, isDirty: boolean }>(); + readonly onDidChangeDirty: Event<{ resource: URI, isDirty: boolean }> = this._onDidChangeDirty.event; + constructor( @IInstantiationService readonly _instantiationService: IInstantiationService, @INotebookService private readonly _notebookService: INotebookService, @@ -38,6 +41,13 @@ class NotebookModelReferenceCollection extends ReferenceCollection { const uri = URI.parse(key); const info = await this._notebookService.withNotebookDataProvider(uri, viewType); @@ -56,7 +66,10 @@ class NotebookModelReferenceCollection extends ReferenceCollection this._onDidSaveNotebook.fire(result.resource))); + this._modelListener.set(result, combinedDisposable( + result.onDidSave(() => this._onDidSaveNotebook.fire(result.resource)), + result.onDidChangeDirty(() => this._onDidChangeDirty.fire({ resource: result.resource, isDirty: result.isDirty() })), + )); return result; } @@ -78,6 +91,7 @@ export class NotebookModelResolverServiceImpl implements INotebookEditorModelRes private readonly _data: NotebookModelReferenceCollection; readonly onDidSaveNotebook: Event; + readonly onDidChangeDirty: Event<{ resource: URI, isDirty: boolean }>; constructor( @IInstantiationService instantiationService: IInstantiationService, @@ -87,6 +101,11 @@ export class NotebookModelResolverServiceImpl implements INotebookEditorModelRes ) { this._data = instantiationService.createInstance(NotebookModelReferenceCollection); this.onDidSaveNotebook = this._data.onDidSaveNotebook; + this.onDidChangeDirty = this._data.onDidChangeDirty; + } + + dispose() { + this._data.dispose(); } async resolve(resource: URI, viewType?: string): Promise> { From 1f43f5ffcff077926fa28cbc2999754e3d419af6 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 30 Mar 2021 12:01:03 +0200 Subject: [PATCH 06/25] use INotebookEditorModelResolverService#onDidChangeDirty to drive notebook file tracker --- .../notebook/browser/notebook.contribution.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index fcb1d035d04..5f445e96680 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -50,7 +50,6 @@ import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/no import { NotebookEditorWidgetService } from 'vs/workbench/contrib/notebook/browser/notebookEditorServiceImpl'; import { IJSONContributionRegistry, Extensions as JSONExtensions } from 'vs/platform/jsonschemas/common/jsonContributionRegistry'; import { IJSONSchema } from 'vs/base/common/jsonSchema'; -import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { Event } from 'vs/base/common/event'; import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility'; import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; @@ -694,32 +693,35 @@ class NotebookFileTracker implements IWorkbenchContribution { private readonly _dirtyListener: IDisposable; constructor( - @INotebookService private readonly _notebookService: INotebookService, @IEditorService private readonly _editorService: IEditorService, - @IWorkingCopyService private readonly _workingCopyService: IWorkingCopyService, + @INotebookEditorModelResolverService private readonly _notebookEditorModelService: INotebookEditorModelResolverService, ) { - this._dirtyListener = Event.debounce(_workingCopyService.onDidChangeDirty, () => { }, 100)(() => { - const inputs = this._createMissingNotebookEditors(); - this._editorService.openEditors(inputs); - }); + + type E = { resource: URI, isDirty: boolean }; + this._dirtyListener = Event.debounce( + this._notebookEditorModelService.onDidChangeDirty, + (last, current) => !last ? [current] : [...last, current], + 100 + )(this._openMissingDirtyNotebookEditors, this); } dispose(): void { this._dirtyListener.dispose(); } - private _createMissingNotebookEditors(): IResourceEditorInput[] { + private _openMissingDirtyNotebookEditors(inputs: { resource: URI, isDirty: boolean }[]): void { const result: IResourceEditorInput[] = []; - - for (const notebook of this._notebookService.getNotebookTextModels()) { - if (this._workingCopyService.isDirty(notebook.uri.with({ scheme: Schemas.vscodeNotebook })) && !this._editorService.isOpen({ resource: notebook.uri })) { + for (let input of inputs) { + if (input.isDirty && !this._editorService.isOpen({ resource: input.resource })) { result.push({ - resource: notebook.uri, + resource: input.resource, options: { inactive: true, preserveFocus: true, pinned: true } }); } } - return result; + if (result.length > 0) { + this._editorService.openEditors(result); + } } } From 26dba7aab5ac546f574444af999e9cd30e3a2ec0 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 30 Mar 2021 13:58:30 +0200 Subject: [PATCH 07/25] better working copy uri for complex notebooks, https://github.com/microsoft/vscode/issues/117899 --- .../contrib/notebook/browser/notebook.contribution.ts | 2 +- .../workbench/contrib/notebook/common/notebookEditorModel.ts | 2 +- .../contrib/notebook/test/notebookEditorModel.test.ts | 4 +--- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index 5f445e96680..1075053e588 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -191,7 +191,7 @@ Registry.as(EditorInputExtensions.EditorInputFactor canResolveBackup(editorInput: IEditorInput, backupResource: URI): boolean { if (editorInput instanceof NotebookEditorInput) { - if (isEqual(editorInput.resource.with({ scheme: Schemas.vscodeNotebook }), backupResource)) { + if (isEqual(URI.from({ scheme: Schemas.vscodeNotebook, path: editorInput.resource.toString() }), backupResource)) { return true; } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts index 04313a3c8a8..c1639d1a2fd 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts @@ -67,7 +67,7 @@ export class ComplexNotebookEditorModel extends EditorModel implements INotebook this._name = labelService.getUriBasenameLabel(resource); const that = this; - this._workingCopyResource = resource.with({ scheme: Schemas.vscodeNotebook }); + this._workingCopyResource = URI.from({ scheme: Schemas.vscodeNotebook, path: resource.toString() }); const workingCopyAdapter = new class implements IWorkingCopy { readonly resource = that._workingCopyResource; get name() { return that._name; } diff --git a/src/vs/workbench/contrib/notebook/test/notebookEditorModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookEditorModel.test.ts index ef688287846..8d4567ed075 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookEditorModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookEditorModel.test.ts @@ -37,9 +37,7 @@ suite('NotebookEditorModel', function () { const notebookDataProvider = new class extends mock() { }; test('working copy uri', function () { - if (1) { - this.skip(); - } + const r1 = URI.parse('foo-files:///my.nb'); const r2 = URI.parse('bar-files:///my.nb'); From 4bb08160c7037f0cc286f246e26bcc22780af77e Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 29 Mar 2021 22:57:43 -0700 Subject: [PATCH 08/25] fix #117670. --- .../contrib/notebook/browser/view/renderers/webviewPreloads.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index c99de3bf539..df3287d946b 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -643,7 +643,7 @@ function webviewPreloads() { } }); - outputNode.style.padding = `${__outputNodePadding__}px ${__outputNodePadding__}px ${__outputNodePadding__}px ${output ? __outputNodeLeftPadding__ : __leftMargin__}px`; + outputNode.style.padding = `${__outputNodePadding__}px ${__outputNodePadding__}px ${__outputNodePadding__}px ${__outputNodeLeftPadding__}px`; } else { postNotebookMessage('dimension', { id: outputId, From 88c58b011c06233c8fb1a61a79a8fffd22ba7ee2 Mon Sep 17 00:00:00 2001 From: rebornix Date: Tue, 30 Mar 2021 08:11:19 -0700 Subject: [PATCH 09/25] re #117623. --- .../notebook/browser/contrib/coreActions.ts | 49 -------- .../browser/contrib/troubleshoot/layout.ts | 109 ++++++++++++++++++ .../notebook/browser/notebook.contribution.ts | 1 + .../notebook/browser/notebookBrowser.ts | 13 ++- .../notebook/browser/notebookEditorWidget.ts | 22 +++- .../view/renderers/backLayerWebView.ts | 2 +- .../browser/view/renderers/cellOutput.ts | 24 ++-- .../browser/viewModel/codeCellViewModel.ts | 26 +++-- 8 files changed, 170 insertions(+), 76 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/browser/contrib/troubleshoot/layout.ts diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index db9cec62661..03ffa9d0644 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -18,9 +18,7 @@ import { InputFocusedContext, InputFocusedContextKey } from 'vs/platform/context import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { IQuickInputService, IQuickPickItem, QuickPickInput } from 'vs/platform/quickinput/common/quickInput'; -import { CATEGORIES } from 'vs/workbench/common/actions'; import { BaseCellRenderTemplate, CellEditState, CellFocusMode, EXECUTE_CELL_COMMAND_ID, EXPAND_CELL_INPUT_COMMAND_ID, getNotebookEditorFromEditorPane, IActiveNotebookEditor, ICellViewModel, NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_INPUT_COLLAPSED, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_OUTPUT_COLLAPSED, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_TYPE, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_OUTPUT_FOCUSED, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_HAS_RUNNING_CELL } 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, NotebookCellExecutionState, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, TransientMetadata, SelectionStateType } 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'; @@ -1684,53 +1682,6 @@ registerAction2(class extends ChangeNotebookCellMetadataAction { } }); -registerAction2(class extends Action2 { - constructor() { - super({ - id: 'notebook.inspectLayout', - title: localize('notebookActions.inspectLayout', "Inspect Notebook Layout"), - category: CATEGORIES.Developer, - f1: true - }); - } - - protected getActiveEditorContext(accessor: ServicesAccessor): INotebookActionContext | undefined { - const editorService = accessor.get(IEditorService); - - const editor = getNotebookEditorFromEditorPane(editorService.activeEditorPane); - if (!editor) { - return; - } - - if (!editor.hasModel()) { - return; - } - - const activeCell = editor.getActiveCell(); - return { - cell: activeCell, - notebookEditor: editor - }; - } - - run(accessor: ServicesAccessor) { - const activeEditorContext = this.getActiveEditorContext(accessor); - - if (activeEditorContext) { - const viewModel = activeEditorContext.notebookEditor.viewModel; - console.log('--- notebook ---'); - console.log(viewModel.layoutInfo); - console.log('--- cells ---'); - for (let i = 0; i < viewModel.length; i++) { - const cell = viewModel.viewCells[i] as CellViewModel; - console.log(`--- cell: ${cell.handle} ---`); - console.log(cell.layoutInfo); - } - - } - } -}); - // Revisit once we have a story for trusted workspace CommandsRegistry.registerCommand('notebook.trust', (accessor, args) => { const uri = URI.revive(args as UriComponents); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/troubleshoot/layout.ts b/src/vs/workbench/contrib/notebook/browser/contrib/troubleshoot/layout.ts new file mode 100644 index 00000000000..42713f4c77b --- /dev/null +++ b/src/vs/workbench/contrib/notebook/browser/contrib/troubleshoot/layout.ts @@ -0,0 +1,109 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable, DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; +import { Action2, registerAction2 } from 'vs/platform/actions/common/actions'; +import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; +import { CATEGORIES } from 'vs/workbench/common/actions'; +import { getNotebookEditorFromEditorPane, ICellViewModel, INotebookEditor, INotebookEditorContribution } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { registerNotebookContribution } from 'vs/workbench/contrib/notebook/browser/notebookEditorExtensions'; +import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; + +export class TroubleshootController extends Disposable implements INotebookEditorContribution { + static id: string = 'workbench.notebook.troubleshoot'; + + private readonly _localStore = this._register(new DisposableStore()); + private _cellStateListeners: IDisposable[] = []; + private _logging: boolean = false; + + constructor(private readonly _notebookEditor: INotebookEditor) { + super(); + + this._register(this._notebookEditor.onDidChangeModel(() => { + this._localStore.clear(); + this._cellStateListeners.forEach(listener => listener.dispose()); + + if (!this._notebookEditor.viewModel) { + return; + } + + this._updateListener(); + })); + + this._updateListener(); + } + + toggleLogging(): void { + this._logging = !this._logging; + } + + private _log(cell: ICellViewModel, e: any) { + if (this._logging) { + const oldHeight = this._notebookEditor.getViewHeight(cell); + if (oldHeight !== cell.layoutInfo.totalHeight) { + console.log(`cell#${cell.handle}`, e, `${oldHeight} -> ${cell.layoutInfo.totalHeight}`); + } + } + } + + private _updateListener() { + if (!this._notebookEditor.viewModel) { + return; + } + + const viewModel = this._notebookEditor.viewModel; + + for (let i = 0; i < viewModel.viewCells.length; i++) { + const cell = viewModel.viewCells[i]; + + this._cellStateListeners.push(cell.onDidChangeLayout(e => { + this._log(cell, e); + })); + } + + this._localStore.add(viewModel.onDidChangeViewCells(e => { + e.splices.reverse().forEach(splice => { + const [start, deleted, newCells] = splice; + const deletedCells = this._cellStateListeners.splice(start, deleted, ...newCells.map(cell => { + return cell.onDidChangeLayout(e => { + this._log(cell, e); + }); + })); + + dispose(deletedCells); + }); + })); + } + + dispose() { + dispose(this._cellStateListeners); + super.dispose(); + } +} + +registerNotebookContribution(TroubleshootController.id, TroubleshootController); + +registerAction2(class extends Action2 { + constructor() { + super({ + id: 'notebook.toggleLayoutTroubleshoot', + title: 'Toggle Notebook Layout Troubleshoot', + category: CATEGORIES.Developer, + f1: true + }); + } + + async run(accessor: ServicesAccessor): Promise { + const editorService = accessor.get(IEditorService); + const editor = getNotebookEditorFromEditorPane(editorService.activeEditorPane); + + if (!editor) { + return; + } + + const controller = editor.getContribution(TroubleshootController.id); + controller?.toggleLogging(); + } +}); diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index 1075053e588..66b5fb6f59a 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -67,6 +67,7 @@ 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'; import 'vs/workbench/contrib/notebook/browser/contrib/viewportCustomMarkdown/viewportCustomMarkdown'; +import 'vs/workbench/contrib/notebook/browser/contrib/troubleshoot/layout'; // Diff Editor Contribution diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index d2f359fc961..fd31f1f9824 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -132,7 +132,7 @@ export interface IGenericCellViewModel { outputIsFocused: boolean; outputsViewModels: ICellOutputViewModel[]; getOutputOffset(index: number): number; - updateOutputHeight(index: number, height: number): void; + updateOutputHeight(index: number, height: number, source?: string): void; } export interface IDisplayOutputLayoutUpdateRequest { @@ -164,7 +164,7 @@ export interface ICommonNotebookEditor { getCellById(cellId: string): IGenericCellViewModel | undefined; focusNotebookCell(cell: IGenericCellViewModel, focus: 'editor' | 'container' | 'output', options?: IFocusNotebookCellOptions): void; focusNextNotebookCell(cell: IGenericCellViewModel, focus: 'editor' | 'container' | 'output'): void; - updateOutputHeight(cellInfo: ICommonCellInfo, output: IDisplayOutputViewModel, height: number, isInit: boolean): void; + updateOutputHeight(cellInfo: ICommonCellInfo, output: IDisplayOutputViewModel, height: number, isInit: boolean, source?: string): void; updateMarkdownCellHeight(cellId: string, height: number, isInit: boolean): void; setMarkdownCellEditState(cellId: string, editState: CellEditState): void; markdownCellDragStart(cellId: string, position: { clientY: number }): void; @@ -209,6 +209,7 @@ export interface CodeCellLayoutInfo { } export interface CodeCellLayoutChangeEvent { + source?: string; editorHeight?: boolean; outputHeight?: boolean; outputShowMoreContainerHeight?: number; @@ -236,6 +237,7 @@ export interface ICellViewModel extends IGenericCellViewModel { readonly id: string; readonly textBuffer: IReadonlyTextBuffer; readonly layoutInfo: { totalHeight: number; }; + readonly onDidChangeLayout: Event; dragging: boolean; handle: number; uri: URI; @@ -574,6 +576,11 @@ export interface INotebookEditor extends ICommonNotebookEditor { */ getViewIndex(cell: ICellViewModel): number; + /** + * Get the view height of a cell (from the list view) + */ + getViewHeight(cell: ICellViewModel): number; + /** * @param startIndex Inclusive * @param endIndex Exclusive @@ -625,7 +632,7 @@ export interface INotebookEditor extends ICommonNotebookEditor { getCellByInfo(cellInfo: ICommonCellInfo): ICellViewModel; getCellById(cellId: string): ICellViewModel | undefined; - updateOutputHeight(cellInfo: ICommonCellInfo, output: IDisplayOutputViewModel, height: number, isInit: boolean): void; + updateOutputHeight(cellInfo: ICommonCellInfo, output: IDisplayOutputViewModel, height: number, isInit: boolean, source?: string): void; } export interface INotebookCellList { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 294528996f7..3b282d03b10 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -136,6 +136,14 @@ export class ListViewInfoAccessor extends Disposable { return this.list.getViewIndex(cell) ?? -1; } + getViewHeight(cell: ICellViewModel): number { + if (!this.list.viewModel) { + return -1; + } + + return this.list.elementHeight(cell); + } + getCellRangeFromViewRange(startIndex: number, endIndex: number): ICellRange | undefined { if (!this.list.viewModel) { return undefined; @@ -1592,6 +1600,14 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return this._listViewInfoAccessor.getViewIndex(cell); } + getViewHeight(cell: ICellViewModel): number { + if (!this._listViewInfoAccessor) { + return -1; + } + + return this._listViewInfoAccessor.getViewHeight(cell); + } + getCellRangeFromViewRange(startIndex: number, endIndex: number): ICellRange | undefined { return this._listViewInfoAccessor.getCellRangeFromViewRange(startIndex, endIndex); } @@ -2237,14 +2253,14 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return this.viewModel?.viewCells.find(vc => vc.id === cellId); } - updateOutputHeight(cellInfo: ICommonCellInfo, output: ICellOutputViewModel, outputHeight: number, isInit: boolean): void { + updateOutputHeight(cellInfo: ICommonCellInfo, output: ICellOutputViewModel, outputHeight: number, isInit: boolean, source?: string): void { const cell = this.viewModel?.viewCells.find(vc => vc.handle === cellInfo.cellHandle); if (cell && cell instanceof CodeCellViewModel) { const outputIndex = cell.outputsViewModels.indexOf(output); if (isInit && outputHeight !== 0) { - cell.outputMinHeight = 0; + cell.updateOutputMinHeight(0); } - cell.updateOutputHeight(outputIndex, outputHeight); + cell.updateOutputHeight(outputIndex, outputHeight, source); this.layoutNotebookCell(cell, cell.layoutInfo.totalHeight); } } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index 29b71007a8f..b26ed3817bd 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -833,7 +833,7 @@ var requirejs = (function() { const resolvedResult = this.resolveOutputId(data.id); if (resolvedResult) { const { cellInfo, output } = resolvedResult; - this.notebookEditor.updateOutputHeight(cellInfo, output, outputHeight, !!data.init); + this.notebookEditor.updateOutputHeight(cellInfo, output, outputHeight, !!data.init, 'webview#dimension'); } } else { const cellId = data.id.substr(0, data.id.length - '_preview'.length); diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts index 04de0c6c9ca..bb2cd899c68 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts @@ -109,7 +109,7 @@ export class CellOutputElement extends Disposable { const [mimeTypes, pick] = this.output.resolveMimeTypes(notebookTextModel); if (!mimeTypes.find(mimeType => mimeType.isTrusted) || mimeTypes.length === 0) { - this.viewCell.updateOutputHeight(index, 0); + this.viewCell.updateOutputHeight(index, 0, 'CellOutputElement#noMimeType'); return undefined; } @@ -137,7 +137,7 @@ export class CellOutputElement extends Disposable { this.output.pickedMimeType = pick; if (!this.renderResult) { - this.viewCell.updateOutputHeight(index, 0); + this.viewCell.updateOutputHeight(index, 0, 'CellOutputElement#renderResultUndefined'); return undefined; } @@ -173,7 +173,7 @@ export class CellOutputElement extends Disposable { height: offsetHeight }; this.bindResizeListener(dimension); - this.viewCell.updateOutputHeight(index, offsetHeight); + this.viewCell.updateOutputHeight(index, offsetHeight, 'CellOutputElement#renderResultInitHeight'); const top = this.viewCell.getOutputOffsetInContainer(index); this.domNode.style.top = `${top}px`; return { initRenderIsSynchronous: true }; @@ -198,7 +198,7 @@ export class CellOutputElement extends Disposable { height: height }; - this.viewCell.updateOutputHeight(currIndex, height); + this.viewCell.updateOutputHeight(currIndex, height, 'CellOutputElement#outputResize'); this.relayoutCell(); } }); @@ -347,7 +347,7 @@ export class CellOutputContainer extends Disposable { render(editorHeight: number) { if (this.viewCell.outputsViewModels.length > 0) { if (this.viewCell.layoutInfo.totalHeight !== 0 && this.viewCell.layoutInfo.editorHeight > editorHeight) { - this.viewCell.outputMinHeight = this.viewCell.layoutInfo.outputTotalHeight; + this.viewCell.updateOutputMinHeight(this.viewCell.layoutInfo.outputTotalHeight); this._relayoutCell(); } @@ -394,7 +394,7 @@ export class CellOutputContainer extends Disposable { if (renderedOutput.renderResult.type !== RenderOutputType.Mainframe) { this.notebookEditor.createOutput(this.viewCell, renderedOutput.renderResult as IInsetRenderOutput, this.viewCell.getOutputOffset(index)); } else { - this.viewCell.updateOutputHeight(index, renderedOutput.domOffsetHeight); + this.viewCell.updateOutputHeight(index, renderedOutput.domOffsetHeight, 'CellOutputContainer#viewUpdateShowOutputs'); } } else { // Wasn't previously rendered, render it now @@ -434,12 +434,12 @@ export class CellOutputContainer extends Disposable { } if (synchronous) { - this.viewCell.outputMinHeight = 0; - this.viewCell.layoutChange({ outputHeight: true }); + this.viewCell.updateOutputMinHeight(0); + this.viewCell.layoutChange({ outputHeight: true }, 'CellOutputContainer#_validateFinalOutputHeight_sync'); } else { this._outputHeightTimer = setTimeout(() => { - this.viewCell.outputMinHeight = 0; - this.viewCell.layoutChange({ outputHeight: true }); + this.viewCell.updateOutputMinHeight(0); + this.viewCell.layoutChange({ outputHeight: true }, 'CellOutputContainer#_validateFinalOutputHeight_async_1000'); }, 1000); } } @@ -452,7 +452,7 @@ export class CellOutputContainer extends Disposable { const previousOutputHeight = this.viewCell.layoutInfo.outputTotalHeight; // for cell output update, we make sure the cell does not shrink before the new outputs are rendered. - this.viewCell.outputMinHeight = previousOutputHeight; + this.viewCell.updateOutputMinHeight(previousOutputHeight); if (this.viewCell.outputsViewModels.length) { DOM.show(this.templateData.outputContainer); @@ -570,7 +570,7 @@ export class CellOutputContainer extends Disposable { } dispose() { - this.viewCell.outputMinHeight = 0; + this.viewCell.updateOutputMinHeight(0); if (this._outputHeightTimer) { clearTimeout(this._outputHeightTimer); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts index c7ef3ec8526..0399a3b6393 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts @@ -33,7 +33,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod set editorHeight(height: number) { this._editorHeight = height; - this.layoutChange({ editorHeight: true }); + this.layoutChange({ editorHeight: true }, 'CodeCellViewModel#editorHeight'); } get editorHeight() { @@ -62,11 +62,15 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod private _outputMinHeight: number = 0; - get outputMinHeight() { + private get outputMinHeight() { return this._outputMinHeight; } - set outputMinHeight(newMin: number) { + /** + * The minimum height of the output region. It's only set to non-zero temporarily when replacing an output with a new one. + * It's reset to 0 when the new output is rendered, or in one second. + */ + private set outputMinHeight(newMin: number) { this._outputMinHeight = newMin; } @@ -124,7 +128,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod return outerWidth - (CODE_CELL_LEFT_MARGIN + (CELL_MARGIN * 2) + CELL_RUN_GUTTER); } - layoutChange(state: CodeCellLayoutChangeEvent) { + layoutChange(state: CodeCellLayoutChangeEvent, source?: string) { // recompute this._ensureOutputsTop(); const outputShowMoreContainerHeight = state.outputShowMoreContainerHeight ? state.outputShowMoreContainerHeight : this._layoutInfo.outputShowMoreContainerHeight; @@ -198,6 +202,8 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod state.totalHeight = true; } + state.source = source; + this._fireOnDidChangeLayout(state); } @@ -274,10 +280,14 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod } updateOutputShowMoreContainerHeight(height: number) { - this.layoutChange({ outputShowMoreContainerHeight: height }); + this.layoutChange({ outputShowMoreContainerHeight: height }, 'CodeCellViewModel#updateOutputShowMoreContainerHeight'); } - updateOutputHeight(index: number, height: number) { + updateOutputMinHeight(height: number) { + this.outputMinHeight = height; + } + + updateOutputHeight(index: number, height: number, source?: string) { if (index >= this._outputCollection.length) { throw new Error('Output index out of range!'); } @@ -285,7 +295,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod this._ensureOutputsTop(); this._outputCollection[index] = height; if (this._outputsTop!.changeValue(index, height)) { - this.layoutChange({ outputHeight: true }); + this.layoutChange({ outputHeight: true }, source); } } @@ -316,7 +326,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod this._outputsTop!.insertValues(start, values); } - this.layoutChange({ outputHeight: true }); + this.layoutChange({ outputHeight: true }, 'CodeCellViewModel#spliceOutputs'); } private _ensureOutputsTop(): void { From 33cc87e1c4e45e1d42e8308e5d0350b37f596124 Mon Sep 17 00:00:00 2001 From: rebornix Date: Tue, 30 Mar 2021 08:41:55 -0700 Subject: [PATCH 10/25] re #118108. separate selection and focus. --- .../contrib/notebook/browser/notebookEditorWidget.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 3b282d03b10..6c36ac2d323 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -963,9 +963,14 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor // todo@rebornix https://github.com/microsoft/vscode/issues/118108 support selections not just focus // todo@rebornix support multipe selections if (options?.cellSelections && this.viewModel) { - const focusedCell = this.viewModel.viewCells[options.cellSelections[0].start]; + const focusCellIndex = options.cellSelections[0].start; + const focusedCell = this.viewModel.viewCells[focusCellIndex]; + this.viewModel.updateSelectionsState({ + kind: SelectionStateType.Index, + focus: { start: focusCellIndex, end: focusCellIndex + 1 }, + selections: options.cellSelections + }); this.revealInCenterIfOutsideViewport(focusedCell); - this.focusElement(focusedCell); } } From 588c2f6c783091bb63819cdc11e1739fc3376b58 Mon Sep 17 00:00:00 2001 From: rebornix Date: Tue, 30 Mar 2021 11:32:00 -0700 Subject: [PATCH 11/25] skip showNotebookDocment. --- .../src/singlefolder-tests/notebook.editor.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts index 9211b98fc1d..afe8bffdbd1 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts @@ -47,7 +47,7 @@ suite('Notebook Editor', function () { }); - test('showNotebookDocment', async function () { + test.skip('showNotebookDocment', async function () { const count1 = vscode.notebook.notebookDocuments.length; From 4fd610fc717b1e45c84eb7f4d47c0e00868b02eb Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 08:45:17 +0200 Subject: [PATCH 12/25] try bigger timeout awaiting events --- .../src/singlefolder-tests/notebook.editor.test.ts | 2 +- extensions/vscode-api-tests/src/utils.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts index afe8bffdbd1..9211b98fc1d 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts @@ -47,7 +47,7 @@ suite('Notebook Editor', function () { }); - test.skip('showNotebookDocment', async function () { + test('showNotebookDocment', async function () { const count1 = vscode.notebook.notebookDocuments.length; diff --git a/extensions/vscode-api-tests/src/utils.ts b/extensions/vscode-api-tests/src/utils.ts index 8d94c8ea120..a36b7549626 100644 --- a/extensions/vscode-api-tests/src/utils.ts +++ b/extensions/vscode-api-tests/src/utils.ts @@ -122,7 +122,7 @@ export function assertNoRpcFromEntry(entry: [obj: any, name: string]) { assert.strictEqual(proxyPaths.length, 0, proxyPaths.join('\n')); // happens... } -export async function asPromise(event: vscode.Event, timeout = 5000): Promise { +export async function asPromise(event: vscode.Event, timeout = vscode.env.uiKind === vscode.UIKind.Desktop ? 5000 : 15000): Promise { return new Promise((resolve, reject) => { const handle = setTimeout(() => { From 367c5e2dd6b6223773cc13bb1bc23170ae56089b Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 09:04:29 +0200 Subject: [PATCH 13/25] add some todo-tags --- src/vs/vscode.proposed.d.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 58c8fcdfdd9..745529213af 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1066,6 +1066,8 @@ declare module 'vscode' { // todo@API should we really expose this? readonly viewType: string; + // todo@API add cellAt(index): NotebookCell + // todo@API add cellCount: number; /** @deprecated Use `getCells(<...>) instead */ readonly cells: ReadonlyArray; From bbdc0e4c79ca2b647b85b1a944553faad7f725cd Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 10:42:32 +0200 Subject: [PATCH 14/25] add NotebookSelector which is like DocumentSelector --- src/vs/vscode.proposed.d.ts | 8 +++++ .../notebook/common/notebookSelector.ts | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 src/vs/workbench/contrib/notebook/common/notebookSelector.ts diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 745529213af..fe984b9c5fb 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1607,6 +1607,14 @@ declare module 'vscode' { filenamePattern?: NotebookFilenamePattern; } + // export interface NotebookFilter { + // readonly viewType?: string; + // readonly scheme?: string; + // readonly pattern?: GlobPattern; + // } + + // export type NotebookSelector = NotebookFilter | string | ReadonlyArray; + // todo@API very unclear, provider MUST not return alive object but only data object // todo@API unclear how the flow goes export interface NotebookKernelProvider { diff --git a/src/vs/workbench/contrib/notebook/common/notebookSelector.ts b/src/vs/workbench/contrib/notebook/common/notebookSelector.ts new file mode 100644 index 00000000000..5536377ec79 --- /dev/null +++ b/src/vs/workbench/contrib/notebook/common/notebookSelector.ts @@ -0,0 +1,31 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IRelativePattern } from 'vs/base/common/glob'; +import { URI } from 'vs/base/common/uri'; +import * as ls from 'vs/editor/common/modes/languageSelector'; + +export interface NotebookFilter { + readonly viewType?: string; + readonly scheme?: string; + readonly pattern?: string | IRelativePattern; +} + +export type NotebookSelector = NotebookFilter | string | ReadonlyArray; + +function _asLanguageSelector(s: NotebookSelector): ls.LanguageFilter | ls.LanguageFilter[] { + if (Array.isArray(s)) { + return s.map(_asLanguageSelector); + } else if (typeof s === 'string') { + return { language: s }; + } else { + const { viewType, scheme, pattern } = s; + return { language: viewType, scheme: scheme, pattern: pattern }; + } +} + +export function score(selector: NotebookSelector, candidateUri: URI, candidateViewType: string): number { + return ls.score(_asLanguageSelector(selector), candidateUri, candidateViewType, true); +} From 6b5f2532d9f4399f1378cbd6b30b6edca3864e8c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 11:41:16 +0200 Subject: [PATCH 15/25] cellCount and cellAt API proposal so that notebook aligns better with text document --- src/vs/vscode.proposed.d.ts | 16 ++++++++++++++-- .../api/common/extHostNotebookDocument.ts | 16 +++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index fe984b9c5fb..7b783774c5b 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1066,11 +1066,23 @@ declare module 'vscode' { // todo@API should we really expose this? readonly viewType: string; - // todo@API add cellAt(index): NotebookCell - // todo@API add cellCount: number; + // todo@API cellsAt(range)? getCell(index>)? /** @deprecated Use `getCells(<...>) instead */ readonly cells: ReadonlyArray; + /** + * The number of cells in the notebook document. + */ + readonly cellCount: number; + + /** + * Return the cell at the specified index. The index will be adjusted to the notebook. + * + * @param index - The index of the cell to retrieve. + * @return A [cell](#NotebookCell). + */ + cellAt(index: number): NotebookCell; + /** * Get the cells of this notebook. A subset can be retrieved by providing * a range. The range will be adjuset to the notebook. diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index 4db521e3880..e243e92ce61 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -179,8 +179,12 @@ export class ExtHostNotebookDocument extends Disposable { get isUntitled() { return that.uri.scheme === Schemas.untitled; }, get isClosed() { return that._disposed; }, get metadata() { return that._metadata; }, - set metadata(_value: Required) { throw new Error('Use WorkspaceEdit to update metadata.'); }, get cells(): ReadonlyArray { return that._cells.map(cell => cell.cell); }, + get cellCount() { return that._cells.length; }, + cellAt(index) { + index = that._validateIndex(index); + return that._cells[index].cell; + }, getCells(range) { const cells = range ? that._getCells(range) : that._cells; return cells.map(cell => cell.cell); @@ -232,6 +236,16 @@ export class ExtHostNotebookDocument extends Disposable { } } + private _validateIndex(index: number): number { + if (index < 0) { + return 0; + } else if (index >= this._cells.length) { + return this._cells.length - 1; + } else { + return index; + } + } + private _validateRange(range: vscode.NotebookCellRange): vscode.NotebookCellRange { if (range.start < 0) { range = range.with({ start: 0 }); From dd360b25da3a3cf972041b2517543b945ff27c20 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 12:06:03 +0200 Subject: [PATCH 16/25] add notebookEditorModelResolverService.isDirty and use that when forwarding events --- .../api/browser/mainThreadNotebook.ts | 22 ++++++++----------- .../notebookEditorModelResolverService.ts | 2 ++ .../notebookEditorModelResolverServiceImpl.ts | 17 +++++++++++++- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index 8b994acf80e..0f47f970516 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -260,7 +260,7 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { return; } const disposableStore = new DisposableStore(); - disposableStore.add(textModel!.onDidChangeContent(event => { + disposableStore.add(textModel.onDidChangeContent(event => { const dto = event.rawEvents.map(e => { const data = e.kind === NotebookCellsChangeType.ModelChange || e.kind === NotebookCellsChangeType.Initialize @@ -285,18 +285,14 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { return data; }); - /** - * TODO@rebornix, @jrieken - * When a document is modified, it will trigger onDidChangeContent events. - * The first event listener is this one, which doesn't know if the text model is dirty or not. It can ask `workingCopyService` but get the wrong result - * The second event listener is `NotebookEditorModel`, which will then set `isDirty` to `true`. - * Since `e.transient` decides if the model should be dirty or not, we will use the same logic here. - */ - const hasNonTransientEvent = event.rawEvents.find(e => !e.transient); - this._proxy.$acceptModelChanged(textModel.uri, { - rawEvents: dto, - versionId: event.versionId - }, !!hasNonTransientEvent); + // using the model resolver service to know if the model is dirty or not. + // assuming this is the first listener it can mean that at first the model + // is marked as dirty and that another event is fired + this._proxy.$acceptModelChanged( + textModel.uri, + { rawEvents: dto, versionId: event.versionId }, + this._notebookEditorModelResolverService.isDirty(textModel.uri) + ); const hasDocumentMetadataChangeEvent = event.rawEvents.find(e => e.kind === NotebookCellsChangeType.ChangeDocumentMetadata); if (!!hasDocumentMetadataChangeEvent) { diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts index 52c13ae3031..fe2fb23f0e1 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts @@ -17,5 +17,7 @@ export interface INotebookEditorModelResolverService { readonly onDidSaveNotebook: Event; readonly onDidChangeDirty: Event<{ resource: URI, isDirty: boolean }>; + isDirty(resource: URI): boolean; + resolve(resource: URI, viewType?: string): Promise>; } diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts index 1daf24a0f39..bf9b8f9f7a1 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverServiceImpl.ts @@ -15,6 +15,7 @@ import { FileWorkingCopyManager, IFileWorkingCopyManager } from 'vs/workbench/se import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity'; import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService'; +import { ResourceMap } from 'vs/base/common/map'; class NotebookModelReferenceCollection extends ReferenceCollection> { @@ -28,6 +29,8 @@ class NotebookModelReferenceCollection extends ReferenceCollection(); readonly onDidChangeDirty: Event<{ resource: URI, isDirty: boolean }> = this._onDidChangeDirty.event; + private readonly _dirtyStates = new ResourceMap(); + constructor( @IInstantiationService readonly _instantiationService: IInstantiationService, @INotebookService private readonly _notebookService: INotebookService, @@ -48,6 +51,10 @@ class NotebookModelReferenceCollection extends ReferenceCollection { const uri = URI.parse(key); const info = await this._notebookService.withNotebookDataProvider(uri, viewType); @@ -68,7 +75,11 @@ class NotebookModelReferenceCollection extends ReferenceCollection this._onDidSaveNotebook.fire(result.resource)), - result.onDidChangeDirty(() => this._onDidChangeDirty.fire({ resource: result.resource, isDirty: result.isDirty() })), + result.onDidChangeDirty(() => { + const isDirty = result.isDirty(); + this._dirtyStates.set(result.resource, isDirty); + this._onDidChangeDirty.fire({ resource: result.resource, isDirty }); + }), )); return result; } @@ -108,6 +119,10 @@ export class NotebookModelResolverServiceImpl implements INotebookEditorModelRes this._data.dispose(); } + isDirty(resource: URI): boolean { + return this._data.isDirty(resource); + } + async resolve(resource: URI, viewType?: string): Promise> { if (resource.scheme === CellUri.scheme) { From fa48622fdf96268ca5b2ba7465bf8ff307fb56da Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 15:44:31 +0200 Subject: [PATCH 17/25] split mainThreadNotebook up into logical pieces * notebookDocumentsAndEditors -> owns the truth of the notebooks and editor that extensions know about * notebookEditor -> everything about editors * notebookDocuments -> everything about documents --- .../api/browser/extensionHost.contribution.ts | 1 + .../api/browser/mainThreadNotebook.ts | 405 +----------------- .../browser/mainThreadNotebookDocuments.ts | 144 +++++++ .../mainThreadNotebookDocumentsAndEditor.ts | 254 +++++++++++ .../api/browser/mainThreadNotebookEditors.ts | 172 ++++++++ .../workbench/api/common/extHost.protocol.ts | 16 +- .../workbench/api/common/extHostNotebook.ts | 58 +-- .../api/common/extHostNotebookDocument.ts | 4 +- .../api/common/extHostNotebookEditor.ts | 7 +- 9 files changed, 624 insertions(+), 437 deletions(-) create mode 100644 src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts create mode 100644 src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor.ts create mode 100644 src/vs/workbench/api/browser/mainThreadNotebookEditors.ts diff --git a/src/vs/workbench/api/browser/extensionHost.contribution.ts b/src/vs/workbench/api/browser/extensionHost.contribution.ts index 583887ec6ad..42b7bfdbbb0 100644 --- a/src/vs/workbench/api/browser/extensionHost.contribution.ts +++ b/src/vs/workbench/api/browser/extensionHost.contribution.ts @@ -63,6 +63,7 @@ import './mainThreadWebviewManager'; import './mainThreadWorkspace'; import './mainThreadComments'; import './mainThreadNotebook'; +import './mainThreadNotebookDocumentsAndEditor'; import './mainThreadTask'; import './mainThreadLabelService'; import './mainThreadTunnelService'; diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index 0f47f970516..d24e48acab3 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -5,97 +5,18 @@ import { VSBuffer } from 'vs/base/common/buffer'; import { CancellationToken } from 'vs/base/common/cancellation'; -import { diffMaps, diffSets } from 'vs/base/common/collections'; import { Emitter } from 'vs/base/common/event'; import { IRelativePattern } from 'vs/base/common/glob'; import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; -import { ResourceMap } from 'vs/base/common/map'; import { URI, UriComponents } from 'vs/base/common/uri'; -import { EditorActivation, EditorOverride } from 'vs/platform/editor/common/editor'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ILogService } from 'vs/platform/log/common/log'; -import { BoundModelReferenceCollection } from 'vs/workbench/api/browser/mainThreadDocuments'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; -import { getNotebookEditorFromEditorPane, IActiveNotebookEditor, INotebookEditor, NotebookEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { NotebookEditorInput } from 'vs/workbench/contrib/notebook/common/notebookEditorInput'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; -import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService'; -import { ICellEditOperation, ICellRange, IImmediateCellEditOperation, IMainCellDto, INotebookDecorationRenderOptions, INotebookDocumentFilter, INotebookExclusiveDocumentFilter, INotebookKernel, NotebookCellsChangeType, NotebookDataDto, TransientMetadata, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService'; +import { ICellRange, INotebookDocumentFilter, INotebookExclusiveDocumentFilter, INotebookKernel, NotebookDataDto, TransientMetadata, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; -import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; -import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity'; -import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookCellStatusBarEntryDto, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, INotebookModelAddedData, MainContext, MainThreadNotebookShape, NotebookEditorRevealType, NotebookExtensionDescription } from '../common/extHost.protocol'; - -class NotebookAndEditorState { - static compute(before: NotebookAndEditorState | undefined, after: NotebookAndEditorState): INotebookDocumentsAndEditorsDelta { - if (!before) { - return { - addedDocuments: [...after.documents].map(NotebookAndEditorState._asModelAddData), - addedEditors: [...after.textEditors.values()].map(NotebookAndEditorState._asEditorAddData), - visibleEditors: [...after.visibleEditors].map(editor => editor[0]) - }; - } - const documentDelta = diffSets(before.documents, after.documents); - const editorDelta = diffMaps(before.textEditors, after.textEditors); - const addedAPIEditors = editorDelta.added.map(NotebookAndEditorState._asEditorAddData); - - const removedAPIEditors = editorDelta.removed.map(removed => removed.getId()); - const newActiveEditor = before.activeEditor !== after.activeEditor ? after.activeEditor : undefined; - const visibleEditorDelta = diffMaps(before.visibleEditors, after.visibleEditors); - - return { - addedDocuments: documentDelta.added.map(NotebookAndEditorState._asModelAddData), - removedDocuments: documentDelta.removed.map(e => e.uri), - addedEditors: addedAPIEditors, - removedEditors: removedAPIEditors, - newActiveEditor: newActiveEditor, - visibleEditors: visibleEditorDelta.added.length === 0 && visibleEditorDelta.removed.length === 0 - ? undefined - : [...after.visibleEditors].map(editor => editor[0]) - }; - } - - constructor( - readonly documents: Set, - readonly textEditors: Map, - readonly activeEditor: string | null | undefined, - readonly visibleEditors: Map - ) { - // - } - - private static _asModelAddData(e: NotebookTextModel): INotebookModelAddedData { - return { - viewType: e.viewType, - uri: e.uri, - metadata: e.metadata, - versionId: e.versionId, - cells: e.cells.map(cell => ({ - handle: cell.handle, - uri: cell.uri, - source: cell.textBuffer.getLinesContent(), - eol: cell.textBuffer.getEOL(), - language: cell.language, - cellKind: cell.cellKind, - outputs: cell.outputs, - metadata: cell.metadata - })) - }; - } - - private static _asEditorAddData(add: IActiveNotebookEditor): INotebookEditorAddData { - return { - id: add.getId(), - documentUri: add.viewModel.uri, - selections: add.getSelections(), - visibleRanges: add.visibleRanges, - viewColumn: undefined - }; - } -} +import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookCellStatusBarEntryDto, MainContext, MainThreadNotebookShape, NotebookExtensionDescription } from '../common/extHost.protocol'; @extHostNamedCustomer(MainContext.MainThreadNotebook) export class MainThreadNotebooks implements MainThreadNotebookShape { @@ -106,34 +27,25 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { private readonly _notebookProviders = new Map(); private readonly _notebookSerializer = new Map(); private readonly _notebookKernelProviders = new Map, provider: IDisposable }>(); - private readonly _editorEventListenersMapping = new Map(); - private readonly _documentEventListenersMapping = new ResourceMap(); private readonly _cellStatusBarEntries = new Map(); - private readonly _modelReferenceCollection: BoundModelReferenceCollection; - - private _currentState?: NotebookAndEditorState; constructor( extHostContext: IExtHostContext, - @IInstantiationService private readonly _instantiationService: IInstantiationService, + @IInstantiationService instantiationService: IInstantiationService, @INotebookService private readonly _notebookService: INotebookService, @INotebookEditorService private readonly _notebookEditorService: INotebookEditorService, - @IEditorService private readonly _editorService: IEditorService, @ILogService private readonly _logService: ILogService, @INotebookCellStatusBarService private readonly _cellStatusBarService: INotebookCellStatusBarService, - @INotebookEditorModelResolverService private readonly _notebookEditorModelResolverService: INotebookEditorModelResolverService, - @IUriIdentityService private readonly _uriIdentityService: IUriIdentityService ) { this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook); - this._modelReferenceCollection = new BoundModelReferenceCollection(this._uriIdentityService.extUri); + + this._registerListeners(); } dispose(): void { this._disposables.dispose(); - this._modelReferenceCollection.dispose(); - // remove all notebook providers for (const item of this._notebookProviders.values()) { item.disposable.dispose(); @@ -145,238 +57,14 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { item.provider.dispose(); } dispose(this._notebookSerializer.values()); - dispose(this._editorEventListenersMapping.values()); - dispose(this._documentEventListenersMapping.values()); dispose(this._cellStatusBarEntries.values()); } - async $tryApplyEdits(_viewType: string, resource: UriComponents, modelVersionId: number, cellEdits: ICellEditOperation[]): Promise { - const textModel = this._notebookService.getNotebookTextModel(URI.from(resource)); - if (!textModel) { - return false; - } - if (textModel.versionId !== modelVersionId) { - return false; - } - return textModel.applyEdits(cellEdits, true, undefined, () => undefined, undefined); - } - - async $applyEdits(resource: UriComponents, cellEdits: IImmediateCellEditOperation[], computeUndoRedo = true): Promise { - const textModel = this._notebookService.getNotebookTextModel(URI.from(resource)); - if (!textModel) { - throw new Error(`Can't apply edits to unknown notebook model: ${resource}`); - } - - textModel.applyEdits(cellEdits, true, undefined, () => undefined, undefined, computeUndoRedo); - } private _registerListeners(): void { - - this._notebookEditorModelResolverService.onDidChangeDirty(model => { - this._proxy.$acceptDirtyStateChanged(model.resource, model.isDirty); - }); - - this._disposables.add(this._notebookEditorModelResolverService.onDidSaveNotebook(e => { - this._proxy.$acceptModelSaved(e); - })); - - this._disposables.add(this._editorService.onDidActiveEditorChange(e => { - this._updateState(); - })); - - this._disposables.add(this._editorService.onDidVisibleEditorsChange(e => { - if (this._notebookProviders.size > 0) { // TODO@rebornix propably wrong, what about providers from another host - if (!this._currentState) { - // no current state means we didn't even create editors in ext host yet. - return; - } - - // we can't simply update visibleEditors as we need to check if we should create editors first. - this._updateState(); - } - })); - - const handleNotebookEditorAdded = (editor: INotebookEditor) => { - if (this._editorEventListenersMapping.has(editor.getId())) { - //todo@jrieken a bug when this happens? - return; - } - const disposableStore = new DisposableStore(); - disposableStore.add(editor.onDidChangeVisibleRanges(() => { - this._proxy.$acceptEditorPropertiesChanged(editor.getId(), { visibleRanges: { ranges: editor.visibleRanges } }); - })); - - disposableStore.add(editor.onDidChangeSelection(() => { - this._proxy.$acceptEditorPropertiesChanged(editor.getId(), { selections: { selections: editor.getSelections() } }); - })); - - disposableStore.add(editor.onDidChangeKernel(() => { - if (!editor.hasModel()) { - return; - } - this._proxy.$acceptNotebookActiveKernelChange({ - uri: editor.viewModel.uri, - providerHandle: editor.activeKernel?.providerHandle, - kernelFriendlyId: editor.activeKernel?.friendlyId - }); - })); - - disposableStore.add(editor.onDidChangeModel(() => this._updateState())); - disposableStore.add(editor.onDidFocusEditorWidget(() => this._updateState(editor))); - - this._editorEventListenersMapping.set(editor.getId(), disposableStore); - - const activeNotebookEditor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane); - this._updateState(activeNotebookEditor); - }; - - this._notebookEditorService.listNotebookEditors().forEach(handleNotebookEditorAdded); - this._disposables.add(this._notebookEditorService.onDidAddNotebookEditor(handleNotebookEditorAdded)); - - this._disposables.add(this._notebookEditorService.onDidRemoveNotebookEditor(editor => { - this._editorEventListenersMapping.get(editor.getId())?.dispose(); - this._editorEventListenersMapping.delete(editor.getId()); - this._updateState(); - })); - - - const cellToDto = (cell: NotebookCellTextModel): IMainCellDto => { - return { - handle: cell.handle, - uri: cell.uri, - source: cell.textBuffer.getLinesContent(), - eol: cell.textBuffer.getEOL(), - language: cell.language, - cellKind: cell.cellKind, - outputs: cell.outputs, - metadata: cell.metadata - }; - }; - - - const handleNotebookDocumentAdded = (textModel: NotebookTextModel) => { - if (this._documentEventListenersMapping.has(textModel.uri)) { - //todo@jrieken a bug when this happens? - return; - } - const disposableStore = new DisposableStore(); - disposableStore.add(textModel.onDidChangeContent(event => { - const dto = event.rawEvents.map(e => { - const data = - e.kind === NotebookCellsChangeType.ModelChange || e.kind === NotebookCellsChangeType.Initialize - ? { - kind: e.kind, - versionId: event.versionId, - changes: e.changes.map(diff => [diff[0], diff[1], diff[2].map(cell => cellToDto(cell as NotebookCellTextModel))] as [number, number, IMainCellDto[]]) - } - : ( - e.kind === NotebookCellsChangeType.Move - ? { - kind: e.kind, - index: e.index, - length: e.length, - newIdx: e.newIdx, - versionId: event.versionId, - cells: e.cells.map(cell => cellToDto(cell as NotebookCellTextModel)) - } - : e - ); - - return data; - }); - - // using the model resolver service to know if the model is dirty or not. - // assuming this is the first listener it can mean that at first the model - // is marked as dirty and that another event is fired - this._proxy.$acceptModelChanged( - textModel.uri, - { rawEvents: dto, versionId: event.versionId }, - this._notebookEditorModelResolverService.isDirty(textModel.uri) - ); - - const hasDocumentMetadataChangeEvent = event.rawEvents.find(e => e.kind === NotebookCellsChangeType.ChangeDocumentMetadata); - if (!!hasDocumentMetadataChangeEvent) { - this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { metadata: textModel.metadata }); - } - })); - this._documentEventListenersMapping.set(textModel!.uri, disposableStore); - }; - - this._notebookService.listNotebookDocuments().forEach(handleNotebookDocumentAdded); - this._disposables.add(this._notebookService.onDidAddNotebookDocument(document => { - handleNotebookDocumentAdded(document); - this._updateState(); - })); - - this._disposables.add(this._notebookService.onDidRemoveNotebookDocument(uri => { - this._documentEventListenersMapping.get(uri)?.dispose(); - this._documentEventListenersMapping.delete(uri); - this._updateState(); - })); - this._disposables.add(this._notebookService.onDidChangeNotebookActiveKernel(e => { this._proxy.$acceptNotebookActiveKernelChange(e); })); - - - const notebookEditor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane); - this._updateState(notebookEditor); - } - - private _updateState(focusedNotebookEditor?: INotebookEditor): void { - - const activeNotebookEditor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane); - let activeEditor = activeNotebookEditor?.hasModel() ? activeNotebookEditor.getId() : null; - - const editors = new Map(); - const visibleEditorsMap = new Map(); - - for (const editor of this._notebookEditorService.listNotebookEditors()) { - if (editor.hasModel()) { - editors.set(editor.getId(), editor); - } - } - - this._editorService.visibleEditorPanes.forEach(editorPane => { - const notebookEditor = getNotebookEditorFromEditorPane(editorPane); - if (notebookEditor?.hasModel() && editors.has(notebookEditor.getId())) { - visibleEditorsMap.set(notebookEditor.getId(), notebookEditor); - } - }); - - if (!activeEditor && focusedNotebookEditor?.textModel) { - activeEditor = focusedNotebookEditor.getId(); - } - - const newState = new NotebookAndEditorState(new Set(this._notebookService.listNotebookDocuments()), editors, activeEditor, visibleEditorsMap); - const delta = NotebookAndEditorState.compute(this._currentState, newState); - - this._currentState = newState; - if (!this._isDeltaEmpty(delta)) { - return this._proxy.$acceptDocumentAndEditorsDelta(delta); - } - } - - private _isDeltaEmpty(delta: INotebookDocumentsAndEditorsDelta): boolean { - if (delta.addedDocuments !== undefined && delta.addedDocuments.length > 0) { - return false; - } - if (delta.removedDocuments !== undefined && delta.removedDocuments.length > 0) { - return false; - } - if (delta.addedEditors !== undefined && delta.addedEditors.length > 0) { - return false; - } - if (delta.removedEditors !== undefined && delta.removedEditors.length > 0) { - return false; - } - if (delta.visibleEditors !== undefined && delta.visibleEditors.length > 0) { - return false; - } - if (delta.newActiveEditor !== undefined) { - return false; - } - return true; } async $registerNotebookProvider(extension: NotebookExtensionDescription, viewType: string, options: { @@ -534,49 +222,6 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { return true; } - async $tryRevealRange(id: string, range: ICellRange, revealType: NotebookEditorRevealType): Promise { - const editor = this._notebookEditorService.getNotebookEditor(id); - if (!editor) { - return; - } - const notebookEditor = editor as INotebookEditor; - if (!notebookEditor.hasModel()) { - return; - } - const viewModel = notebookEditor.viewModel; - const cell = viewModel.viewCells[range.start]; - if (!cell) { - return; - } - - switch (revealType) { - case NotebookEditorRevealType.Default: - return notebookEditor.revealCellRangeInView(range); - case NotebookEditorRevealType.InCenter: - return notebookEditor.revealInCenter(cell); - case NotebookEditorRevealType.InCenterIfOutsideViewport: - return notebookEditor.revealInCenterIfOutsideViewport(cell); - case NotebookEditorRevealType.AtTop: - return notebookEditor.revealInViewAtTop(cell); - } - } - - $registerNotebookEditorDecorationType(key: string, options: INotebookDecorationRenderOptions): void { - this._notebookEditorService.registerEditorDecorationType(key, options); - } - - $removeNotebookEditorDecorationType(key: string): void { - this._notebookEditorService.removeEditorDecorationType(key); - } - - $trySetDecorations(id: string, range: ICellRange, key: string): void { - const editor = this._notebookEditorService.getNotebookEditor(id); - if (editor) { - const notebookEditor = editor as INotebookEditor; - notebookEditor.setEditorDecorations(key, range); - } - } - async $setStatusBarEntry(id: number, rawStatusBarEntry: INotebookCellStatusBarEntryDto): Promise { const statusBarEntry = { ...rawStatusBarEntry, @@ -592,44 +237,4 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { this._cellStatusBarEntries.set(id, this._cellStatusBarService.addEntry(statusBarEntry)); } } - - - async $tryOpenDocument(uriComponents: UriComponents): Promise { - const uri = URI.revive(uriComponents); - const ref = await this._notebookEditorModelResolverService.resolve(uri, undefined); - this._modelReferenceCollection.add(uri, ref); - return uri; - } - - async $trySaveDocument(uriComponents: UriComponents) { - const uri = URI.revive(uriComponents); - - const ref = await this._notebookEditorModelResolverService.resolve(uri); - const saveResult = await ref.object.save(); - ref.dispose(); - return saveResult; - } - - async $tryShowNotebookDocument(resource: UriComponents, viewType: string, options: INotebookDocumentShowOptions): Promise { - const editorOptions = new NotebookEditorOptions({ - cellSelections: options.selection && [options.selection], - preserveFocus: options.preserveFocus, - pinned: options.pinned, - // selection: options.selection, - // preserve pre 1.38 behaviour to not make group active when preserveFocus: true - // but make sure to restore the editor to fix https://github.com/microsoft/vscode/issues/79633 - activation: options.preserveFocus ? EditorActivation.RESTORE : undefined, - override: EditorOverride.DISABLED, - }); - - const input = NotebookEditorInput.create(this._instantiationService, URI.revive(resource), viewType); - const editorPane = await this._editorService.openEditor(input, editorOptions, options.position); - const notebookEditor = getNotebookEditorFromEditorPane(editorPane); - - if (notebookEditor) { - return notebookEditor.getId(); - } else { - throw new Error(`Notebook Editor creation failure for documenet ${resource}`); - } - } } diff --git a/src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts b/src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts new file mode 100644 index 00000000000..1d98c60e7cf --- /dev/null +++ b/src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts @@ -0,0 +1,144 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { DisposableStore, dispose } from 'vs/base/common/lifecycle'; +import { ResourceMap } from 'vs/base/common/map'; +import { URI, UriComponents } from 'vs/base/common/uri'; +import { BoundModelReferenceCollection } from 'vs/workbench/api/browser/mainThreadDocuments'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; +import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; +import { IImmediateCellEditOperation, IMainCellDto, NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService'; +import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; +import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity'; +import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, MainThreadNotebookDocumentsShape } from '../common/extHost.protocol'; +import { MainThreadNotebooksAndEditors } from 'vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor'; + +export class MainThreadNotebookDocuments implements MainThreadNotebookDocumentsShape { + + private readonly _disposables = new DisposableStore(); + + private readonly _proxy: ExtHostNotebookShape; + private readonly _documentEventListenersMapping = new ResourceMap(); + private readonly _modelReferenceCollection: BoundModelReferenceCollection; + + constructor( + extHostContext: IExtHostContext, + notebooksAndEditors: MainThreadNotebooksAndEditors, + @INotebookService private readonly _notebookService: INotebookService, + @INotebookEditorModelResolverService private readonly _notebookEditorModelResolverService: INotebookEditorModelResolverService, + @IUriIdentityService private readonly _uriIdentityService: IUriIdentityService + ) { + this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook); + this._modelReferenceCollection = new BoundModelReferenceCollection(this._uriIdentityService.extUri); + + notebooksAndEditors.onDidAddNotebooks(this._handleNotebooksAdded, this, this._disposables); + notebooksAndEditors.onDidRemoveNotebooks(this._handleNotebooksRemoved, this, this._disposables); + + // forward dirty and save events + this._disposables.add(this._notebookEditorModelResolverService.onDidChangeDirty(model => this._proxy.$acceptDirtyStateChanged(model.resource, model.isDirty))); + this._disposables.add(this._notebookEditorModelResolverService.onDidSaveNotebook(e => this._proxy.$acceptModelSaved(e))); + } + + dispose(): void { + this._disposables.dispose(); + this._modelReferenceCollection.dispose(); + dispose(this._documentEventListenersMapping.values()); + + } + + private _handleNotebooksAdded(notebooks: readonly NotebookTextModel[]): void { + + for (const textModel of notebooks) { + const disposableStore = new DisposableStore(); + disposableStore.add(textModel.onDidChangeContent(event => { + const dto = event.rawEvents.map(e => { + const data = + e.kind === NotebookCellsChangeType.ModelChange || e.kind === NotebookCellsChangeType.Initialize + ? { + kind: e.kind, + versionId: event.versionId, + changes: e.changes.map(diff => [diff[0], diff[1], diff[2].map(cell => MainThreadNotebookDocuments._cellToDto(cell as NotebookCellTextModel))] as [number, number, IMainCellDto[]]) + } + : ( + e.kind === NotebookCellsChangeType.Move + ? { + kind: e.kind, + index: e.index, + length: e.length, + newIdx: e.newIdx, + versionId: event.versionId, + cells: e.cells.map(cell => MainThreadNotebookDocuments._cellToDto(cell as NotebookCellTextModel)) + } + : e + ); + + return data; + }); + + // using the model resolver service to know if the model is dirty or not. + // assuming this is the first listener it can mean that at first the model + // is marked as dirty and that another event is fired + this._proxy.$acceptModelChanged( + textModel.uri, + { rawEvents: dto, versionId: event.versionId }, + this._notebookEditorModelResolverService.isDirty(textModel.uri) + ); + + const hasDocumentMetadataChangeEvent = event.rawEvents.find(e => e.kind === NotebookCellsChangeType.ChangeDocumentMetadata); + if (hasDocumentMetadataChangeEvent) { + this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { metadata: textModel.metadata }); + } + })); + + this._documentEventListenersMapping.set(textModel.uri, disposableStore); + } + } + + private _handleNotebooksRemoved(uris: URI[]): void { + for (const uri of uris) { + this._documentEventListenersMapping.get(uri)?.dispose(); + this._documentEventListenersMapping.delete(uri); + } + } + + private static _cellToDto(cell: NotebookCellTextModel): IMainCellDto { + return { + handle: cell.handle, + uri: cell.uri, + source: cell.textBuffer.getLinesContent(), + eol: cell.textBuffer.getEOL(), + language: cell.language, + cellKind: cell.cellKind, + outputs: cell.outputs, + metadata: cell.metadata + }; + } + + async $tryOpenDocument(uriComponents: UriComponents): Promise { + const uri = URI.revive(uriComponents); + const ref = await this._notebookEditorModelResolverService.resolve(uri, undefined); + this._modelReferenceCollection.add(uri, ref); + return uri; + } + + async $trySaveDocument(uriComponents: UriComponents) { + const uri = URI.revive(uriComponents); + + const ref = await this._notebookEditorModelResolverService.resolve(uri); + const saveResult = await ref.object.save(); + ref.dispose(); + return saveResult; + } + + async $applyEdits(resource: UriComponents, cellEdits: IImmediateCellEditOperation[], computeUndoRedo = true): Promise { + const textModel = this._notebookService.getNotebookTextModel(URI.from(resource)); + if (!textModel) { + throw new Error(`Can't apply edits to unknown notebook model: ${resource}`); + } + + textModel.applyEdits(cellEdits, true, undefined, () => undefined, undefined, computeUndoRedo); + } +} diff --git a/src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor.ts b/src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor.ts new file mode 100644 index 00000000000..7954d1ae70e --- /dev/null +++ b/src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor.ts @@ -0,0 +1,254 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { diffMaps, diffSets } from 'vs/base/common/collections'; +import { Emitter, Event } from 'vs/base/common/event'; +import { combinedDisposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; +import { URI } from 'vs/base/common/uri'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { MainThreadNotebookDocuments } from 'vs/workbench/api/browser/mainThreadNotebookDocuments'; +import { MainThreadNotebookEditors } from 'vs/workbench/api/browser/mainThreadNotebookEditors'; +import { extHostCustomer } from 'vs/workbench/api/common/extHostCustomers'; +import { editorGroupToViewColumn } from 'vs/workbench/common/editor'; +import { getNotebookEditorFromEditorPane, IActiveNotebookEditor, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; +import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; +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'; +import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookDocumentsAndEditorsDelta, INotebookEditorAddData, INotebookModelAddedData, MainContext } from '../common/extHost.protocol'; + +interface INotebookAndEditorDelta { + removedDocuments: URI[]; + addedDocuments: NotebookTextModel[]; + removedEditors: string[]; + addedEditors: IActiveNotebookEditor[]; + newActiveEditor?: string | null; + visibleEditors?: string[]; +} + +class NotebookAndEditorState { + static compute(before: NotebookAndEditorState | undefined, after: NotebookAndEditorState): INotebookAndEditorDelta { + if (!before) { + return { + addedDocuments: [...after.documents], + removedDocuments: [], + addedEditors: [...after.textEditors.values()], + removedEditors: [], + visibleEditors: [...after.visibleEditors].map(editor => editor[0]) + }; + } + const documentDelta = diffSets(before.documents, after.documents); + const editorDelta = diffMaps(before.textEditors, after.textEditors); + + const newActiveEditor = before.activeEditor !== after.activeEditor ? after.activeEditor : undefined; + const visibleEditorDelta = diffMaps(before.visibleEditors, after.visibleEditors); + + return { + addedDocuments: documentDelta.added, + removedDocuments: documentDelta.removed.map(e => e.uri), + addedEditors: editorDelta.added, + removedEditors: editorDelta.removed.map(removed => removed.getId()), + newActiveEditor: newActiveEditor, + visibleEditors: visibleEditorDelta.added.length === 0 && visibleEditorDelta.removed.length === 0 + ? undefined + : [...after.visibleEditors].map(editor => editor[0]) + }; + } + + constructor( + readonly documents: Set, + readonly textEditors: Map, + readonly activeEditor: string | null | undefined, + readonly visibleEditors: Map + ) { + // + } +} + +@extHostCustomer +export class MainThreadNotebooksAndEditors { + + private readonly _onDidAddNotebooks = new Emitter(); + private readonly _onDidRemoveNotebooks = new Emitter(); + private readonly _onDidAddEditors = new Emitter(); + private readonly _onDidRemoveEditors = new Emitter(); + + readonly onDidAddNotebooks: Event = this._onDidAddNotebooks.event; + readonly onDidRemoveNotebooks: Event = this._onDidRemoveNotebooks.event; + readonly onDidAddEditors: Event = this._onDidAddEditors.event; + readonly onDidRemoveEditors: Event = this._onDidRemoveEditors.event; + + private readonly _proxy: Pick; + private readonly _disposables = new DisposableStore(); + + private readonly _editorListeners = new Map(); + + private _currentState?: NotebookAndEditorState; + + private readonly _mainThreadNotebooks: MainThreadNotebookDocuments; + private readonly _mainThreadEditors: MainThreadNotebookEditors; + + constructor( + extHostContext: IExtHostContext, + @IInstantiationService instantiationService: IInstantiationService, + @INotebookService private readonly _notebookService: INotebookService, + @INotebookEditorService private readonly _notebookEditorService: INotebookEditorService, + @IEditorService private readonly _editorService: IEditorService, + @IEditorGroupsService private readonly _editorGroupService: IEditorGroupsService, + ) { + this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook); + + this._mainThreadNotebooks = instantiationService.createInstance(MainThreadNotebookDocuments, extHostContext, this); + this._mainThreadEditors = instantiationService.createInstance(MainThreadNotebookEditors, extHostContext, this); + + extHostContext.set(MainContext.MainThreadNotebookDocuments, this._mainThreadNotebooks); + extHostContext.set(MainContext.MainThreadNotebookEditors, this._mainThreadEditors); + + this._notebookService.onDidAddNotebookDocument(() => this._updateState(), this, this._disposables); + this._notebookService.onDidRemoveNotebookDocument(() => this._updateState(), this, this._disposables); + this._editorService.onDidActiveEditorChange(() => this._updateState(), this, this._disposables); + this._editorService.onDidVisibleEditorsChange(() => this._updateState(), this, this._disposables); + this._notebookEditorService.onDidAddNotebookEditor(this._handleEditorAdd, this, this._disposables); + this._notebookEditorService.onDidRemoveNotebookEditor(this._handleEditorRemove, this, this._disposables); + this._updateState(); + } + + dispose() { + this._mainThreadNotebooks.dispose(); + this._mainThreadEditors.dispose(); + this._onDidAddEditors.dispose(); + this._onDidRemoveEditors.dispose(); + this._onDidAddNotebooks.dispose(); + this._onDidRemoveNotebooks.dispose(); + this._disposables.dispose(); + } + + private _handleEditorAdd(editor: INotebookEditor): void { + this._editorListeners.set(editor.getId(), combinedDisposable( + editor.onDidChangeModel(() => this._updateState()), + editor.onDidFocusEditorWidget(() => this._updateState(editor)), + )); + this._updateState(); + } + + private _handleEditorRemove(editor: INotebookEditor): void { + this._editorListeners.get(editor.getId())?.dispose(); + this._editorListeners.delete(editor.getId()); + this._updateState(); + } + + private _updateState(focusedEditor?: INotebookEditor): void { + + const editors = new Map(); + const visibleEditorsMap = new Map(); + + for (const editor of this._notebookEditorService.listNotebookEditors()) { + if (editor.hasModel()) { + editors.set(editor.getId(), editor); + } + } + + const activeNotebookEditor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane); + let activeEditor: string | null = null; + if (activeNotebookEditor) { + activeEditor = activeNotebookEditor.getId(); + } else if (focusedEditor?.textModel) { + activeEditor = focusedEditor.getId(); + } + if (activeEditor && !editors.has(activeEditor)) { + activeEditor = null; + } + + for (const editorPane of this._editorService.visibleEditorPanes) { + const notebookEditor = getNotebookEditorFromEditorPane(editorPane); + if (notebookEditor?.hasModel() && editors.has(notebookEditor.getId())) { + visibleEditorsMap.set(notebookEditor.getId(), notebookEditor); + } + } + + const newState = new NotebookAndEditorState(new Set(this._notebookService.listNotebookDocuments()), editors, activeEditor, visibleEditorsMap); + this._onDelta(NotebookAndEditorState.compute(this._currentState, newState)); + this._currentState = newState; + } + + private _onDelta(delta: INotebookAndEditorDelta): void { + if (MainThreadNotebooksAndEditors._isDeltaEmpty(delta)) { + return; + } + + const dto: INotebookDocumentsAndEditorsDelta = { + removedDocuments: delta.removedDocuments, + removedEditors: delta.removedEditors, + newActiveEditor: delta.newActiveEditor, + visibleEditors: delta.visibleEditors, + addedDocuments: delta.addedDocuments.map(MainThreadNotebooksAndEditors._asModelAddData), + addedEditors: delta.addedEditors.map(this._asEditorAddData, this), + }; + + // send to extension FIRST + this._proxy.$acceptDocumentAndEditorsDelta(dto); + + // handle internally + this._onDidRemoveEditors.fire(delta.removedEditors); + this._onDidRemoveNotebooks.fire(delta.removedDocuments); + this._onDidAddNotebooks.fire(delta.addedDocuments); + this._onDidAddEditors.fire(delta.addedEditors); + } + + private static _isDeltaEmpty(delta: INotebookAndEditorDelta): boolean { + if (delta.addedDocuments !== undefined && delta.addedDocuments.length > 0) { + return false; + } + if (delta.removedDocuments !== undefined && delta.removedDocuments.length > 0) { + return false; + } + if (delta.addedEditors !== undefined && delta.addedEditors.length > 0) { + return false; + } + if (delta.removedEditors !== undefined && delta.removedEditors.length > 0) { + return false; + } + if (delta.visibleEditors !== undefined && delta.visibleEditors.length > 0) { + return false; + } + if (delta.newActiveEditor !== undefined) { + return false; + } + return true; + } + + private static _asModelAddData(e: NotebookTextModel): INotebookModelAddedData { + return { + viewType: e.viewType, + uri: e.uri, + metadata: e.metadata, + versionId: e.versionId, + cells: e.cells.map(cell => ({ + handle: cell.handle, + uri: cell.uri, + source: cell.textBuffer.getLinesContent(), + eol: cell.textBuffer.getEOL(), + language: cell.language, + cellKind: cell.cellKind, + outputs: cell.outputs, + metadata: cell.metadata + })) + }; + } + + private _asEditorAddData(add: IActiveNotebookEditor): INotebookEditorAddData { + + const pane = this._editorService.visibleEditorPanes.find(pane => getNotebookEditorFromEditorPane(pane) === add); + + return { + id: add.getId(), + documentUri: add.viewModel.uri, + selections: add.getSelections(), + visibleRanges: add.visibleRanges, + viewColumn: pane && editorGroupToViewColumn(this._editorGroupService, pane.group) + }; + } +} diff --git a/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts b/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts new file mode 100644 index 00000000000..9cb6973d3cd --- /dev/null +++ b/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts @@ -0,0 +1,172 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { DisposableStore, dispose } from 'vs/base/common/lifecycle'; +import { getNotebookEditorFromEditorPane, INotebookEditor, NotebookEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; +import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookDocumentShowOptions, MainThreadNotebookEditorsShape, NotebookEditorRevealType } from '../common/extHost.protocol'; +import { MainThreadNotebooksAndEditors } from 'vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor'; +import { ICellEditOperation, ICellRange, INotebookDecorationRenderOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ILogService } from 'vs/platform/log/common/log'; +import { URI, UriComponents } from 'vs/base/common/uri'; +import { EditorActivation, EditorOverride } from 'vs/platform/editor/common/editor'; +import { NotebookEditorInput } from 'vs/workbench/contrib/notebook/common/notebookEditorInput'; +import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; + +class MainThreadEditor { + constructor( + readonly editor: INotebookEditor, + readonly disposables: DisposableStore + ) { } + dispose() { + this.disposables.dispose(); + } +} + +export class MainThreadNotebookEditors implements MainThreadNotebookEditorsShape { + + private readonly _disposables = new DisposableStore(); + + private readonly _proxy: ExtHostNotebookShape; + private readonly _editorEventListenersMapping = new Map(); + + constructor( + extHostContext: IExtHostContext, + notebooksAndEditors: MainThreadNotebooksAndEditors, + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @IEditorService private readonly _editorService: IEditorService, + @ILogService private readonly _logService: ILogService, + @INotebookEditorService private readonly _notebookEditorService: INotebookEditorService, + ) { + this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook); + + notebooksAndEditors.onDidAddEditors(this._handleEditorsAdded, this, this._disposables); + notebooksAndEditors.onDidRemoveEditors(this._handleEditorsRemoved, this, this._disposables); + } + + dispose(): void { + this._disposables.dispose(); + dispose(this._editorEventListenersMapping.values()); + } + + private _handleEditorsAdded(editors: readonly INotebookEditor[]): void { + + for (const editor of editors) { + + const editorDisposables = new DisposableStore(); + editorDisposables.add(editor.onDidChangeVisibleRanges(() => { + this._proxy.$acceptEditorPropertiesChanged(editor.getId(), { visibleRanges: { ranges: editor.visibleRanges } }); + })); + + editorDisposables.add(editor.onDidChangeSelection(() => { + this._proxy.$acceptEditorPropertiesChanged(editor.getId(), { selections: { selections: editor.getSelections() } }); + })); + + editorDisposables.add(editor.onDidChangeKernel(() => { + if (!editor.hasModel()) { + return; + } + this._proxy.$acceptNotebookActiveKernelChange({ + uri: editor.viewModel.uri, + providerHandle: editor.activeKernel?.providerHandle, + kernelFriendlyId: editor.activeKernel?.friendlyId + }); + })); + + this._editorEventListenersMapping.set(editor.getId(), new MainThreadEditor(editor, editorDisposables)); + } + } + + private _handleEditorsRemoved(editorIds: readonly string[]): void { + for (const id of editorIds) { + this._editorEventListenersMapping.get(id)?.dispose(); + this._editorEventListenersMapping.delete(id); + } + } + + async $tryApplyEdits(editorId: string, modelVersionId: number, cellEdits: ICellEditOperation[]): Promise { + const wrapper = this._editorEventListenersMapping.get(editorId); + if (!wrapper) { + return false; + } + const { editor } = wrapper; + if (!editor.textModel) { + this._logService.warn('Notebook editor has NO model', editorId); + return false; + } + if (editor.textModel.versionId !== modelVersionId) { + return false; + } + //todo@jrieken use proper selection logic! + return editor.textModel.applyEdits(cellEdits, true, undefined, () => undefined, undefined); + } + + async $tryShowNotebookDocument(resource: UriComponents, viewType: string, options: INotebookDocumentShowOptions): Promise { + const editorOptions = new NotebookEditorOptions({ + cellSelections: options.selection && [options.selection], + preserveFocus: options.preserveFocus, + pinned: options.pinned, + // selection: options.selection, + // preserve pre 1.38 behaviour to not make group active when preserveFocus: true + // but make sure to restore the editor to fix https://github.com/microsoft/vscode/issues/79633 + activation: options.preserveFocus ? EditorActivation.RESTORE : undefined, + override: EditorOverride.DISABLED, + }); + + const input = NotebookEditorInput.create(this._instantiationService, URI.revive(resource), viewType); + const editorPane = await this._editorService.openEditor(input, editorOptions, options.position); + const notebookEditor = getNotebookEditorFromEditorPane(editorPane); + + if (notebookEditor) { + return notebookEditor.getId(); + } else { + throw new Error(`Notebook Editor creation failure for documenet ${resource}`); + } + } + + async $tryRevealRange(id: string, range: ICellRange, revealType: NotebookEditorRevealType): Promise { + const editor = this._notebookEditorService.getNotebookEditor(id); + if (!editor) { + return; + } + const notebookEditor = editor as INotebookEditor; + if (!notebookEditor.hasModel()) { + return; + } + const viewModel = notebookEditor.viewModel; + const cell = viewModel.viewCells[range.start]; + if (!cell) { + return; + } + + switch (revealType) { + case NotebookEditorRevealType.Default: + return notebookEditor.revealCellRangeInView(range); + case NotebookEditorRevealType.InCenter: + return notebookEditor.revealInCenter(cell); + case NotebookEditorRevealType.InCenterIfOutsideViewport: + return notebookEditor.revealInCenterIfOutsideViewport(cell); + case NotebookEditorRevealType.AtTop: + return notebookEditor.revealInViewAtTop(cell); + } + } + + $registerNotebookEditorDecorationType(key: string, options: INotebookDecorationRenderOptions): void { + this._notebookEditorService.registerEditorDecorationType(key, options); + } + + $removeNotebookEditorDecorationType(key: string): void { + this._notebookEditorService.removeEditorDecorationType(key); + } + + $trySetDecorations(id: string, range: ICellRange, key: string): void { + const editor = this._notebookEditorService.getNotebookEditor(id); + if (editor) { + const notebookEditor = editor as INotebookEditor; + notebookEditor.setEditorDecorations(key, range); + } + } +} diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 5362193a889..a5574bfec16 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -877,17 +877,23 @@ export interface MainThreadNotebookShape extends IDisposable { $registerNotebookKernelProvider(extension: NotebookExtensionDescription, handle: number, documentFilter: INotebookDocumentFilter): Promise; $unregisterNotebookKernelProvider(handle: number): Promise; $onNotebookKernelChange(handle: number, uri: UriComponents | undefined): void; - $trySaveDocument(uri: UriComponents): Promise; - $tryApplyEdits(viewType: string, resource: UriComponents, modelVersionId: number, edits: ICellEditOperation[]): Promise; - $applyEdits(resource: UriComponents, edits: IImmediateCellEditOperation[], computeUndoRedo?: boolean): Promise; $postMessage(editorId: string, forRendererId: string | undefined, value: any): Promise; $setStatusBarEntry(id: number, statusBarEntry: INotebookCellStatusBarEntryDto): Promise; - $tryOpenDocument(uriComponents: UriComponents): Promise; +} + +export interface MainThreadNotebookEditorsShape extends IDisposable { $tryShowNotebookDocument(uriComponents: UriComponents, viewType: string, options: INotebookDocumentShowOptions): Promise; $tryRevealRange(id: string, range: ICellRange, revealType: NotebookEditorRevealType): Promise; $registerNotebookEditorDecorationType(key: string, options: INotebookDecorationRenderOptions): void; $removeNotebookEditorDecorationType(key: string): void; $trySetDecorations(id: string, range: ICellRange, decorationKey: string): void; + $tryApplyEdits(editorId: string, modelVersionId: number, cellEdits: ICellEditOperation[]): Promise +} + +export interface MainThreadNotebookDocumentsShape extends IDisposable { + $tryOpenDocument(uriComponents: UriComponents): Promise; + $trySaveDocument(uri: UriComponents): Promise; + $applyEdits(resource: UriComponents, edits: IImmediateCellEditOperation[], computeUndoRedo?: boolean): Promise; } export interface MainThreadUrlsShape extends IDisposable { @@ -2007,6 +2013,8 @@ export const MainContext = { MainThreadWindow: createMainId('MainThreadWindow'), MainThreadLabelService: createMainId('MainThreadLabelService'), MainThreadNotebook: createMainId('MainThreadNotebook'), + MainThreadNotebookDocuments: createMainId('MainThreadNotebookDocumentsShape'), + MainThreadNotebookEditors: createMainId('MainThreadNotebookEditorsShape'), MainThreadTheming: createMainId('MainThreadTheming'), MainThreadTunnelService: createMainId('MainThreadTunnelService'), MainThreadTimeline: createMainId('MainThreadTimeline'), diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index f10b8c896d4..94ee6b2a4e3 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -9,7 +9,7 @@ import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/ import { URI, UriComponents } from 'vs/base/common/uri'; import * as UUID from 'vs/base/common/uuid'; import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'; -import { ExtHostNotebookShape, ICommandDto, IMainContext, IModelAddedData, INotebookDocumentPropertiesChangeData, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, INotebookEditorPropertiesChangeData, INotebookKernelInfoDto2, MainContext, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; +import { ExtHostNotebookShape, ICommandDto, IMainContext, IModelAddedData, INotebookDocumentPropertiesChangeData, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, INotebookEditorPropertiesChangeData, INotebookKernelInfoDto2, MainContext, MainThreadNotebookDocumentsShape, MainThreadNotebookEditorsShape, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; import { ILogService } from 'vs/platform/log/common/log'; import { CommandsConverter, ExtHostCommands } from 'vs/workbench/api/common/extHostCommands'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; @@ -180,7 +180,7 @@ export class NotebookEditorDecorationType { readonly value: vscode.NotebookEditorDecorationType; - constructor(proxy: MainThreadNotebookShape, options: vscode.NotebookDecorationRenderOptions) { + constructor(proxy: MainThreadNotebookEditorsShape, options: vscode.NotebookDecorationRenderOptions) { const key = NotebookEditorDecorationType._Keys.nextId(); proxy.$registerNotebookEditorDecorationType(key, typeConverters.NotebookDecorationRenderOptions.from(options)); @@ -202,7 +202,10 @@ type NotebookContentProviderData = { export class ExtHostNotebookController implements ExtHostNotebookShape { private static _notebookKernelProviderHandlePool: number = 0; - private readonly _proxy: MainThreadNotebookShape; + private readonly _notebookProxy: MainThreadNotebookShape; + private readonly _notebookDocumentsProxy: MainThreadNotebookDocumentsShape; + private readonly _notebookEditorsProxy: MainThreadNotebookEditorsShape; + private readonly _notebookContentProviders = new Map(); private readonly _notebookKernelProviders = new Map(); private readonly _documents = new ResourceMap(); @@ -257,7 +260,9 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { private readonly logService: ILogService, private readonly _extensionStoragePaths: IExtensionStoragePaths, ) { - this._proxy = mainContext.getProxy(MainContext.MainThreadNotebook); + this._notebookProxy = mainContext.getProxy(MainContext.MainThreadNotebook); + this._notebookDocumentsProxy = mainContext.getProxy(MainContext.MainThreadNotebookDocuments); + this._notebookEditorsProxy = mainContext.getProxy(MainContext.MainThreadNotebookEditors); this._commandsConverter = commands.converter; commands.registerArgumentProcessor({ @@ -328,7 +333,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { if (provider.onDidChangeNotebookContentOptions) { listener = provider.onDidChangeNotebookContentOptions(() => { const internalOptions = typeConverters.NotebookDocumentContentOptions.from(provider.options); - this._proxy.$updateNotebookProviderOptions(viewType, internalOptions); + this._notebookProxy.$updateNotebookProviderOptions(viewType, internalOptions); }); } @@ -341,7 +346,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { } const internalOptions = typeConverters.NotebookDocumentContentOptions.from(options); - this._proxy.$registerNotebookProvider({ id: extension.identifier, location: extension.extensionLocation, description: extension.description }, viewType, { + this._notebookProxy.$registerNotebookProvider({ id: extension.identifier, location: extension.extensionLocation, description: extension.description }, viewType, { transientOutputs: internalOptions.transientOutputs, transientMetadata: internalOptions.transientMetadata, viewOptions: options?.viewOptions && viewOptionsFilenamePattern ? { displayName: options.viewOptions.displayName, filenamePattern: viewOptionsFilenamePattern, exclusive: options.viewOptions.exclusive || false } : undefined @@ -350,15 +355,15 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { return new extHostTypes.Disposable(() => { listener?.dispose(); this._notebookContentProviders.delete(viewType); - this._proxy.$unregisterNotebookProvider(viewType); + this._notebookProxy.$unregisterNotebookProvider(viewType); }); } registerNotebookKernelProvider(extension: IExtensionDescription, selector: vscode.NotebookDocumentFilter, provider: vscode.NotebookKernelProvider) { const handle = ExtHostNotebookController._notebookKernelProviderHandlePool++; - const adapter = new ExtHostNotebookKernelProviderAdapter(this._proxy, handle, extension, provider); + const adapter = new ExtHostNotebookKernelProviderAdapter(this._notebookProxy, handle, extension, provider); this._notebookKernelProviders.set(handle, adapter); - this._proxy.$registerNotebookKernelProvider({ id: extension.identifier, location: extension.extensionLocation, description: extension.description }, handle, { + this._notebookProxy.$registerNotebookKernelProvider({ id: extension.identifier, location: extension.extensionLocation, description: extension.description }, handle, { viewType: selector.viewType, filenamePattern: selector.filenamePattern ? typeConverters.NotebookExclusiveDocumentPattern.from(selector.filenamePattern) : undefined }); @@ -366,12 +371,12 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { return new extHostTypes.Disposable(() => { adapter.dispose(); this._notebookKernelProviders.delete(handle); - this._proxy.$unregisterNotebookKernelProvider(handle); + this._notebookProxy.$unregisterNotebookKernelProvider(handle); }); } createNotebookEditorDecorationType(options: vscode.NotebookDecorationRenderOptions): vscode.NotebookEditorDecorationType { - return new NotebookEditorDecorationType(this._proxy, options).value; + return new NotebookEditorDecorationType(this._notebookEditorsProxy, options).value; } async openNotebookDocument(uri: URI): Promise { @@ -379,7 +384,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { if (cached) { return cached.notebookDocument; } - const canonicalUri = await this._proxy.$tryOpenDocument(uri); + const canonicalUri = await this._notebookDocumentsProxy.$tryOpenDocument(uri); const document = this._documents.get(URI.revive(canonicalUri)); return assertIsDefined(document?.notebookDocument); } @@ -420,7 +425,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { }; } - const editorId = await this._proxy.$tryShowNotebookDocument(notebookOrUri.uri, notebookOrUri.viewType, resolvedOptions); + const editorId = await this._notebookEditorsProxy.$tryShowNotebookDocument(notebookOrUri.uri, notebookOrUri.viewType, resolvedOptions); const editor = editorId && this._editors.get(editorId)?.editor; if (editor) { @@ -460,7 +465,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { let webComm = this._webviewComm.get(editorId); if (!webComm) { - webComm = new ExtHostWebviewCommWrapper(editorId, revivedUri, this._proxy, this._webviewInitData, document); + webComm = new ExtHostWebviewCommWrapper(editorId, revivedUri, this._notebookProxy, this._webviewInitData, document); this._webviewComm.set(editorId, webComm); } } @@ -480,14 +485,14 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { const handle = this._handlePool++; this._notebookSerializer.set(handle, serializer); const internalOptions = typeConverters.NotebookDocumentContentOptions.from(options); - this._proxy.$registerNotebookSerializer( + this._notebookProxy.$registerNotebookSerializer( handle, { id: extension.identifier, location: extension.extensionLocation, description: extension.description }, viewType, internalOptions ); return toDisposable(() => { - this._proxy.$unregisterNotebookSerializer(handle); + this._notebookProxy.$unregisterNotebookSerializer(handle); }); } @@ -618,7 +623,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { const editor = this._editors.get(id); if (!editor) { - throw new Error(`unknown text editor: ${id}`); + throw new Error(`unknown text editor: ${id}. known editors: ${[...this._editors.keys()]} `); } // ONE: make all state updates @@ -658,14 +663,13 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { let webComm = this._webviewComm.get(editorId); if (!webComm) { - webComm = new ExtHostWebviewCommWrapper(editorId, revivedUri, this._proxy, this._webviewInitData, document); + webComm = new ExtHostWebviewCommWrapper(editorId, revivedUri, this._notebookProxy, this._webviewInitData, document); this._webviewComm.set(editorId, webComm); } const editor = new ExtHostNotebookEditor( editorId, - document.notebookDocument.viewType, - this._proxy, + this._notebookEditorsProxy, document, data.visibleRanges.map(typeConverters.NotebookCellRange.to), data.selections.map(typeConverters.NotebookCellRange.to), @@ -711,12 +715,12 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { const viewType = modelData.viewType; if (this._documents.has(uri)) { - throw new Error(`adding EXISTING notebook ${uri}`); + throw new Error(`adding EXISTING notebook ${uri} `); } const that = this; const document = new ExtHostNotebookDocument( - this._proxy, + this._notebookDocumentsProxy, this._textDocumentsAndEditors, this._textDocuments, { @@ -824,7 +828,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { } createNotebookCellStatusBarItemInternal(cell: vscode.NotebookCell, alignment: extHostTypes.NotebookCellStatusBarAlignment | undefined, priority: number | undefined) { - const statusBarItem = new NotebookCellStatusBarItemInternal(this._proxy, this._commandsConverter, cell, alignment, priority); + const statusBarItem = new NotebookCellStatusBarItemInternal(this._notebookProxy, this._commandsConverter, cell, alignment, priority); // Look up the ExtHostCell for this NotebookCell URI, bind to its disposable lifecycle const parsedUri = CellUri.parse(cell.document.uri); @@ -844,12 +848,12 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { createNotebookCellExecution(docUri: vscode.Uri, index: number, kernelId: string): vscode.NotebookCellExecutionTask | undefined { const document = this.lookupNotebookDocument(docUri); if (!document) { - throw new Error(`Invalid cell uri/index: ${docUri}, ${index}`); + throw new Error(`Invalid cell uri / index: ${docUri}, ${index} `); } const cell = document.getCellFromIndex(index); if (!cell) { - throw new Error(`Invalid cell uri/index: ${docUri}, ${index}`); + throw new Error(`Invalid cell uri / index: ${docUri}, ${index} `); } // TODO@roblou also validate kernelId, once kernel has moved from editor to document @@ -857,7 +861,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { return; } - const execution = new NotebookCellExecutionTask(docUri, document, cell, this._proxy); + const execution = new NotebookCellExecutionTask(docUri, document, cell, this._notebookDocumentsProxy); this._activeExecutions.set(cell.uri, execution); const listener = execution.onDidChangeState(() => { if (execution.state === NotebookCellExecutionTaskState.Resolved) { @@ -1067,7 +1071,7 @@ class NotebookCellExecutionTask extends Disposable { private readonly _uri: vscode.Uri, private readonly _document: ExtHostNotebookDocument, private readonly _cell: ExtHostCell, - private readonly _proxy: MainThreadNotebookShape) { + private readonly _proxy: MainThreadNotebookDocumentsShape) { super(); this._tokenSource = this._register(new CancellationTokenSource()); diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index e243e92ce61..f5f2f35ad08 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -8,7 +8,7 @@ import { Disposable, DisposableStore, dispose } from 'vs/base/common/lifecycle'; import { Schemas } from 'vs/base/common/network'; import { deepFreeze, equals } from 'vs/base/common/objects'; import { URI } from 'vs/base/common/uri'; -import { CellKind, INotebookDocumentPropertiesChangeData, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; +import { CellKind, INotebookDocumentPropertiesChangeData, MainThreadNotebookDocumentsShape } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments'; import { ExtHostDocumentsAndEditors, IExtHostModelAddedData } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; import * as extHostTypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; @@ -149,7 +149,7 @@ export class ExtHostNotebookDocument extends Disposable { private _disposed: boolean = false; constructor( - private readonly _proxy: MainThreadNotebookShape, + private readonly _proxy: MainThreadNotebookDocumentsShape, private readonly _textDocumentsAndEditors: ExtHostDocumentsAndEditors, private readonly _textDocuments: ExtHostDocuments, private readonly _emitter: INotebookEventEmitter, diff --git a/src/vs/workbench/api/common/extHostNotebookEditor.ts b/src/vs/workbench/api/common/extHostNotebookEditor.ts index b8b752a876a..9d57ba51ee1 100644 --- a/src/vs/workbench/api/common/extHostNotebookEditor.ts +++ b/src/vs/workbench/api/common/extHostNotebookEditor.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Emitter, Event } from 'vs/base/common/event'; -import { MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; +import { MainThreadNotebookEditorsShape } from 'vs/workbench/api/common/extHost.protocol'; import * as extHostTypes from 'vs/workbench/api/common/extHostTypes'; import * as extHostConverter from 'vs/workbench/api/common/extHostTypeConverters'; import { CellEditType, ICellEditOperation, ICellReplaceEdit } from 'vs/workbench/contrib/notebook/common/notebookCommon'; @@ -101,8 +101,7 @@ export class ExtHostNotebookEditor { constructor( readonly id: string, - private readonly _viewType: string, - private readonly _proxy: MainThreadNotebookShape, + private readonly _proxy: MainThreadNotebookEditorsShape, readonly notebookData: ExtHostNotebookDocument, visibleRanges: vscode.NotebookCellRange[], selections: vscode.NotebookCellRange[], @@ -217,7 +216,7 @@ export class ExtHostNotebookEditor { compressedEditsIndex++; } - return this._proxy.$tryApplyEdits(this._viewType, this.notebookData.uri, editData.documentVersionId, compressedEdits); + return this._proxy.$tryApplyEdits(this.id, editData.documentVersionId, compressedEdits); } setDecorations(decorationType: vscode.NotebookEditorDecorationType, range: vscode.NotebookCellRange): void { From 48e11a0cab41e22eba11609011ef38c020fb9d9c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 15:46:51 +0200 Subject: [PATCH 18/25] rename --- src/vs/workbench/api/browser/extensionHost.contribution.ts | 2 +- src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts | 2 +- ...ntsAndEditor.ts => mainThreadNotebookDocumentsAndEditors.ts} | 0 src/vs/workbench/api/browser/mainThreadNotebookEditors.ts | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename src/vs/workbench/api/browser/{mainThreadNotebookDocumentsAndEditor.ts => mainThreadNotebookDocumentsAndEditors.ts} (100%) diff --git a/src/vs/workbench/api/browser/extensionHost.contribution.ts b/src/vs/workbench/api/browser/extensionHost.contribution.ts index 42b7bfdbbb0..b2bce3f32b3 100644 --- a/src/vs/workbench/api/browser/extensionHost.contribution.ts +++ b/src/vs/workbench/api/browser/extensionHost.contribution.ts @@ -63,7 +63,7 @@ import './mainThreadWebviewManager'; import './mainThreadWorkspace'; import './mainThreadComments'; import './mainThreadNotebook'; -import './mainThreadNotebookDocumentsAndEditor'; +import './mainThreadNotebookDocumentsAndEditors'; import './mainThreadTask'; import './mainThreadLabelService'; import './mainThreadTunnelService'; diff --git a/src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts b/src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts index 1d98c60e7cf..0c3f796a44c 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts @@ -14,7 +14,7 @@ import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebo import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity'; import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, MainThreadNotebookDocumentsShape } from '../common/extHost.protocol'; -import { MainThreadNotebooksAndEditors } from 'vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor'; +import { MainThreadNotebooksAndEditors } from 'vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditors'; export class MainThreadNotebookDocuments implements MainThreadNotebookDocumentsShape { diff --git a/src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor.ts b/src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditors.ts similarity index 100% rename from src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor.ts rename to src/vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditors.ts diff --git a/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts b/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts index 9cb6973d3cd..dde9b82a2e1 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts @@ -7,7 +7,7 @@ import { DisposableStore, dispose } from 'vs/base/common/lifecycle'; import { getNotebookEditorFromEditorPane, INotebookEditor, NotebookEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookDocumentShowOptions, MainThreadNotebookEditorsShape, NotebookEditorRevealType } from '../common/extHost.protocol'; -import { MainThreadNotebooksAndEditors } from 'vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditor'; +import { MainThreadNotebooksAndEditors } from 'vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditors'; import { ICellEditOperation, ICellRange, INotebookDecorationRenderOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ILogService } from 'vs/platform/log/common/log'; import { URI, UriComponents } from 'vs/base/common/uri'; From c1209ae4e0194ac9aa589eab06c417d2116382fd Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 16:15:52 +0200 Subject: [PATCH 19/25] extract separte interfaces from ExtHostNotebookShape but not change anything yet --- src/vs/workbench/api/common/extHost.protocol.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index a5574bfec16..992abc1c03b 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1888,7 +1888,7 @@ export interface INotebookKernelInfoDto2 { implementsInterrupt?: boolean; } -export interface ExtHostNotebookShape { +export interface ExtHostNotebookShape extends ExtHostNotebookDocumentsAndEditorsShape, ExtHostNotebookDocumentsShape, ExtHostNotebookEditorsShape { $resolveNotebookEditor(viewType: string, uri: UriComponents, editorId: string): Promise; $acceptNotebookActiveKernelChange(event: { uri: UriComponents, providerHandle: number | undefined, kernelFriendlyId: string | undefined }): void; $provideNotebookKernels(handle: number, uri: UriComponents, token: CancellationToken): Promise; @@ -1904,12 +1904,20 @@ export interface ExtHostNotebookShape { $dataToNotebook(handle: number, data: VSBuffer): Promise; $notebookToData(handle: number, data: NotebookDataDto): Promise; +} +export interface ExtHostNotebookDocumentsAndEditorsShape { + $acceptDocumentAndEditorsDelta(delta: INotebookDocumentsAndEditorsDelta): void; +} + +export interface ExtHostNotebookDocumentsShape { $acceptModelChanged(uriComponents: UriComponents, event: NotebookCellsChangedEventDto, isDirty: boolean): void; $acceptDirtyStateChanged(uriComponents: UriComponents, isDirty: boolean): void; $acceptModelSaved(uriComponents: UriComponents): void; $acceptDocumentPropertiesChanged(uriComponents: UriComponents, data: INotebookDocumentPropertiesChangeData): void; - $acceptDocumentAndEditorsDelta(delta: INotebookDocumentsAndEditorsDelta): void; +} + +export interface ExtHostNotebookEditorsShape { $acceptEditorPropertiesChanged(id: string, data: INotebookEditorPropertiesChangeData): void; } From 1372233695c18c556c4f19919178a4c8c478d55c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 16:26:29 +0200 Subject: [PATCH 20/25] remove unused service --- src/vs/workbench/api/browser/mainThreadNotebook.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index d24e48acab3..c71dec330a5 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -9,7 +9,6 @@ import { Emitter } from 'vs/base/common/event'; import { IRelativePattern } from 'vs/base/common/glob'; import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; import { URI, UriComponents } from 'vs/base/common/uri'; -import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ILogService } from 'vs/platform/log/common/log'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; @@ -31,15 +30,12 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { constructor( extHostContext: IExtHostContext, - @IInstantiationService instantiationService: IInstantiationService, @INotebookService private readonly _notebookService: INotebookService, @INotebookEditorService private readonly _notebookEditorService: INotebookEditorService, @ILogService private readonly _logService: ILogService, @INotebookCellStatusBarService private readonly _cellStatusBarService: INotebookCellStatusBarService, ) { this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook); - - this._registerListeners(); } From e715199ccc7e2c1d4f6c32d955e895447127f912 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 16:49:21 +0200 Subject: [PATCH 21/25] extract createTestNotebookEditor to testing outside of with-util --- .../notebook/browser/notebookBrowser.ts | 1 + .../notebook/test/testNotebookEditor.ts | 22 +++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index a1be93c95d9..71d5228ad46 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -367,6 +367,7 @@ export interface INotebookEditor extends ICommonNotebookEditor { readonly onDidChangeKernel: Event; readonly onDidChangeActiveCell: Event; isDisposed: boolean; + dispose(): void; getId(): string; getDomNode(): HTMLElement; diff --git a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts index 831e16ec8dc..193732afc06 100644 --- a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts @@ -135,8 +135,8 @@ export function setupInstantiationService() { return instantiationService; } -export async function withTestNotebook(cells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][], callback: (editor: IActiveNotebookEditor, accessor: TestInstantiationService) => Promise | R): Promise { - const instantiationService = setupInstantiationService(); +function _createTestNotebookEditor(instantiationService: TestInstantiationService, cells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][]): IActiveNotebookEditor { + const viewType = 'notebook'; const notebook = instantiationService.createInstance(NotebookTextModel, viewType, URI.parse('test'), cells.map(cell => { return { @@ -157,6 +157,9 @@ export async function withTestNotebook(cells: [source: string, lang: st const listViewInfoAccessor = new ListViewInfoAccessor(cellList); const notebookEditor: IActiveNotebookEditor = new class extends mock() { + dispose() { + viewModel.dispose(); + } onDidChangeModel: Event = new Emitter().event; get viewModel() { return viewModel; } hasModel(): this is IActiveNotebookEditor { @@ -184,11 +187,22 @@ export async function withTestNotebook(cells: [source: string, lang: st } }; + return notebookEditor; +} + +export function createTestNotebookEditor(cells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][]): IActiveNotebookEditor { + return _createTestNotebookEditor(setupInstantiationService(), cells); +} + +export async function withTestNotebook(cells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][], callback: (editor: IActiveNotebookEditor, accessor: TestInstantiationService) => Promise | R): Promise { + const instantiationService = setupInstantiationService(); + const notebookEditor = _createTestNotebookEditor(instantiationService, cells); + const res = await callback(notebookEditor, instantiationService); if (res instanceof Promise) { - res.finally(() => viewModel.dispose()); + res.finally(() => notebookEditor.dispose()); } else { - viewModel.dispose(); + notebookEditor.dispose(); } return res; } From 407a0e372351a1dc5c1f312a8dfbebba31d3c1aa Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 17:51:15 +0200 Subject: [PATCH 22/25] assert that we have static view columns https://github.com/microsoft/vscode/issues/115704 --- .../src/singlefolder-tests/notebook.editor.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts index 9211b98fc1d..031101a0306 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.editor.test.ts @@ -65,6 +65,18 @@ suite('Notebook Editor', function () { }); + test('notebook editor has viewColumn', async function () { + + const uri1 = await utils.createRandomFile(undefined, undefined, '.nbdtest'); + const editor1 = await vscode.window.showNotebookDocument(uri1); + + assert.strictEqual(editor1.viewColumn, vscode.ViewColumn.One); + + const uri2 = await utils.createRandomFile(undefined, undefined, '.nbdtest'); + const editor2 = await vscode.window.showNotebookDocument(uri2, { viewColumn: vscode.ViewColumn.Beside }); + assert.strictEqual(editor2.viewColumn, vscode.ViewColumn.Two); + }); + test.skip('Opening a notebook should fire activeNotebook event changed only once', async function () { const openedEditor = utils.asPromise(vscode.window.onDidChangeActiveNotebookEditor); const resource = await utils.createRandomFile(undefined, undefined, '.nbdtest'); From ca980ecfac391affbd445b905eaa6222c71d60e3 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 31 Mar 2021 18:09:39 +0200 Subject: [PATCH 23/25] update notebook editor view column when moving editors, https://github.com/microsoft/vscode/issues/115704 --- .../api/browser/mainThreadNotebookEditors.ts | 38 +++++++++++++++---- .../workbench/api/common/extHost.protocol.ts | 3 ++ .../workbench/api/common/extHostNotebook.ts | 12 +++++- .../api/common/extHostNotebookEditor.ts | 4 ++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts b/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts index dde9b82a2e1..80cbf500817 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookEditors.ts @@ -6,7 +6,7 @@ import { DisposableStore, dispose } from 'vs/base/common/lifecycle'; import { getNotebookEditorFromEditorPane, INotebookEditor, NotebookEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; -import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookDocumentShowOptions, MainThreadNotebookEditorsShape, NotebookEditorRevealType } from '../common/extHost.protocol'; +import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookDocumentShowOptions, INotebookEditorViewColumnInfo, MainThreadNotebookEditorsShape, NotebookEditorRevealType } from '../common/extHost.protocol'; import { MainThreadNotebooksAndEditors } from 'vs/workbench/api/browser/mainThreadNotebookDocumentsAndEditors'; import { ICellEditOperation, ICellRange, INotebookDecorationRenderOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ILogService } from 'vs/platform/log/common/log'; @@ -15,6 +15,9 @@ import { EditorActivation, EditorOverride } from 'vs/platform/editor/common/edit import { NotebookEditorInput } from 'vs/workbench/contrib/notebook/common/notebookEditorInput'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; +import { editorGroupToViewColumn } from 'vs/workbench/common/editor'; +import { equals } from 'vs/base/common/objects'; class MainThreadEditor { constructor( @@ -31,7 +34,9 @@ export class MainThreadNotebookEditors implements MainThreadNotebookEditorsShape private readonly _disposables = new DisposableStore(); private readonly _proxy: ExtHostNotebookShape; - private readonly _editorEventListenersMapping = new Map(); + private readonly _mainThreadEditors = new Map(); + + private _currentViewColumnInfo?: INotebookEditorViewColumnInfo; constructor( extHostContext: IExtHostContext, @@ -40,16 +45,21 @@ export class MainThreadNotebookEditors implements MainThreadNotebookEditorsShape @IEditorService private readonly _editorService: IEditorService, @ILogService private readonly _logService: ILogService, @INotebookEditorService private readonly _notebookEditorService: INotebookEditorService, + @IEditorGroupsService private readonly _editorGroupService: IEditorGroupsService ) { this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook); notebooksAndEditors.onDidAddEditors(this._handleEditorsAdded, this, this._disposables); notebooksAndEditors.onDidRemoveEditors(this._handleEditorsRemoved, this, this._disposables); + + this._editorService.onDidActiveEditorChange(() => this._updateEditorViewColumns(), this, this._disposables); + this._editorGroupService.onDidRemoveGroup(() => this._updateEditorViewColumns(), this, this._disposables); + this._editorGroupService.onDidMoveGroup(() => this._updateEditorViewColumns(), this, this._disposables); } dispose(): void { this._disposables.dispose(); - dispose(this._editorEventListenersMapping.values()); + dispose(this._mainThreadEditors.values()); } private _handleEditorsAdded(editors: readonly INotebookEditor[]): void { @@ -76,19 +86,33 @@ export class MainThreadNotebookEditors implements MainThreadNotebookEditorsShape }); })); - this._editorEventListenersMapping.set(editor.getId(), new MainThreadEditor(editor, editorDisposables)); + this._mainThreadEditors.set(editor.getId(), new MainThreadEditor(editor, editorDisposables)); } } private _handleEditorsRemoved(editorIds: readonly string[]): void { for (const id of editorIds) { - this._editorEventListenersMapping.get(id)?.dispose(); - this._editorEventListenersMapping.delete(id); + this._mainThreadEditors.get(id)?.dispose(); + this._mainThreadEditors.delete(id); + } + } + + private _updateEditorViewColumns(): void { + const result: INotebookEditorViewColumnInfo = Object.create(null); + for (let editorPane of this._editorService.visibleEditorPanes) { + const candidate = getNotebookEditorFromEditorPane(editorPane); + if (candidate && this._mainThreadEditors.has(candidate.getId())) { + result[candidate.getId()] = editorGroupToViewColumn(this._editorGroupService, editorPane.group); + } + } + if (!equals(result, this._currentViewColumnInfo)) { + this._currentViewColumnInfo = result; + this._proxy.$acceptEditorViewColumns(result); } } async $tryApplyEdits(editorId: string, modelVersionId: number, cellEdits: ICellEditOperation[]): Promise { - const wrapper = this._editorEventListenersMapping.get(editorId); + const wrapper = this._mainThreadEditors.get(editorId); if (!wrapper) { return false; } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 992abc1c03b..541891d7cdd 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1917,8 +1917,11 @@ export interface ExtHostNotebookDocumentsShape { $acceptDocumentPropertiesChanged(uriComponents: UriComponents, data: INotebookDocumentPropertiesChangeData): void; } +export type INotebookEditorViewColumnInfo = Record; + export interface ExtHostNotebookEditorsShape { $acceptEditorPropertiesChanged(id: string, data: INotebookEditorPropertiesChangeData): void; + $acceptEditorViewColumns(data: INotebookEditorViewColumnInfo): void; } export interface ExtHostStorageShape { diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 94ee6b2a4e3..2b46067949c 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -9,7 +9,7 @@ import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/ import { URI, UriComponents } from 'vs/base/common/uri'; import * as UUID from 'vs/base/common/uuid'; import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'; -import { ExtHostNotebookShape, ICommandDto, IMainContext, IModelAddedData, INotebookDocumentPropertiesChangeData, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, INotebookEditorPropertiesChangeData, INotebookKernelInfoDto2, MainContext, MainThreadNotebookDocumentsShape, MainThreadNotebookEditorsShape, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; +import { ExtHostNotebookShape, ICommandDto, IMainContext, IModelAddedData, INotebookDocumentPropertiesChangeData, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, INotebookEditorPropertiesChangeData, INotebookEditorViewColumnInfo, INotebookKernelInfoDto2, MainContext, MainThreadNotebookDocumentsShape, MainThreadNotebookEditorsShape, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; import { ILogService } from 'vs/platform/log/common/log'; import { CommandsConverter, ExtHostCommands } from 'vs/workbench/api/common/extHostCommands'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; @@ -649,6 +649,16 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { } } + $acceptEditorViewColumns(data: INotebookEditorViewColumnInfo): void { + for (const id in data) { + const editor = this._editors.get(id); + if (!editor) { + throw new Error(`unknown text editor: ${id}. known editors: ${[...this._editors.keys()]} `); + } + editor.editor._acceptViewColumn(typeConverters.ViewColumn.to(data[id])); + } + } + $acceptDocumentPropertiesChanged(uri: UriComponents, data: INotebookDocumentPropertiesChangeData): void { this.logService.debug('ExtHostNotebook#$acceptDocumentPropertiesChanged', uri.path, data); const document = this._getNotebookDocument(URI.revive(uri)); diff --git a/src/vs/workbench/api/common/extHostNotebookEditor.ts b/src/vs/workbench/api/common/extHostNotebookEditor.ts index 9d57ba51ee1..0430d67ca7b 100644 --- a/src/vs/workbench/api/common/extHostNotebookEditor.ts +++ b/src/vs/workbench/api/common/extHostNotebookEditor.ts @@ -183,6 +183,10 @@ export class ExtHostNotebookEditor { this._selections = selections; } + _acceptViewColumn(value: vscode.ViewColumn | undefined) { + this._viewColumn = value; + } + private _applyEdit(editData: INotebookEditData): Promise { // return when there is nothing to do From f1c4a8676e56896fd44c02b74d7513d1e9edadff Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 31 Mar 2021 11:25:00 -0700 Subject: [PATCH 24/25] fix #119205. --- src/vs/base/browser/ui/list/listView.ts | 1 + .../contrib/notebook/browser/notebookBrowser.ts | 1 + .../contrib/notebook/browser/notebookEditorWidget.ts | 11 +++++++++-- .../contrib/notebook/browser/view/notebookCellList.ts | 4 ++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index b773496b257..0ebc24f9ff7 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -362,6 +362,7 @@ export class ListView implements ISpliceable, IDisposable { updateOptions(options: IListViewOptionsUpdate) { if (options.additionalScrollHeight !== undefined) { this.additionalScrollHeight = options.additionalScrollHeight; + this.scrollableElement.setScrollDimensions({ scrollHeight: this.scrollHeight }); } if (options.smoothScrolling !== undefined) { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index 71d5228ad46..b71a45a6bd1 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -694,6 +694,7 @@ export interface INotebookCellList { domFocus(): void; setCellSelection(element: ICellViewModel, range: Range): void; style(styles: IListStyles): void; + getRenderHeight(): number; updateOptions(options: IListOptions): void; layout(height?: number, width?: number): void; dispose(): void; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 78f37cf41b6..9cd4d693311 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -1428,8 +1428,15 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor this._dimension = new DOM.Dimension(dimension.width, dimension.height); DOM.size(this._body, dimension.width, dimension.height - (this._useGlobalToolbar ? /** Toolbar height */ 26 : 0)); - this._list.updateOptions({ additionalScrollHeight: this._scrollBeyondLastLine ? dimension.height - SCROLLABLE_ELEMENT_PADDING_TOP : SCROLLABLE_ELEMENT_PADDING_TOP }); - this._list.layout(dimension.height - SCROLLABLE_ELEMENT_PADDING_TOP, dimension.width); + if (this._list.getRenderHeight() < dimension.height - SCROLLABLE_ELEMENT_PADDING_TOP) { + // the new dimension is larger than the list viewport, update its additional height first, otherwise the list view will move down a bit (as the `scrollBottom` will move down) + this._list.updateOptions({ additionalScrollHeight: this._scrollBeyondLastLine ? Math.max(0, (dimension.height - SCROLLABLE_ELEMENT_PADDING_TOP - 50)) : SCROLLABLE_ELEMENT_PADDING_TOP }); + this._list.layout(dimension.height - SCROLLABLE_ELEMENT_PADDING_TOP, dimension.width); + } else { + // the new dimension is smaller than the list viewport, if we update the additional height, the `scrollBottom` will move up, which moves the whole list view upwards a bit. So we run a layout first. + this._list.layout(dimension.height - SCROLLABLE_ELEMENT_PADDING_TOP, dimension.width); + this._list.updateOptions({ additionalScrollHeight: this._scrollBeyondLastLine ? Math.max(0, (dimension.height - SCROLLABLE_ELEMENT_PADDING_TOP - 50)) : SCROLLABLE_ELEMENT_PADDING_TOP }); + } this._overlayContainer.style.visibility = 'visible'; this._overlayContainer.style.display = 'block'; diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index ace3194125e..e79bde040bf 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -1246,6 +1246,10 @@ export class NotebookCellList extends WorkbenchList implements ID } } + getRenderHeight() { + return this.view.renderHeight; + } + layout(height?: number, width?: number): void { this._isInLayout = true; super.layout(height, width); From f35264bfc61511e95c3bb23455b4bef8a5a18b08 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 31 Mar 2021 11:33:54 -0700 Subject: [PATCH 25/25] fetch focus from view model other than list. --- .../notebook/browser/notebookBrowser.ts | 3 --- .../notebook/browser/notebookEditorWidget.ts | 18 +++++++++--------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index b71a45a6bd1..0d02f8abe6c 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -698,9 +698,6 @@ export interface INotebookCellList { updateOptions(options: IListOptions): void; layout(height?: number, width?: number): void; dispose(): void; - - // TODO@roblourens resolve differences between List and INotebookCellList - getFocus(): number[]; } export interface BaseCellRenderTemplate { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 9cd4d693311..ab8f968feaf 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -1384,15 +1384,15 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor state.cellTotalHeights = cellHeights; - const focus = this._list.getFocus()[0]; - if (typeof focus === 'number' && this.viewModel) { - const element = this.viewModel.viewCells[focus]; + if (this.viewModel) { + const focusRange = this.viewModel.getFocus(); + const element = this.viewModel.viewCells[focusRange.start]; if (element) { const itemDOM = this._list.domElementOfElement(element); const editorFocused = element.editState === CellEditState.Editing && !!(document.activeElement && itemDOM && itemDOM.contains(document.activeElement)); state.editorFocused = editorFocused; - state.focus = focus; + state.focus = focusRange.start; } } } @@ -1465,18 +1465,18 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor if (this._webiewFocused) { this._webview?.focusWebview(); } else { - const focus = this._list.getFocus()[0]; - if (typeof focus === 'number' && this.viewModel) { - const element = this.viewModel.viewCells[focus]; + if (this.viewModel) { + const focusRange = this.viewModel.getFocus(); + const element = this.viewModel.viewCells[focusRange.start]; - if (element.focusMode === CellFocusMode.Editor) { + if (element && element.focusMode === CellFocusMode.Editor) { element.editState = CellEditState.Editing; element.focusMode = CellFocusMode.Editor; this._onDidFocusEditorWidget.fire(); return; } - } + this._list.domFocus(); }