From da16f9bef171b8b92e5da1493e2d7a572ddc6593 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Thu, 15 Apr 2021 15:59:46 -0700 Subject: [PATCH 01/18] Move cell language picker to statusbar API --- .../notebook/browser/contrib/coreActions.ts | 8 +-- .../contrib/statusBar/cellStatusBar.ts | 17 ++++- .../contrib/statusBar/statusBarProviders.ts | 43 ++++++++++-- .../notebook/browser/notebookBrowser.ts | 1 + .../notebookCellStatusBarServiceImpl.ts | 4 +- .../browser/view/renderers/cellWidgets.ts | 67 +------------------ .../contrib/notebook/common/notebookCommon.ts | 2 +- 7 files changed, 64 insertions(+), 78 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index 1630f25609bcb..01720585afaaf 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -18,7 +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 { 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 { 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, CHANGE_CELL_LANGUAGE } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellExecutionState, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, TransientMetadata, SelectionStateType, ICellReplaceEdit } 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'; @@ -60,7 +60,6 @@ const CANCEL_CELL_COMMAND_ID = 'notebook.cell.cancelExecution'; const EXECUTE_CELL_SELECT_BELOW = 'notebook.cell.executeAndSelectBelow'; const EXECUTE_CELL_INSERT_BELOW = 'notebook.cell.executeAndInsertBelow'; const CLEAR_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.clearOutputs'; -const CHANGE_CELL_LANGUAGE = 'notebook.cell.changeLanguage'; const CENTER_ACTIVE_CELL = 'notebook.centerActiveCell'; const FOCUS_IN_OUTPUT_COMMAND_ID = 'notebook.cell.focusInOutput'; @@ -1339,7 +1338,7 @@ interface IChangeCellContext extends INotebookCellActionContext { language?: string; } -export class ChangeCellLanguageAction extends NotebookCellAction { +registerAction2(class ChangeCellLanguageAction extends NotebookCellAction { constructor() { super({ id: CHANGE_CELL_LANGUAGE, @@ -1495,8 +1494,7 @@ export class ChangeCellLanguageAction extends NotebookCellAction { return fakeResource; } -} -registerAction2(ChangeCellLanguageAction); +}); registerAction2(class extends NotebookAction { constructor() { diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/cellStatusBar.ts b/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/cellStatusBar.ts index 13911ac11947a..4e03ac9d920c1 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/cellStatusBar.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/cellStatusBar.ts @@ -6,7 +6,7 @@ import { flatten } from 'vs/base/common/arrays'; import { Throttler } from 'vs/base/common/async'; import { CancellationTokenSource } from 'vs/base/common/cancellation'; -import { Disposable, toDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle'; import { ICellViewModel, INotebookEditor, INotebookEditorContribution } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { registerNotebookContribution } from 'vs/workbench/contrib/notebook/browser/notebookEditorExtensions'; import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; @@ -18,6 +18,8 @@ export class NotebookStatusBarController extends Disposable implements INotebook private readonly _visibleCells = new Map(); + private readonly _viewModelDisposables = new DisposableStore(); + constructor( private readonly _notebookEditor: INotebookEditor, @INotebookCellStatusBarService private readonly _notebookCellStatusBarService: INotebookCellStatusBarService @@ -25,11 +27,22 @@ export class NotebookStatusBarController extends Disposable implements INotebook super(); this._updateVisibleCells(); this._register(this._notebookEditor.onDidChangeVisibleRanges(this._updateVisibleCells, this)); - this._register(this._notebookEditor.onDidChangeModel(this._updateEverything, this)); + this._register(this._notebookEditor.onDidChangeModel(this._onModelChange, this)); this._register(this._notebookCellStatusBarService.onDidChangeProviders(this._updateEverything, this)); this._register(this._notebookCellStatusBarService.onDidChangeItems(this._updateEverything, this)); } + private _onModelChange() { + this._viewModelDisposables.clear(); + const vm = this._notebookEditor.viewModel; + if (!vm) { + return; + } + + this._viewModelDisposables.add(vm.onDidChangeViewCells(() => this._updateEverything())); + this._updateEverything(); + } + private _updateEverything(): void { this._visibleCells.forEach(cell => cell.dispose()); this._visibleCells.clear(); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts b/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts index 0429bd8d2afdb..e6721777ba5b8 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts @@ -3,8 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { localize } from 'vs/nls'; import { CancellationToken } from 'vs/base/common/cancellation'; -import { Event } from 'vs/base/common/event'; import { Disposable } from 'vs/base/common/lifecycle'; import { isWindows } from 'vs/base/common/platform'; import { URI } from 'vs/base/common/uri'; @@ -12,9 +12,11 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions as WorkbenchExtensions, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions'; import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService'; -import { CellStatusbarAlignment, INotebookCellStatusBarItem, INotebookCellStatusBarItemList, INotebookCellStatusBarItemProvider, INotebookDocumentFilter } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, CellStatusbarAlignment, INotebookCellStatusBarItem, INotebookCellStatusBarItemList, INotebookCellStatusBarItemProvider, INotebookDocumentFilter } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; +import { IModeService } from 'vs/editor/common/services/modeService'; +import { CHANGE_CELL_LANGUAGE } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; class CellStatusBarPlaceholderProvider implements INotebookCellStatusBarItemProvider { readonly selector: INotebookDocumentFilter = { @@ -23,8 +25,6 @@ class CellStatusBarPlaceholderProvider implements INotebookCellStatusBarItemProv constructor(@INotebookService private readonly _notebookService: INotebookService) { } - onDidChangeStatusBarItems?: Event | undefined; - async provideCellStatusBarItems(uri: URI, index: number, token: CancellationToken): Promise { const doc = this._notebookService.getNotebookTextModel(uri); const cell = doc?.cells[index]; @@ -46,12 +46,47 @@ class CellStatusBarPlaceholderProvider implements INotebookCellStatusBarItemProv } } +class CellStatusBarLanguagePickerProvider implements INotebookCellStatusBarItemProvider { + readonly selector: INotebookDocumentFilter = { + filenamePattern: '**/*' + }; + + constructor( + @INotebookService private readonly _notebookService: INotebookService, + @IModeService private readonly _modeService: IModeService, + ) { } + + async provideCellStatusBarItems(uri: URI, index: number, _token: CancellationToken): Promise { + const doc = this._notebookService.getNotebookTextModel(uri); + const cell = doc?.cells[index]; + if (!cell) { + return; + } + + const modeId = cell.cellKind === CellKind.Markdown ? + 'markdown' : + (this._modeService.getModeIdForLanguageName(cell.language) || cell.language); + const text = this._modeService.getLanguageName(modeId) || this._modeService.getLanguageName('plaintext'); + const item = { + text, + command: CHANGE_CELL_LANGUAGE, + tooltip: localize('notebook.cell.status.language', "Select Cell Language Mode"), + alignment: CellStatusbarAlignment.Right, + priority: -Number.MAX_SAFE_INTEGER + }; + return { + items: [item] + }; + } +} + class BuiltinCellStatusBarProviders extends Disposable { constructor( @IInstantiationService instantiationService: IInstantiationService, @INotebookCellStatusBarService notebookCellStatusBarService: INotebookCellStatusBarService) { super(); this._register(notebookCellStatusBarService.registerCellStatusBarItemProvider(instantiationService.createInstance(CellStatusBarPlaceholderProvider))); + this._register(notebookCellStatusBarService.registerCellStatusBarItemProvider(instantiationService.createInstance(CellStatusBarLanguagePickerProvider))); } } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index b022a7314f10e..f9d10730d6a09 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -72,6 +72,7 @@ export const NOTEBOOK_INTERRUPTIBLE_KERNEL = new RawContextKey('noteboo //#region Shared commands export const EXPAND_CELL_INPUT_COMMAND_ID = 'notebook.cell.expandCellInput'; export const EXECUTE_CELL_COMMAND_ID = 'notebook.cell.execute'; +export const CHANGE_CELL_LANGUAGE = 'notebook.cell.changeLanguage'; //#endregion diff --git a/src/vs/workbench/contrib/notebook/browser/notebookCellStatusBarServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookCellStatusBarServiceImpl.ts index 35b74591b6a36..01060a99f747f 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookCellStatusBarServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookCellStatusBarServiceImpl.ts @@ -43,9 +43,9 @@ export class NotebookCellStatusBarService extends Disposable implements INoteboo async getStatusBarItemsForCell(docUri: URI, cellIndex: number, viewType: string, token: CancellationToken): Promise { const providers = this._providers.filter(p => notebookDocumentFilterMatch(p.selector, viewType, docUri)); - return await Promise.all(providers.map(p => { + return await Promise.all(providers.map(async p => { try { - return p.provideCellStatusBarItems(docUri, cellIndex, token); + return await p.provideCellStatusBarItems(docUri, cellIndex, token) ?? { items: [] }; } catch (e) { onUnexpectedExternalError(e); return { items: [] }; diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellWidgets.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellWidgets.ts index b82b758f9bfda..8598cac1a980b 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellWidgets.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellWidgets.ts @@ -7,22 +7,19 @@ import * as DOM from 'vs/base/browser/dom'; import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { SimpleIconLabel } from 'vs/base/browser/ui/iconLabel/simpleIconLabel'; import { WorkbenchActionExecutedClassification, WorkbenchActionExecutedEvent } from 'vs/base/common/actions'; -import { stripIcons } from 'vs/base/common/iconLabels'; import { toErrorMessage } from 'vs/base/common/errorMessage'; import { Emitter, Event } from 'vs/base/common/event'; +import { stripIcons } from 'vs/base/common/iconLabels'; import { KeyCode } from 'vs/base/common/keyCodes'; import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import { ElementSizeObserver } from 'vs/editor/browser/config/elementSizeObserver'; import { IDimension } from 'vs/editor/common/editorCommon'; -import { IModeService } from 'vs/editor/common/services/modeService'; -import { localize } from 'vs/nls'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; -import { ChangeCellLanguageAction, INotebookCellActionContext } from 'vs/workbench/contrib/notebook/browser/contrib/coreActions'; -import { ICellViewModel, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { CellKind, CellStatusbarAlignment, INotebookCellStatusBarItem } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookCellActionContext } from 'vs/workbench/contrib/notebook/browser/contrib/coreActions'; +import { CellStatusbarAlignment, INotebookCellStatusBarItem } from 'vs/workbench/contrib/notebook/common/notebookCommon'; const $ = DOM.$; @@ -41,7 +38,6 @@ export const enum ClickTargetType { export class CellEditorStatusBar extends Disposable { readonly cellRunStatusContainer: HTMLElement; readonly statusBarContainer: HTMLElement; - readonly languageStatusBarItem: CellLanguageStatusBarItem; readonly durationContainer: HTMLElement; private readonly leftContributedItemsContainer: HTMLElement; @@ -65,7 +61,6 @@ export class CellEditorStatusBar extends Disposable { this.durationContainer = DOM.append(leftItemsContainer, $('.cell-run-duration')); this.leftContributedItemsContainer = DOM.append(leftItemsContainer, $('.cell-contributed-items.cell-contributed-items-left')); this.rightContributedItemsContainer = DOM.append(rightItemsContainer, $('.cell-contributed-items.cell-contributed-items-right')); - this.languageStatusBarItem = instantiationService.createInstance(CellLanguageStatusBarItem, rightItemsContainer); this.itemsDisposable = this._register(new DisposableStore()); @@ -104,7 +99,6 @@ export class CellEditorStatusBar extends Disposable { update(context: INotebookCellActionContext) { this.currentContext = context; this.itemsDisposable.clear(); - this.languageStatusBarItem.update(context.cell, context.notebookEditor); this.updateStatusBarItems(); } @@ -224,61 +218,6 @@ class CellStatusBarItem extends Disposable { } } -export class CellLanguageStatusBarItem extends Disposable { - private readonly labelElement: HTMLElement; - - private cell: ICellViewModel | undefined; - private editor: INotebookEditor | undefined; - - private cellDisposables: DisposableStore; - - constructor( - readonly container: HTMLElement, - @IModeService private readonly modeService: IModeService, - @IInstantiationService private readonly instantiationService: IInstantiationService - ) { - super(); - this.labelElement = DOM.append(container, $('.cell-language-picker.cell-status-item')); - this.labelElement.tabIndex = 0; - this.labelElement.classList.add('cell-status-item-has-command'); - - this._register(DOM.addDisposableListener(this.labelElement, DOM.EventType.CLICK, () => { - this.run(); - })); - this._register(DOM.addDisposableListener(this.labelElement, DOM.EventType.KEY_DOWN, e => { - const event = new StandardKeyboardEvent(e); - if (event.equals(KeyCode.Space) || event.equals(KeyCode.Enter)) { - this.run(); - } - })); - this._register(this.cellDisposables = new DisposableStore()); - } - - private run() { - this.instantiationService.invokeFunction(accessor => { - if (!this.editor || !this.editor.hasModel() || !this.cell) { - return; - } - new ChangeCellLanguageAction().run(accessor, { notebookEditor: this.editor, cell: this.cell }); - }); - } - - update(cell: ICellViewModel, editor: INotebookEditor): void { - this.cellDisposables.clear(); - this.cell = cell; - this.editor = editor; - - this.render(); - this.cellDisposables.add(this.cell.model.onDidChangeLanguage(() => this.render())); - } - - private render(): void { - const modeId = this.cell?.cellKind === CellKind.Markdown ? 'markdown' : this.modeService.getModeIdForLanguageName(this.cell!.language) || this.cell!.language; - this.labelElement.textContent = this.modeService.getLanguageName(modeId) || this.modeService.getLanguageName('plaintext'); - this.labelElement.title = localize('notebook.cell.status.language', "Select Cell Language Mode"); - } -} - declare const ResizeObserver: any; export interface IResizeObserver { diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 0bf4ecb9c4486..aab2b90d11388 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -793,7 +793,7 @@ export interface INotebookKernelProvider { export interface INotebookCellStatusBarItemProvider { selector: INotebookDocumentFilter; onDidChangeStatusBarItems?: Event; - provideCellStatusBarItems(uri: URI, index: number, token: CancellationToken): Promise; + provideCellStatusBarItems(uri: URI, index: number, token: CancellationToken): Promise; } export class CellSequence implements ISequence { From e7004428b7e0ffb90725c1e5ced624d9544fe773 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 15 Apr 2021 16:45:03 -0700 Subject: [PATCH 02/18] update todos. --- src/vs/vscode.proposed.d.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 2fcc5cf1f20d9..17cbd45435732 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1208,13 +1208,11 @@ declare module 'vscode' { // todo@API support ids https://github.com/jupyter/enhancement-proposals/blob/master/62-cell-id/cell-id.md export class NotebookCellData { - // todo@API should they all be readonly? kind: NotebookCellKind; // todo@API better names: value? text? source: string; // todo@API how does language and MD relate? language: string; - // todo@API ReadonlyArray? outputs?: NotebookCellOutput[]; metadata?: NotebookCellMetadata; latestExecutionSummary?: NotebookCellExecutionSummary; @@ -1222,7 +1220,6 @@ declare module 'vscode' { } export class NotebookData { - // todo@API should they all be readonly? cells: NotebookCellData[]; metadata: NotebookDocumentMetadata; constructor(cells: NotebookCellData[], metadata?: NotebookDocumentMetadata); From 7193b08f344eed3e0e8f68a5d3696c99bd19eee9 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Thu, 15 Apr 2021 16:03:11 -0700 Subject: [PATCH 03/18] Remove old cell language picker item CSS --- .../contrib/notebook/browser/media/notebook.css | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/media/notebook.css b/src/vs/workbench/contrib/notebook/browser/media/notebook.css index 278cabbc997ad..5be823e87a19e 100644 --- a/src/vs/workbench/contrib/notebook/browser/media/notebook.css +++ b/src/vs/workbench/contrib/notebook/browser/media/notebook.css @@ -448,24 +448,15 @@ cursor: pointer; } -.monaco-workbench .notebookOverlay .cell-statusbar-container .cell-language-picker { - cursor: pointer; -} - .monaco-workbench .notebookOverlay .cell-statusbar-container .cell-run-duration { margin-right: 8px; } -.monaco-workbench .notebookOverlay .cell-statusbar-container .cell-run-duration, -.monaco-workbench .notebookOverlay .cell-statusbar-container .cell-status-message { +.monaco-workbench .notebookOverlay .cell-statusbar-container .cell-run-duration { display: flex; align-items: center; } -.monaco-workbench .notebookOverlay .cell-statusbar-container .cell-status-message { - margin-right: 6px; -} - .monaco-workbench .notebookOverlay .cell-statusbar-container .cell-run-status { height: 100%; display: flex; From a7c67442b97a573f99cdda74bad86f8ac199fc4a Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Thu, 15 Apr 2021 16:57:52 -0700 Subject: [PATCH 04/18] Clean up cell statusbar keybinding tip placeholder --- .../notebook/browser/contrib/coreActions.ts | 3 +- .../contrib/statusBar/statusBarProviders.ts | 33 +++++++++++++++---- .../notebook/browser/media/notebook.css | 1 - .../notebook/browser/notebookBrowser.ts | 1 + 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index 01720585afaaf..824b8e53b4841 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -18,7 +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 { 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, CHANGE_CELL_LANGUAGE } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +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, CHANGE_CELL_LANGUAGE, QUIT_EDIT_CELL_COMMAND_ID } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellExecutionState, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, TransientMetadata, SelectionStateType, ICellReplaceEdit } 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'; @@ -53,7 +53,6 @@ const CHANGE_CELL_TO_CODE_COMMAND_ID = 'notebook.cell.changeToCode'; const CHANGE_CELL_TO_MARKDOWN_COMMAND_ID = 'notebook.cell.changeToMarkdown'; const EDIT_CELL_COMMAND_ID = 'notebook.cell.edit'; -const QUIT_EDIT_CELL_COMMAND_ID = 'notebook.cell.quitEdit'; const DELETE_CELL_COMMAND_ID = 'notebook.cell.delete'; const CANCEL_CELL_COMMAND_ID = 'notebook.cell.cancelExecution'; diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts b/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts index e6721777ba5b8..f88cb921fdc60 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/statusBar/statusBarProviders.ts @@ -6,7 +6,6 @@ import { localize } from 'vs/nls'; import { CancellationToken } from 'vs/base/common/cancellation'; import { Disposable } from 'vs/base/common/lifecycle'; -import { isWindows } from 'vs/base/common/platform'; import { URI } from 'vs/base/common/uri'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { Registry } from 'vs/platform/registry/common/platform'; @@ -16,23 +15,43 @@ import { CellKind, CellStatusbarAlignment, INotebookCellStatusBarItem, INotebook import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { IModeService } from 'vs/editor/common/services/modeService'; -import { CHANGE_CELL_LANGUAGE } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { CHANGE_CELL_LANGUAGE, EXECUTE_CELL_COMMAND_ID, QUIT_EDIT_CELL_COMMAND_ID } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; class CellStatusBarPlaceholderProvider implements INotebookCellStatusBarItemProvider { readonly selector: INotebookDocumentFilter = { filenamePattern: '**/*' }; - constructor(@INotebookService private readonly _notebookService: INotebookService) { } + constructor( + @INotebookService private readonly _notebookService: INotebookService, + @IKeybindingService private readonly _keybindingService: IKeybindingService, + ) { } - async provideCellStatusBarItems(uri: URI, index: number, token: CancellationToken): Promise { + async provideCellStatusBarItems(uri: URI, index: number, token: CancellationToken): Promise { const doc = this._notebookService.getNotebookTextModel(uri); const cell = doc?.cells[index]; - if (typeof cell?.metadata.runState !== 'undefined' || typeof cell?.metadata.lastRunSuccess !== 'undefined') { - return { items: [] }; + if (!cell || typeof cell.metadata.runState !== 'undefined' || typeof cell.metadata.lastRunSuccess !== 'undefined') { + return; + } + + let text: string; + if (cell.cellKind === CellKind.Code) { + const keybinding = this._keybindingService.lookupKeybinding(EXECUTE_CELL_COMMAND_ID)?.getLabel(); + if (!keybinding) { + return; + } + + text = localize('notebook.cell.status.codeExecuteTip', "Press {0} to execute cell", keybinding); + } else { + const keybinding = this._keybindingService.lookupKeybinding(QUIT_EDIT_CELL_COMMAND_ID)?.getLabel(); + if (!keybinding) { + return; + } + + text = localize('notebook.cell.status.markdownExecuteTip', "Press {0} to stop editing", keybinding); } - const text = isWindows ? 'Ctrl + Alt + Enter to run' : 'Ctrl + Enter to run'; const item = { text, tooltip: text, diff --git a/src/vs/workbench/contrib/notebook/browser/media/notebook.css b/src/vs/workbench/contrib/notebook/browser/media/notebook.css index 5be823e87a19e..08f00a78366e5 100644 --- a/src/vs/workbench/contrib/notebook/browser/media/notebook.css +++ b/src/vs/workbench/contrib/notebook/browser/media/notebook.css @@ -420,7 +420,6 @@ .monaco-workbench .notebookOverlay .cell-statusbar-container .cell-status-left, .monaco-workbench .notebookOverlay .cell-statusbar-container .cell-status-right { - padding-right: 12px; display: flex; z-index: 26; } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index f9d10730d6a09..635346a0f0995 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -73,6 +73,7 @@ export const NOTEBOOK_INTERRUPTIBLE_KERNEL = new RawContextKey('noteboo export const EXPAND_CELL_INPUT_COMMAND_ID = 'notebook.cell.expandCellInput'; export const EXECUTE_CELL_COMMAND_ID = 'notebook.cell.execute'; export const CHANGE_CELL_LANGUAGE = 'notebook.cell.changeLanguage'; +export const QUIT_EDIT_CELL_COMMAND_ID = 'notebook.cell.quitEdit'; //#endregion From 881a304efe95bc9eef2cb2d135cdd7cfed773a33 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Thu, 15 Apr 2021 17:00:01 -0700 Subject: [PATCH 05/18] Add some NotebookCellStatusBarItemProvider jsdoc --- src/vs/vscode.proposed.d.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 17cbd45435732..2f25fa41fe05b 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1746,7 +1746,14 @@ declare module 'vscode' { } interface NotebookCellStatusBarItemProvider { + /** + * Implement and fire this event to signal that statusbar items have changed. The provide method will be called again. + */ onDidChangeCellStatusBarItems?: Event; + + /** + * The provider will be called when the cell scrolls into view, when its content, outputs, language, or metadata change, and when it changes execution state. + */ provideCellStatusBarItems(cell: NotebookCell, token: CancellationToken): ProviderResult; } From 8629b48030ea827e176c9f735f67f296d2cd0462 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 15 Apr 2021 17:19:39 -0700 Subject: [PATCH 06/18] rename transientMetadata to transientCellMetadata. --- src/vs/vscode.proposed.d.ts | 15 +++++++++++++-- .../workbench/api/browser/mainThreadNotebook.ts | 12 +++++++----- src/vs/workbench/api/common/extHost.protocol.ts | 7 ++++--- src/vs/workbench/api/common/extHostApiCommands.ts | 11 ++++++++--- src/vs/workbench/api/common/extHostNotebook.ts | 3 ++- .../workbench/api/common/extHostTypeConverters.ts | 7 ++++--- .../notebook/browser/contrib/coreActions.ts | 10 +++++++--- .../notebook/browser/diff/diffElementViewModel.ts | 4 ++-- .../common/model/notebookCellTextModel.ts | 4 ++-- .../notebook/common/model/notebookTextModel.ts | 6 +++--- .../contrib/notebook/common/notebookCommon.ts | 6 ++++-- .../contrib/notebook/common/notebookProvider.ts | 3 ++- .../notebook/test/notebookViewModel.test.ts | 2 +- .../contrib/notebook/test/testNotebookEditor.ts | 4 ++-- 14 files changed, 61 insertions(+), 33 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 2f25fa41fe05b..3dd87ac446e9d 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1035,10 +1035,21 @@ declare module 'vscode' { transientOutputs?: boolean; /** - * Controls if a meetadata property change will trigger notebook document content change and if it will be used in the diff editor - * Default to false. If the content provider doesn't persisit a metadata property in the file document, it should be set to true. + * @deprecated use transientCellMetadata instead */ transientMetadata?: { [K in keyof NotebookCellMetadata]?: boolean }; + + /** + * Controls if a cell metadata property change will trigger notebook document content change and if it will be used in the diff editor + * Default to false. If the content provider doesn't persisit a metadata property in the file document, it should be set to true. + */ + transientCellMetadata?: { [K in keyof NotebookCellMetadata]?: boolean }; + + /** + * Controls if a document metadata property change will trigger notebook document content change and if it will be used in the diff editor + * Default to false. If the content provider doesn't persisit a metadata property in the file document, it should be set to true. + */ + transientDocumentMetadata?: { [K in keyof NotebookDocumentMetadata]?: boolean }; } export interface NotebookDocument { diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index be31750123beb..ad7fc64e50b5a 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -14,7 +14,7 @@ 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'; import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService'; -import { ICellRange, INotebookCellStatusBarItemProvider, INotebookDocumentFilter, INotebookExclusiveDocumentFilter, INotebookKernel, NotebookDataDto, TransientMetadata, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ICellRange, INotebookCellStatusBarItemProvider, INotebookDocumentFilter, INotebookExclusiveDocumentFilter, INotebookKernel, NotebookDataDto, TransientCellMetadata, TransientDocumentMetadata, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, MainContext, MainThreadNotebookShape, NotebookExtensionDescription } from '../common/extHost.protocol'; @@ -65,17 +65,19 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { async $registerNotebookProvider(extension: NotebookExtensionDescription, viewType: string, options: { transientOutputs: boolean; - transientMetadata: TransientMetadata; + transientCellMetadata: TransientCellMetadata; + transientDocumentMetadata: TransientDocumentMetadata; viewOptions?: { displayName: string; filenamePattern: (string | IRelativePattern | INotebookExclusiveDocumentFilter)[]; exclusive: boolean; }; }): Promise { - let contentOptions = { transientOutputs: options.transientOutputs, transientMetadata: options.transientMetadata }; + let contentOptions = { transientOutputs: options.transientOutputs, transientCellMetadata: options.transientCellMetadata, transientDocumentMetadata: options.transientDocumentMetadata }; const controller: IMainNotebookController = { get options() { return contentOptions; }, set options(newOptions) { - contentOptions.transientMetadata = newOptions.transientMetadata; + contentOptions.transientCellMetadata = newOptions.transientCellMetadata; + contentOptions.transientDocumentMetadata = newOptions.transientDocumentMetadata; contentOptions.transientOutputs = newOptions.transientOutputs; }, viewOptions: options.viewOptions, @@ -107,7 +109,7 @@ export class MainThreadNotebooks implements MainThreadNotebookShape { this._notebookProviders.set(viewType, { controller, disposable }); } - async $updateNotebookProviderOptions(viewType: string, options?: { transientOutputs: boolean; transientMetadata: TransientMetadata; }): Promise { + async $updateNotebookProviderOptions(viewType: string, options?: { transientOutputs: boolean; transientCellMetadata: TransientCellMetadata; transientDocumentMetadata: TransientDocumentMetadata; }): Promise { const provider = this._notebookProviders.get(viewType); if (provider && options) { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 8d2b7126b2992..eba9420ea931c 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -50,7 +50,7 @@ import { TunnelDto } from 'vs/workbench/api/common/extHostTunnelService'; import { TunnelCreationOptions, TunnelProviderFeatures, TunnelOptions, ProvidedPortAttributes } from 'vs/platform/remote/common/tunnel'; import { Timeline, TimelineChangeEvent, TimelineOptions, TimelineProviderDescriptor, InternalTimelineOptions } from 'vs/workbench/contrib/timeline/common/timeline'; import { revive } from 'vs/base/common/marshalling'; -import { NotebookCellMetadata, NotebookDocumentMetadata, ICellEditOperation, NotebookCellsChangedEventDto, NotebookDataDto, IMainCellDto, INotebookDocumentFilter, TransientMetadata, ICellRange, INotebookDecorationRenderOptions, INotebookExclusiveDocumentFilter, IOutputDto, TransientOptions, IImmediateCellEditOperation, INotebookCellStatusBarItem } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { NotebookCellMetadata, NotebookDocumentMetadata, ICellEditOperation, NotebookCellsChangedEventDto, NotebookDataDto, IMainCellDto, INotebookDocumentFilter, TransientCellMetadata, ICellRange, INotebookDecorationRenderOptions, INotebookExclusiveDocumentFilter, IOutputDto, TransientOptions, IImmediateCellEditOperation, INotebookCellStatusBarItem, TransientDocumentMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CallHierarchyItem } from 'vs/workbench/contrib/callHierarchy/common/callHierarchy'; import { Dto } from 'vs/base/common/types'; import { DebugConfigurationProviderTriggerKind, TestResultState } from 'vs/workbench/api/common/extHostTypes'; @@ -874,10 +874,11 @@ export interface INotebookCellStatusBarListDto { export interface MainThreadNotebookShape extends IDisposable { $registerNotebookProvider(extension: NotebookExtensionDescription, viewType: string, options: { transientOutputs: boolean; - transientMetadata: TransientMetadata; + transientCellMetadata: TransientCellMetadata; + transientDocumentMetadata: TransientDocumentMetadata; viewOptions?: { displayName: string; filenamePattern: (string | IRelativePattern | INotebookExclusiveDocumentFilter)[]; exclusive: boolean; }; }): Promise; - $updateNotebookProviderOptions(viewType: string, options?: { transientOutputs: boolean; transientMetadata: TransientMetadata; }): Promise; + $updateNotebookProviderOptions(viewType: string, options?: { transientOutputs: boolean; transientCellMetadata: TransientCellMetadata; transientDocumentMetadata: TransientDocumentMetadata; }): Promise; $unregisterNotebookProvider(viewType: string): Promise; $registerNotebookSerializer(handle: number, extension: NotebookExtensionDescription, viewType: string, options: TransientOptions): void; diff --git a/src/vs/workbench/api/common/extHostApiCommands.ts b/src/vs/workbench/api/common/extHostApiCommands.ts index 7c2367c41f27f..534c4e6b32fcf 100644 --- a/src/vs/workbench/api/common/extHostApiCommands.ts +++ b/src/vs/workbench/api/common/extHostApiCommands.ts @@ -18,7 +18,7 @@ import { ICommandsExecutor, RemoveFromRecentlyOpenedAPICommand, OpenIssueReporte import { isFalsyOrEmpty } from 'vs/base/common/arrays'; import { IRange } from 'vs/editor/common/core/range'; import { IPosition } from 'vs/editor/common/core/position'; -import { TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { TransientCellMetadata, TransientDocumentMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ITextEditorOptions } from 'vs/platform/editor/common/editor'; import { VSBuffer } from 'vs/base/common/buffer'; import { decodeSemanticTokensDto } from 'vs/editor/common/services/semanticTokensDto'; @@ -343,7 +343,7 @@ const newCommands: ApiCommand[] = [ new ApiCommandResult<{ viewType: string; displayName: string; - options: { transientOutputs: boolean; transientMetadata: TransientMetadata }; + options: { transientOutputs: boolean; transientCellMetadata: TransientCellMetadata; transientDocumentMetadata: TransientDocumentMetadata; }; filenamePattern: (string | types.RelativePattern | { include: string | types.RelativePattern, exclude: string | types.RelativePattern })[] }[], { viewType: string; @@ -354,7 +354,12 @@ const newCommands: ApiCommand[] = [ return { viewType: item.viewType, displayName: item.displayName, - options: { transientOutputs: item.options.transientOutputs, transientMetadata: item.options.transientMetadata }, + options: { + transientOutputs: item.options.transientOutputs, + transientMetadata: item.options.transientCellMetadata, + transientCellMetadata: item.options.transientCellMetadata, + transientDocumentMetadata: item.options.transientDocumentMetadata + }, filenamePattern: item.filenamePattern.map(pattern => typeConverters.NotebookExclusiveDocumentPattern.to(pattern)) }; })) diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 76d2868f15fe8..a391c3fc2cdbd 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -376,7 +376,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { const internalOptions = typeConverters.NotebookDocumentContentOptions.from(options); this._notebookProxy.$registerNotebookProvider({ id: extension.identifier, location: extension.extensionLocation, description: extension.description }, viewType, { transientOutputs: internalOptions.transientOutputs, - transientMetadata: internalOptions.transientMetadata, + transientCellMetadata: internalOptions.transientCellMetadata, + transientDocumentMetadata: internalOptions.transientDocumentMetadata, viewOptions: options?.viewOptions && viewOptionsFilenamePattern ? { displayName: options.viewOptions.displayName, filenamePattern: viewOptionsFilenamePattern, exclusive: options.viewOptions.exclusive || false } : undefined }); diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 66a4a2096c768..0e038d0388d2f 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -1633,15 +1633,16 @@ export namespace NotebookDocumentContentOptions { export function from(options: vscode.NotebookDocumentContentOptions | undefined): notebooks.TransientOptions { return { transientOutputs: options?.transientOutputs ?? false, - transientMetadata: { - ...options?.transientMetadata, + transientCellMetadata: { + ...(options?.transientCellMetadata ?? options?.transientMetadata), executionOrder: true, runState: true, runStartTime: true, runStartTimeAdjustment: true, runEndTime: true, lastRunSuccess: true - } + }, + transientDocumentMetadata: options?.transientDocumentMetadata ?? {} }; } } diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index 824b8e53b4841..24c56ad11391f 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -19,7 +19,7 @@ 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 { 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, CHANGE_CELL_LANGUAGE, QUIT_EDIT_CELL_COMMAND_ID } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellExecutionState, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, TransientMetadata, SelectionStateType, ICellReplaceEdit } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, CellKind, ICellEditOperation, ICellRange, INotebookDocumentFilter, isDocumentExcludePattern, NotebookCellMetadata, NotebookCellExecutionState, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, TransientCellMetadata, TransientDocumentMetadata, SelectionStateType, ICellReplaceEdit } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; @@ -1692,7 +1692,7 @@ CommandsRegistry.registerCommand('notebook.trust', (accessor, args) => { CommandsRegistry.registerCommand('_resolveNotebookContentProvider', (accessor, args): { viewType: string; displayName: string; - options: { transientOutputs: boolean; transientMetadata: TransientMetadata; }; + options: { transientOutputs: boolean; transientCellMetadata: TransientCellMetadata; transientDocumentMetadata: TransientDocumentMetadata; }; filenamePattern: (string | glob.IRelativePattern | { include: string | glob.IRelativePattern, exclude: string | glob.IRelativePattern; })[]; }[] => { const notebookService = accessor.get(INotebookService); @@ -1721,7 +1721,11 @@ CommandsRegistry.registerCommand('_resolveNotebookContentProvider', (accessor, a viewType: provider.id, displayName: provider.displayName, filenamePattern: filenamePatterns, - options: { transientMetadata: provider.options.transientMetadata, transientOutputs: provider.options.transientOutputs } + options: { + transientCellMetadata: provider.options.transientCellMetadata, + transientDocumentMetadata: provider.options.transientDocumentMetadata, + transientOutputs: provider.options.transientOutputs + } }; }); }); diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts index 53c6484ba101d..925415cd8ba29 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts @@ -493,11 +493,11 @@ export function getFormatedMetadataJSON(documentTextModel: NotebookTextModel, me let filteredMetadata: { [key: string]: any } = {}; if (documentTextModel) { - const transientMetadata = documentTextModel.transientOptions.transientMetadata; + const transientCellMetadata = documentTextModel.transientOptions.transientCellMetadata; const keys = new Set([...Object.keys(metadata)]); for (let key of keys) { - if (!(transientMetadata[key as keyof NotebookCellMetadata]) + if (!(transientCellMetadata[key as keyof NotebookCellMetadata]) ) { filteredMetadata[key] = metadata[key as keyof NotebookCellMetadata]; } diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts index 31d4fb746b9a7..b5e6a193a79b8 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts @@ -159,11 +159,11 @@ export class NotebookCellTextModel extends Disposable implements ICell { private _getPersisentMetadata() { let filteredMetadata: { [key: string]: any } = {}; - const transientMetadata = this.transientOptions.transientMetadata; + const transientCellMetadata = this.transientOptions.transientCellMetadata; const keys = new Set([...Object.keys(this.metadata)]); for (let key of keys) { - if (!(transientMetadata[key as keyof NotebookCellMetadata]) + if (!(transientCellMetadata[key as keyof NotebookCellMetadata]) ) { filteredMetadata[key] = this.metadata[key as keyof NotebookCellMetadata]; } diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index 2db4d08fc7e0a..9b89f9d44d6ba 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -193,7 +193,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel private _cells: NotebookCellTextModel[] = []; metadata: NotebookDocumentMetadata = notebookDocumentMetadataDefaults; - transientOptions: TransientOptions = { transientMetadata: {}, transientOutputs: false }; + transientOptions: TransientOptions = { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }; private _versionId = 0; /** @@ -601,14 +601,14 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel if (key === 'custom') { if (!this._customMetadataEqual(a[key], b[key]) && - !(this.transientOptions.transientMetadata[key as keyof NotebookCellMetadata]) + !(this.transientOptions.transientCellMetadata[key as keyof NotebookCellMetadata]) ) { return true; } } else if ( (a[key as keyof NotebookCellMetadata] !== b[key as keyof NotebookCellMetadata]) && - !(this.transientOptions.transientMetadata[key as keyof NotebookCellMetadata]) + !(this.transientOptions.transientCellMetadata[key as keyof NotebookCellMetadata]) ) { return true; } diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index aab2b90d11388..a22954a200e92 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -92,11 +92,13 @@ export interface NotebookCellMetadata { custom?: { [key: string]: unknown }; } -export type TransientMetadata = { [K in keyof NotebookCellMetadata]?: boolean }; +export type TransientCellMetadata = { [K in keyof NotebookCellMetadata]?: boolean }; +export type TransientDocumentMetadata = { [K in keyof NotebookDocumentMetadata]?: boolean }; export interface TransientOptions { transientOutputs: boolean; - transientMetadata: TransientMetadata; + transientCellMetadata: TransientCellMetadata; + transientDocumentMetadata: TransientDocumentMetadata; } export interface INotebookMimeTypeSelector { diff --git a/src/vs/workbench/contrib/notebook/common/notebookProvider.ts b/src/vs/workbench/contrib/notebook/common/notebookProvider.ts index a47f53754659b..25fe5afc6bdc2 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookProvider.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookProvider.ts @@ -61,7 +61,8 @@ export class NotebookProviderInfo { this.dynamicContribution = descriptor.dynamicContribution; this.exclusive = descriptor.exclusive; this._options = { - transientMetadata: {}, + transientCellMetadata: {}, + transientDocumentMetadata: {}, transientOutputs: false }; } diff --git a/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts index 213b6c019be7d..86556d7032b18 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts @@ -34,7 +34,7 @@ suite('NotebookViewModel', () => { instantiationService.stub(IThemeService, new TestThemeService()); test('ctor', function () { - const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], notebookDocumentMetadataDefaults, { transientMetadata: {}, transientOutputs: false }, undoRedoService, modelService, modeService); + const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], notebookDocumentMetadataDefaults, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }, undoRedoService, modelService, modeService); const model = new NotebookEditorTestModel(notebook); const eventDispatcher = new NotebookEventDispatcher(); const viewModel = new NotebookViewModel('notebook', model.notebook, eventDispatcher, null, instantiationService, bulkEditService, undoRedoService, textModelService); diff --git a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts index ddc046d8f798f..70a88c92cb363 100644 --- a/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/testNotebookEditor.ts @@ -47,7 +47,7 @@ export class TestCell extends NotebookCellTextModel { outputs: IOutputDto[], modeService: IModeService, ) { - super(CellUri.generate(URI.parse('test:///fake/notebook'), handle), handle, source, language, cellKind, outputs, undefined, { transientMetadata: {}, transientOutputs: false }, modeService); + super(CellUri.generate(URI.parse('test:///fake/notebook'), handle), handle, source, language, cellKind, outputs, undefined, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }, modeService); } } @@ -152,7 +152,7 @@ function _createTestNotebookEditor(instantiationService: TestInstantiationServic outputs: cell[3] ?? [], metadata: cell[4] }; - }), notebookDocumentMetadataDefaults, { transientMetadata: {}, transientOutputs: false }); + }), notebookDocumentMetadataDefaults, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }); const model = new NotebookEditorTestModel(notebook); const eventDispatcher = new NotebookEventDispatcher(); From 4db99a10b862a774b21699a0acc5ea1066e6674e Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 15 Apr 2021 17:23:46 -0700 Subject: [PATCH 07/18] transientDocumentMetadata --- .../common/model/notebookTextModel.ts | 57 +++++++++++++------ 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index 9b89f9d44d6ba..fd36077f5025f 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -515,25 +515,28 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel private _updateNotebookMetadata(metadata: NotebookDocumentMetadata, computeUndoRedo: boolean) { const oldMetadata = this.metadata; - this.metadata = metadata; + const triggerDirtyChange = this._isDocumentMetadataChanged(this.metadata, metadata); - if (computeUndoRedo) { - const that = this; - this._operationManager.pushEditOperation(new class implements IResourceUndoRedoElement { - readonly type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; - get resource() { - return that.uri; - } - readonly label = 'Update Notebook Metadata'; - undo() { - that._updateNotebookMetadata(oldMetadata, false); - } - redo() { - that._updateNotebookMetadata(metadata, false); - } - }(), undefined, undefined); + if (triggerDirtyChange) { + if (computeUndoRedo) { + const that = this; + this._operationManager.pushEditOperation(new class implements IResourceUndoRedoElement { + readonly type: UndoRedoElementType.Resource = UndoRedoElementType.Resource; + get resource() { + return that.uri; + } + readonly label = 'Update Notebook Metadata'; + undo() { + that._updateNotebookMetadata(oldMetadata, false); + } + redo() { + that._updateNotebookMetadata(metadata, false); + } + }(), undefined, undefined); + } } + this.metadata = metadata; this._eventEmitter.emit({ kind: NotebookCellsChangeType.ChangeDocumentMetadata, metadata: this.metadata, transient: this._isDocumentMetadataChangeTransient(oldMetadata, metadata) }, true); } @@ -595,6 +598,28 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel } + private _isDocumentMetadataChanged(a: NotebookDocumentMetadata, b: NotebookDocumentMetadata) { + const keys = new Set([...Object.keys(a || {}), ...Object.keys(b || {})]); + for (let key of keys) { + if (key === 'custom') { + if (!this._customMetadataEqual(a[key], b[key]) + && + !(this.transientOptions.transientCellMetadata[key as keyof NotebookCellMetadata]) + ) { + return true; + } + } else if ( + (a[key as keyof NotebookDocumentMetadata] !== b[key as keyof NotebookDocumentMetadata]) + && + !(this.transientOptions.transientDocumentMetadata[key as keyof NotebookDocumentMetadata]) + ) { + return true; + } + } + + return false; + } + private _isCellMetadataChanged(a: NotebookCellMetadata, b: NotebookCellMetadata) { const keys = new Set([...Object.keys(a || {}), ...Object.keys(b || {})]); for (let key of keys) { From 55b2ab7fa4b0e058ac182a1e9a785dc2a3286639 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 15 Apr 2021 17:30:39 -0700 Subject: [PATCH 08/18] re #121329. NotebookCellExecutionSummary readonly properties. --- src/vs/vscode.proposed.d.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 3dd87ac446e9d..4ce72aff0557e 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -988,10 +988,10 @@ declare module 'vscode' { } export interface NotebookCellExecutionSummary { - executionOrder?: number; - success?: boolean; - startTime?: number; - endTime?: number; + readonly executionOrder?: number; + readonly success?: boolean; + readonly startTime?: number; + readonly endTime?: number; } // todo@API support ids https://github.com/jupyter/enhancement-proposals/blob/master/62-cell-id/cell-id.md From 644e1d0bc42bd29b34aa81e6607fd312c26beaa0 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 09:58:22 +0200 Subject: [PATCH 09/18] :lipstick: in notebook service, also make sure to clean-up editor contributions when disabling/enabling extensions --- .../notebook/browser/notebookServiceImpl.ts | 54 +++++++++---------- .../notebook/test/notebookServiceImpl.test.ts | 6 +-- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts index 400ebd17df726..55aa4501c5c58 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts @@ -83,18 +83,23 @@ export class NotebookKernelProviderInfoStore { } export class NotebookProviderInfoStore extends Disposable { + private static readonly CUSTOM_EDITORS_STORAGE_ID = 'notebookEditors'; private static readonly CUSTOM_EDITORS_ENTRY_ID = 'editors'; private readonly _memento: Memento; private _handled: boolean = false; + + private readonly _contributedEditors = new Map(); + private readonly _contributedEditorDisposables = new DisposableStore(); + constructor( - storageService: IStorageService, - extensionService: IExtensionService, - private readonly _editorOverrideService: IEditorOverrideService, - private readonly _instantiationService: IInstantiationService, - private readonly _configurationService: IConfigurationService, - private readonly _accessibilityService: IAccessibilityService, + @IStorageService storageService: IStorageService, + @IExtensionService extensionService: IExtensionService, + @IEditorOverrideService private readonly _editorOverrideService: IEditorOverrideService, + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IAccessibilityService private readonly _accessibilityService: IAccessibilityService, + @IInstantiationService private readonly _instantiationService: IInstantiationService, ) { super(); this._memento = new Memento(NotebookProviderInfoStore.CUSTOM_EDITORS_STORAGE_ID, storageService); @@ -110,18 +115,25 @@ export class NotebookProviderInfoStore extends Disposable { if (!this._handled) { // there is no extension point registered for notebook content provider // clear the memento and cache - this.clear(); + this._clear(); mementoObject[NotebookProviderInfoStore.CUSTOM_EDITORS_ENTRY_ID] = []; this._memento.saveMemento(); this._updateProviderExtensionsInfo(); } })); + + notebookProviderExtensionPoint.setHandler(extensions => this._setupHandler(extensions)); } - setupHandler(extensions: readonly IExtensionPointUser[]) { + override dispose(): void { + this._clear(); + super.dispose(); + } + + private _setupHandler(extensions: readonly IExtensionPointUser[]) { this._handled = true; - this.clear(); + this._clear(); for (const extension of extensions) { for (const notebookContribution of extension.value) { @@ -174,10 +186,10 @@ export class NotebookProviderInfoStore extends Disposable { } - private registerContributionPoint(notebookProviderInfo: NotebookProviderInfo): void { + private _registerContributionPoint(notebookProviderInfo: NotebookProviderInfo): void { for (const selector of notebookProviderInfo.selectors) { const globPattern = (selector as INotebookExclusiveDocumentFilter).include || selector as glob.IRelativePattern | string; - this._register(this._editorOverrideService.registerContributionPoint( + this._contributedEditorDisposables.add(this._editorOverrideService.registerContributionPoint( globPattern, priorityToRank(notebookProviderInfo.exclusive ? ContributedEditorPriority.exclusive : notebookProviderInfo.priority), { @@ -215,10 +227,10 @@ export class NotebookProviderInfoStore extends Disposable { } } - private readonly _contributedEditors = new Map(); - clear() { + private _clear(): void { this._contributedEditors.clear(); + this._contributedEditorDisposables.clear(); } get(viewType: string): NotebookProviderInfo | undefined { @@ -230,7 +242,7 @@ export class NotebookProviderInfoStore extends Disposable { return; } this._contributedEditors.set(info.id, info); - this.registerContributionPoint(info); + this._registerContributionPoint(info); const mementoObject = this._memento.getMemento(StorageScope.GLOBAL, StorageTarget.MACHINE); mementoObject[NotebookProviderInfoStore.CUSTOM_EDITORS_ENTRY_ID] = Array.from(this._contributedEditors.values()); @@ -318,8 +330,6 @@ class ModelData implements IDisposable { } } - - export class NotebookService extends Disposable implements INotebookService, IEditorTypesHandler { declare readonly _serviceBrand: undefined; @@ -352,25 +362,15 @@ export class NotebookService extends Disposable implements INotebookService, IEd @IExtensionService private readonly _extensionService: IExtensionService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IAccessibilityService private readonly _accessibilityService: IAccessibilityService, - @IStorageService private readonly _storageService: IStorageService, @IInstantiationService private readonly _instantiationService: IInstantiationService, @ICodeEditorService private readonly _codeEditorService: ICodeEditorService, @IConfigurationService private readonly configurationService: IConfigurationService, - @IEditorOverrideService private readonly _editorOverrideService: IEditorOverrideService, ) { super(); - this._notebookProviderInfoStore = new NotebookProviderInfoStore( - this._storageService, - this._extensionService, this._editorOverrideService, - this._instantiationService, this._configurationService, - this._accessibilityService - ); + this._notebookProviderInfoStore = _instantiationService.createInstance(NotebookProviderInfoStore); this._register(this._notebookProviderInfoStore); - notebookProviderExtensionPoint.setHandler((extensions) => { - this._notebookProviderInfoStore.setupHandler(extensions); - }); notebookRendererExtensionPoint.setHandler((renderers) => { this._notebookRenderersInfoStore.clear(); diff --git a/src/vs/workbench/contrib/notebook/test/notebookServiceImpl.test.ts b/src/vs/workbench/contrib/notebook/test/notebookServiceImpl.test.ts index cac96403a87cd..f7db87dad5744 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookServiceImpl.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookServiceImpl.test.ts @@ -31,11 +31,9 @@ suite('NotebookProviderInfoStore', function () { override onDidRegisterExtensions = Event.None; }, instantiationService.createInstance(EditorOverrideService), - instantiationService, new TestConfigurationService(), - new class extends mock() { - - } + new class extends mock() { }, + instantiationService, ); const fooInfo = new NotebookProviderInfo({ From 8877d8ca9410bfd5113b9a7675512e18c585c2c8 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 10:51:45 +0200 Subject: [PATCH 10/18] hook up execution task cancellation from controllers --- src/vs/workbench/api/common/extHostNotebook.ts | 2 +- src/vs/workbench/api/common/extHostNotebookKernels.ts | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index a391c3fc2cdbd..2291021e66f55 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -624,7 +624,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { } } - private cancelOneNotebookCellExecution(cell: ExtHostCell): void { + cancelOneNotebookCellExecution(cell: ExtHostCell): void { const execution = this._activeExecutions.get(cell.uri); execution?.cancel(); } diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index 2629fb9806294..9728952aad589 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -223,6 +223,16 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { if (obj.controller.interruptHandler) { obj.controller.interruptHandler(document.notebookDocument); } + + // we do both? interrupt and cancellation or should we be selective? + for (const range of ranges) { + for (let i = range.start; i < range.end; i++) { + const cell = document.getCellFromIndex(i); + if (cell) { + this._extHostNotebook.cancelOneNotebookCellExecution(cell); + } + } + } } $acceptRendererMessage(handle: number, editorId: string, message: any): void { From fd7cedae033841ff9ba0aed5bbec513f3e25d2d6 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 11:02:30 +0200 Subject: [PATCH 11/18] :lipstick: --- src/vs/vscode.proposed.d.ts | 45 +++++++------------------------------ 1 file changed, 8 insertions(+), 37 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 4ce72aff0557e..06059e991e44f 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1236,44 +1236,18 @@ declare module 'vscode' { constructor(cells: NotebookCellData[], metadata?: NotebookDocumentMetadata); } - /** - * Communication object passed to the {@link NotebookContentProvider} and - * {@link NotebookOutputRenderer} to communicate with the webview. - */ + /** @deprecated used NotebookController */ export interface NotebookCommunication { - /** - * ID of the editor this object communicates with. A single notebook - * document can have multiple attached webviews and editors, when the - * notebook is split for instance. The editor ID lets you differentiate - * between them. - */ + /** @deprecated used NotebookController */ readonly editorId: string; - - /** - * Fired when the output hosting webview posts a message. - */ + /** @deprecated used NotebookController */ readonly onDidReceiveMessage: Event; - /** - * Post a message to the output hosting webview. - * - * Messages are only delivered if the editor is live. - * - * @param message Body of the message. This must be a string or other json serializable object. - */ + /** @deprecated used NotebookController */ postMessage(message: any): Thenable; - - /** - * Convert a uri for the local file system to one that can be used inside outputs webview. - */ + /** @deprecated used NotebookController */ asWebviewUri(localResource: Uri): Uri; - - // @rebornix - // readonly onDidDispose: Event; } - // export function registerNotebookKernel(selector: string, kernel: NotebookKernel): Disposable; - - export interface NotebookDocumentShowOptions { viewColumn?: ViewColumn; preserveFocus?: boolean; @@ -1566,6 +1540,7 @@ declare module 'vscode' { uri: Uri; } + /** @deprecated used NotebookController */ export interface NotebookKernel { // todo@API make this mandatory? @@ -1679,8 +1654,7 @@ declare module 'vscode' { filenamePattern?: NotebookFilenamePattern; } - // todo@API very unclear, provider MUST not return alive object but only data object - // todo@API unclear how the flow goes + /** @deprecated used NotebookController */ export interface NotebookKernelProvider { onDidChangeKernels?: Event; provideKernels(document: NotebookDocument, token: CancellationToken): ProviderResult; @@ -1688,9 +1662,6 @@ declare module 'vscode' { } export interface NotebookEditor { - - // todo@API unsure about that - // kernel, kernel selection, kernel provider /** @deprecated kernels are private object*/ readonly kernel?: NotebookKernel; } @@ -1698,7 +1669,7 @@ declare module 'vscode' { export namespace notebook { /** @deprecated */ export const onDidChangeActiveNotebookKernel: Event<{ document: NotebookDocument, kernel: NotebookKernel | undefined; }>; - /** @deprecated use createNotebookKernel */ + /** @deprecated used NotebookController */ export function registerNotebookKernelProvider(selector: NotebookDocumentFilter, provider: NotebookKernelProvider): Disposable; } From c074bf897c0c3cd6ba5705828402924a3cc2b4dc Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 11:29:28 +0200 Subject: [PATCH 12/18] make preload a creation argument, no editor needed when calling asWebviewUri --- src/vs/vscode.proposed.d.ts | 5 +++-- src/vs/workbench/api/common/extHostNotebookKernels.ts | 11 ++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 06059e991e44f..0c382edc80d0e 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1414,7 +1414,6 @@ declare module 'vscode' { supportedLanguages: string[]; hasExecutionOrder?: boolean; - preloads?: NotebookKernelPreload[]; /** * The execute handler is invoked when the run gestures in the UI are selected, e.g Run Cell, Run All, @@ -1439,9 +1438,10 @@ declare module 'vscode' { createNotebookCellExecutionTask(cell: NotebookCell): NotebookCellExecutionTask; // ipc + readonly preloads: NotebookKernelPreload[]; readonly onDidReceiveMessage: Event<{ editor: NotebookEditor, message: any }>; postMessage(message: any, editor?: NotebookEditor): Thenable; - asWebviewUri(localResource: Uri, editor: NotebookEditor): Uri; + asWebviewUri(localResource: Uri): Uri; } export interface NotebookControllerOptions { @@ -1453,6 +1453,7 @@ declare module 'vscode' { hasExecutionOrder?: boolean; executeHandler: (cells: NotebookCell[], controller: NotebookController) => void; interruptHandler?: (notebook: NotebookDocument) => void + preloads?: NotebookKernelPreload[] } export namespace notebook { diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index 9728952aad589..15f7e5651d6c3 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -58,6 +58,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { extensionLocation: extension.extensionLocation, label: options.label, supportedLanguages: [], + preloads: options.preloads ? options.preloads.map(extHostTypeConverters.NotebookKernelPreload.from) : [] }; // @@ -123,11 +124,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { _update(); }, get preloads() { - return data.preloads && data.preloads.map(extHostTypeConverters.NotebookKernelPreload.to); - }, - set preloads(value) { - data.preloads = value && value.map(extHostTypeConverters.NotebookKernelPreload.from); - _update(); + return data.preloads ? data.preloads.map(extHostTypeConverters.NotebookKernelPreload.to) : []; }, get executeHandler() { return _executeHandler; @@ -162,8 +159,8 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { postMessage(message, editor) { return that._proxy.$postMessage(handle, editor && that._extHostNotebook.getIdByEditor(editor), message); }, - asWebviewUri(uri: URI, editor) { - return asWebviewUri(that._initData.environment, that._extHostNotebook.getIdByEditor(editor)!, uri); + asWebviewUri(uri: URI) { + return asWebviewUri(that._initData.environment, data.id, uri); } }; From 98f69856ea656df30a5f55f667ed008e3f4b4937 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 11:45:04 +0200 Subject: [PATCH 13/18] extract types for execute/interrupt handler, interrupt in not per document but per controller --- src/vs/vscode.proposed.d.ts | 24 +++++++++++++------ .../api/common/extHostNotebookKernels.ts | 10 ++++---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 0c382edc80d0e..43ececda86151 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1393,6 +1393,18 @@ declare module 'vscode' { export type NotebookSelector = NotebookFilter | string | ReadonlyArray; + export interface NotebookExecutionHandler { + /** + * @param cells The notebook cells to execute + * @param controller The controller that the handler is attached to + */ + (this: NotebookController, cells: NotebookCell[], controller: NotebookController): void + } + + export interface NotebookInterruptHandler { + (this: NotebookController): void; + } + export interface NotebookController { readonly id: string; @@ -1419,12 +1431,10 @@ declare module 'vscode' { * The execute handler is invoked when the run gestures in the UI are selected, e.g Run Cell, Run All, * Run Selection etc. */ - readonly executeHandler: (cells: NotebookCell[], controller: NotebookController) => void; + executeHandler: NotebookExecutionHandler; // optional kernel interrupt command - interruptHandler?: (notebook: NotebookDocument) => void - - // remove kernel + interruptHandler?: NotebookInterruptHandler dispose(): void; /** @@ -1451,9 +1461,9 @@ declare module 'vscode' { selector: NotebookSelector; supportedLanguages?: string[]; hasExecutionOrder?: boolean; - executeHandler: (cells: NotebookCell[], controller: NotebookController) => void; - interruptHandler?: (notebook: NotebookDocument) => void - preloads?: NotebookKernelPreload[] + executeHandler: NotebookExecutionHandler; + interruptHandler?: NotebookInterruptHandler; + preloads?: NotebookKernelPreload[]; } export namespace notebook { diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index 15f7e5651d6c3..f46bde322445d 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -16,8 +16,6 @@ import { isNonEmptyArray } from 'vs/base/common/arrays'; import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService'; import { asWebviewUri } from 'vs/workbench/api/common/shared/webview'; -type ExecuteHandler = (cells: vscode.NotebookCell[], controller: vscode.NotebookController) => void; -type InterruptHandler = (notebook: vscode.NotebookDocument) => void; interface IKernelData { controller: vscode.NotebookController; @@ -62,8 +60,8 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { }; // - let _executeHandler: ExecuteHandler = options.executeHandler; - let _interruptHandler: InterruptHandler | undefined = options.interruptHandler; + let _executeHandler = options.executeHandler; + let _interruptHandler = options.interruptHandler; // todo@jrieken the selector needs to be massaged this._proxy.$addKernel(handle, data); @@ -200,7 +198,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { } try { - obj.controller.executeHandler(cells, obj.controller); + obj.controller.executeHandler.call(obj.controller, cells, obj.controller); } catch (err) { // console.error(err); @@ -218,7 +216,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { throw new Error('MISSING notebook'); } if (obj.controller.interruptHandler) { - obj.controller.interruptHandler(document.notebookDocument); + obj.controller.interruptHandler.call(obj.controller); } // we do both? interrupt and cancellation or should we be selective? From 217359a80d257bc21232bc402a093fc8830ff2f1 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 12:39:23 +0200 Subject: [PATCH 14/18] some jsdoc and relaxed NotebookRange --- src/vs/vscode.proposed.d.ts | 64 +++++++++++++++++++-- src/vs/workbench/api/common/extHostTypes.ts | 13 +++-- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 43ececda86151..f2c4650bf6de7 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1052,11 +1052,39 @@ declare module 'vscode' { transientDocumentMetadata?: { [K in keyof NotebookDocumentMetadata]?: boolean }; } + /** + * Represents a notebook. Notebooks are composed of cells and metadata. + */ export interface NotebookDocument { + + /** + * The associated uri for this notebook. + * + * *Note* that most notebooks use the `file`-scheme, which means they are files on disk. However, **not** all notebooks are + * saved on disk and therefore the `scheme` must be checked before trying to access the underlying file or siblings on disk. + * + * @see [FileSystemProvider](#FileSystemProvider) + * @see [TextDocumentContentProvider](#TextDocumentContentProvider) + */ readonly uri: Uri; + + // todo@API should we really expose this? + readonly viewType: string; + + /** + * The version number of this notebook (it will strictly increase after each + * change, including undo/redo). + */ readonly version: number; + /** + * `true` if there are unpersisted changes. + */ readonly isDirty: boolean; + + /** + * Is this notebook representing an untitled file which has not been saved yet. + */ readonly isUntitled: boolean; /** @@ -1065,13 +1093,13 @@ declare module 'vscode' { */ readonly isClosed: boolean; + /** + * The [metadata](#NotebookDocumentMetadata) for this notebook. + */ readonly metadata: NotebookDocumentMetadata; - // todo@API should we really expose this? - readonly viewType: string; - /** - * The number of cells in the notebook document. + * The number of cells in the notebook. */ readonly cellCount: number; @@ -1102,17 +1130,43 @@ declare module 'vscode' { save(): Thenable; } + /** + * A notebook range represents on ordered pair of two cell indicies. + * It is guaranteed that start is less than or equal to end. + */ export class NotebookRange { + + /** + * The zero-based start index of this range. + */ readonly start: number; + /** - * exclusive + * The exclusive end index of this range (zero-based). */ readonly end: number; + /** + * `true` if `start` and `end` are equals + */ readonly isEmpty: boolean; + /** + * Create a new notebook range. If `start` is not + * before or equal to `end`, the values will be swapped. + * + * @param start start index + * @param end end index. + */ constructor(start: number, end: number); + /** + * Derive a new range for this range. + * + * @param change An object that describes a change to this range. + * @return A range that reflects the given change. Will return `this` range if the change + * is not changing anything. + */ with(change: { start?: number, end?: number }): NotebookRange; } diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 0ffbab2f9be95..3d510c2f43049 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -2916,11 +2916,16 @@ export class NotebookRange { if (start < 0) { throw illegalArgument('start must be positive'); } - if (end < start) { - throw illegalArgument('end cannot be smaller than start'); + if (end < 0) { + throw illegalArgument('end must be positive'); + } + if (start <= end) { + this._start = start; + this._end = end; + } else { + this._start = end; + this._end = start; } - this._start = start; - this._end = end; } with(change: { start?: number, end?: number }): NotebookRange { From 95932045c7ddbb31b61e2a409bc39ecceb64f674 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 14:22:44 +0200 Subject: [PATCH 15/18] check that notebook controller are unique per id --- .../api/browser/mainThreadNotebookKernels.ts | 2 +- .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostNotebookKernels.ts | 24 +++++++++++++++---- .../browser/notebookKernelServiceImpl.ts | 2 +- .../api/extHostNotebookKernel2.test.ts | 6 ++--- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 95c87645843f5..8306de1f23abc 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -177,7 +177,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape // --- kernel adding/updating/removal - $addKernel(handle: number, data: INotebookKernelDto2): void { + async $addKernel(handle: number, data: INotebookKernelDto2): Promise { const that = this; const kernel = new class extends MainThreadKernel { executeNotebookCellsRequest(uri: URI, ranges: ICellRange[]): void { diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index cd9fbb04c2e19..439e3b6384c8c 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1107,7 +1107,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I }, createNotebookController(options) { checkProposedApiEnabled(extension); - return extHostNotebookKernels.createKernel(extension, options); + return extHostNotebookKernels.createNotebookController(extension, options); } }; diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index eba9420ea931c..e8b9d92c21025 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -924,7 +924,7 @@ export interface INotebookKernelDto2 { export interface MainThreadNotebookKernelsShape extends IDisposable { $postMessage(handle: number, editorId: string | undefined, message: any): Promise; - $addKernel(handle: number, data: INotebookKernelDto2): void; + $addKernel(handle: number, data: INotebookKernelDto2): Promise; $updateKernel(handle: number, data: Partial): void; $removeKernel(handle: number): void; } diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index f46bde322445d..ce96dbab8d231 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -8,7 +8,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { ExtHostNotebookKernelsShape, IMainContext, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from 'vs/workbench/api/common/extHost.protocol'; import * as vscode from 'vscode'; import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook'; -import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'; +import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions'; import { URI, UriComponents } from 'vs/base/common/uri'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import * as extHostTypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; @@ -16,8 +16,8 @@ import { isNonEmptyArray } from 'vs/base/common/arrays'; import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService'; import { asWebviewUri } from 'vs/workbench/api/common/shared/webview'; - interface IKernelData { + extensionId: ExtensionIdentifier, controller: vscode.NotebookController; onDidChangeSelection: Emitter<{ selected: boolean; notebook: vscode.NotebookDocument; }>; onDidReceiveMessage: Emitter<{ editor: vscode.NotebookEditor, message: any }>; @@ -38,7 +38,13 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { this._proxy = mainContext.getProxy(MainContext.MainThreadNotebookKernels); } - createKernel(extension: IExtensionDescription, options: vscode.NotebookControllerOptions): vscode.NotebookController { + createNotebookController(extension: IExtensionDescription, options: vscode.NotebookControllerOptions): vscode.NotebookController { + + for (let data of this._kernelData.values()) { + if (data.controller.id === options.id) { + throw new Error(`notebook controller with id '${options.id}' ALREADY exist`); + } + } const handle = this._handlePool++; const that = this; @@ -64,7 +70,11 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { let _interruptHandler = options.interruptHandler; // todo@jrieken the selector needs to be massaged - this._proxy.$addKernel(handle, data); + this._proxy.$addKernel(handle, data).catch(err => { + // this can happen when a kernel with that ID is already registered + console.log(err); + isDisposed = true; + }); // update: all setters write directly into the dto object // and trigger an update. the actual update will only happen @@ -127,6 +137,9 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { get executeHandler() { return _executeHandler; }, + set executeHandler(value) { + _executeHandler = value ?? (() => console.warn(`NO execute handler from notebook controller '${data.id}' of extension: '${extension.identifier}'`)); + }, get interruptHandler() { return _interruptHandler; }, @@ -162,9 +175,10 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { } }; - this._kernelData.set(handle, { controller, onDidChangeSelection, onDidReceiveMessage }); + this._kernelData.set(handle, { extensionId: extension.identifier, controller, onDidChangeSelection, onDidReceiveMessage }); controller.supportedLanguages = options.supportedLanguages ?? []; + controller.executeHandler = options.executeHandler; controller.interruptHandler = options.interruptHandler; controller.hasExecutionOrder = options.hasExecutionOrder ?? false; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts index 6ceaa6b68f6d1..448dea23ef39b 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts @@ -40,7 +40,7 @@ export class NotebookKernelService implements INotebookKernelService { registerKernel(kernel: INotebookKernel2): IDisposable { if (this._kernels.has(kernel.id)) { - throw new Error(`KERNEL with id '${kernel.id}' already exists`); + throw new Error(`NOTEBOOK CONTROLLER with id '${kernel.id}' already exists`); } this._kernels.set(kernel.id, kernel); diff --git a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts index 02b4cce8d000e..ee38b4fdba5f0 100644 --- a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts +++ b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts @@ -29,7 +29,7 @@ suite('NotebookKernel', function () { override $registerCommand() { } }); rpcProtocol.set(MainContext.MainThreadNotebookKernels, new class extends mock() { - override $addKernel(handle: number, data: INotebookKernelDto2): void { + async override $addKernel(handle: number, data: INotebookKernelDto2): Promise { kernelData.set(handle, data); } override $removeKernel(handle: number) { @@ -50,7 +50,7 @@ suite('NotebookKernel', function () { test('create/dispose kernel', async function () { - const kernel = extHostNotebookKernels.createKernel(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); assert.ok(kernel); assert.strictEqual(kernel.id, 'foo'); @@ -73,7 +73,7 @@ suite('NotebookKernel', function () { test('update kernel', async function () { - const kernel = extHostNotebookKernels.createKernel(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); await rpcProtocol.sync(); assert.ok(kernel); From 93dbc7ac44c88d23a174cf648bef3a18215e9c2c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 16 Apr 2021 15:23:34 +0200 Subject: [PATCH 16/18] remove NotebookControllerOptions and simplify createNotebookController-signature --- src/vs/vscode.proposed.d.ts | 24 ++++++------- .../workbench/api/common/extHost.api.impl.ts | 4 +-- .../api/common/extHostNotebookKernels.ts | 35 +++++++++---------- .../api/extHostNotebookKernel2.test.ts | 4 +-- 4 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index f2c4650bf6de7..3a7686be4c8b9 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1508,20 +1508,18 @@ declare module 'vscode' { asWebviewUri(localResource: Uri): Uri; } - export interface NotebookControllerOptions { - id: string; - label: string; - description?: string; - selector: NotebookSelector; - supportedLanguages?: string[]; - hasExecutionOrder?: boolean; - executeHandler: NotebookExecutionHandler; - interruptHandler?: NotebookInterruptHandler; - preloads?: NotebookKernelPreload[]; - } - export namespace notebook { - export function createNotebookController(options: NotebookControllerOptions): NotebookController; + + /** + * Creates a new notebook controller. + * + * @param id Unique identifier of the controller + * @param selector A notebook selector to narrow down notebook type or path + * @param label The label of the controller + * @param handler + * @param preloads + */ + export function createNotebookController(id: string, selector: NotebookSelector, label: string, handler?: NotebookExecutionHandler, preloads?: NotebookKernelPreload[]): NotebookController; } //#endregion diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 439e3b6384c8c..3f830963b679c 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1105,9 +1105,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I checkProposedApiEnabled(extension); return extHostNotebook.createNotebookCellExecution(uri, index, kernelId); }, - createNotebookController(options) { + createNotebookController(id, selector, label, executeHandler, preloads) { checkProposedApiEnabled(extension); - return extHostNotebookKernels.createNotebookController(extension, options); + return extHostNotebookKernels.createNotebookController(extension, id, selector, label, executeHandler, preloads); } }; diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index ce96dbab8d231..9430414832b9e 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -38,17 +38,20 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { this._proxy = mainContext.getProxy(MainContext.MainThreadNotebookKernels); } - createNotebookController(extension: IExtensionDescription, options: vscode.NotebookControllerOptions): vscode.NotebookController { + createNotebookController(extension: IExtensionDescription, id: string, selector: vscode.NotebookSelector, label: string, handler?: vscode.NotebookExecutionHandler, preloads?: vscode.NotebookKernelPreload[]): vscode.NotebookController { for (let data of this._kernelData.values()) { - if (data.controller.id === options.id) { - throw new Error(`notebook controller with id '${options.id}' ALREADY exist`); + if (data.controller.id === id) { + throw new Error(`notebook controller with id '${id}' ALREADY exist`); } } const handle = this._handlePool++; const that = this; + const _defaultSupportedLanguages = ['plaintext']; + const _defaultExecutHandler = () => console.warn(`NO execute handler from notebook controller '${data.id}' of extension: '${extension.identifier}'`); + let isDisposed = false; const commandDisposables = new DisposableStore(); @@ -56,18 +59,18 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { const onDidReceiveMessage = new Emitter<{ editor: vscode.NotebookEditor, message: any }>(); const data: INotebookKernelDto2 = { - id: options.id, - selector: options.selector, + id: id, + selector: selector, extensionId: extension.identifier, extensionLocation: extension.extensionLocation, - label: options.label, - supportedLanguages: [], - preloads: options.preloads ? options.preloads.map(extHostTypeConverters.NotebookKernelPreload.from) : [] + label: label || extension.identifier.value, + supportedLanguages: _defaultSupportedLanguages, + preloads: preloads ? preloads.map(extHostTypeConverters.NotebookKernelPreload.from) : [] }; // - let _executeHandler = options.executeHandler; - let _interruptHandler = options.interruptHandler; + let _executeHandler: vscode.NotebookExecutionHandler = handler ?? _defaultExecutHandler; + let _interruptHandler: vscode.NotebookInterruptHandler | undefined; // todo@jrieken the selector needs to be massaged this._proxy.$addKernel(handle, data).catch(err => { @@ -100,7 +103,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { return data.label; }, set label(value) { - data.label = value; + data.label = value ?? extension.displayName ?? extension.name; _update(); }, get description() { @@ -121,7 +124,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { return data.supportedLanguages; }, set supportedLanguages(value) { - data.supportedLanguages = isNonEmptyArray(value) ? value : ['plaintext']; + data.supportedLanguages = isNonEmptyArray(value) ? value : _defaultSupportedLanguages; _update(); }, get hasExecutionOrder() { @@ -138,7 +141,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { return _executeHandler; }, set executeHandler(value) { - _executeHandler = value ?? (() => console.warn(`NO execute handler from notebook controller '${data.id}' of extension: '${extension.identifier}'`)); + _executeHandler = value ?? _defaultExecutHandler; }, get interruptHandler() { return _interruptHandler; @@ -176,12 +179,6 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { }; this._kernelData.set(handle, { extensionId: extension.identifier, controller, onDidChangeSelection, onDidReceiveMessage }); - - controller.supportedLanguages = options.supportedLanguages ?? []; - controller.executeHandler = options.executeHandler; - controller.interruptHandler = options.interruptHandler; - controller.hasExecutionOrder = options.hasExecutionOrder ?? false; - return controller; } diff --git a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts index ee38b4fdba5f0..3710f4b9e8adc 100644 --- a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts +++ b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts @@ -50,7 +50,7 @@ suite('NotebookKernel', function () { test('create/dispose kernel', async function () { - const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, 'foo', '*', 'Foo'); assert.ok(kernel); assert.strictEqual(kernel.id, 'foo'); @@ -73,7 +73,7 @@ suite('NotebookKernel', function () { test('update kernel', async function () { - const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, 'foo', '*', 'Foo'); await rpcProtocol.sync(); assert.ok(kernel); From c657cdbef3096311df67a488a7250a990d94e93c Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 16 Apr 2021 13:37:04 -0700 Subject: [PATCH 17/18] avoid stripping custom data. --- .../workbench/api/common/extHostTypeConverters.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 0e038d0388d2f..e360f85de1a0d 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -1419,7 +1419,17 @@ export namespace NotebookRange { export namespace NotebookCellMetadata { export function to(data: notebooks.NotebookCellMetadata): types.NotebookCellMetadata { - return new types.NotebookCellMetadata(data.inputCollapsed, data.outputCollapsed).with({ custom: data.custom }); + return new types.NotebookCellMetadata().with({ + ...data, + ...{ + executionOrder: null, + lastRunSuccess: null, + runState: null, + runStartTime: null, + runStartTimeAdjustment: null, + runEndTime: null + } + }); } } @@ -1430,7 +1440,7 @@ export namespace NotebookDocumentMetadata { } export function to(data: notebooks.NotebookDocumentMetadata): types.NotebookDocumentMetadata { - return new types.NotebookDocumentMetadata(data.trusted, data.custom); + return new types.NotebookDocumentMetadata().with(data); } } From 85ca912f72c006aff3fe0e98ef02e233ac3ad3a2 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 16 Apr 2021 13:39:14 -0700 Subject: [PATCH 18/18] extract NotebookDocumentContentOptions viewOptions. --- src/vs/vscode.proposed.d.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 3a7686be4c8b9..e64c3be2d22eb 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1052,6 +1052,17 @@ declare module 'vscode' { transientDocumentMetadata?: { [K in keyof NotebookDocumentMetadata]?: boolean }; } + export interface NotebookDocumentContentOptions { + /** + * Not ready for production or development use yet. + */ + viewOptions?: { + displayName: string; + filenamePattern: NotebookFilenamePattern[]; + exclusive?: boolean; + }; + } + /** * Represents a notebook. Notebooks are composed of cells and metadata. */ @@ -1580,18 +1591,7 @@ declare module 'vscode' { // TODO@api use NotebookDocumentFilter instead of just notebookType:string? // TODO@API options duplicates the more powerful variant on NotebookContentProvider - export function registerNotebookContentProvider(notebookType: string, provider: NotebookContentProvider, - options?: NotebookDocumentContentOptions & { - /** - * Not ready for production or development use yet. - */ - viewOptions?: { - displayName: string; - filenamePattern: NotebookFilenamePattern[]; - exclusive?: boolean; - }; - } - ): Disposable; + export function registerNotebookContentProvider(notebookType: string, provider: NotebookContentProvider, options?: NotebookDocumentContentOptions): Disposable; } //#endregion