From a3bc7bab820ebfe5a15076f38a63de42a2341001 Mon Sep 17 00:00:00 2001 From: Jonah Iden Date: Thu, 4 Apr 2024 15:23:01 +0200 Subject: [PATCH 1/4] reenable initial reading of execution summary Signed-off-by: Jonah Iden --- packages/plugin-ext/src/plugin/type-converters.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/plugin-ext/src/plugin/type-converters.ts b/packages/plugin-ext/src/plugin/type-converters.ts index 22f679de04007..86e70d7258510 100644 --- a/packages/plugin-ext/src/plugin/type-converters.ts +++ b/packages/plugin-ext/src/plugin/type-converters.ts @@ -749,9 +749,9 @@ export function isUriComponents(arg: unknown): arg is UriComponents { return isObject(arg) && typeof arg.scheme === 'string' && (arg['$mid'] === 1 || ( - typeof arg.path === 'string' && - typeof arg.query === 'string' && - typeof arg.fragment === 'string')); + typeof arg.path === 'string' && + typeof arg.query === 'string' && + typeof arg.fragment === 'string')); } export function isModelCallHierarchyItem(arg: unknown): arg is model.CallHierarchyItem { @@ -1522,8 +1522,8 @@ export namespace NotebookCellData { cellKind: NotebookCellKind.from(data.kind), language: data.languageId, source: data.value, - // metadata: data.metadata, - // internalMetadata: NotebookCellExecutionSummary.from(data.executionSummary ?? {}), + metadata: data.metadata, + internalMetadata: NotebookCellExecutionSummary.from(data.executionSummary ?? {}), outputs: data.outputs ? data.outputs.map(NotebookCellOutputConverter.from) : [] }; } From 77fd694c557f04e5a9913a83e25e1df17129c3c4 Mon Sep 17 00:00:00 2001 From: Jonah Iden Date: Fri, 5 Apr 2024 11:29:54 +0200 Subject: [PATCH 2/4] disabling clear all outputs command in notebook toolbar when no outputs are available Signed-off-by: Jonah Iden --- packages/core/src/common/event.ts | 18 +++++++++++ .../notebook-actions-contribution.ts | 10 +++++-- .../notebook-cell-actions-contribution.ts | 5 +++- .../notebook/src/browser/notebook-types.ts | 2 ++ .../service/notebook-context-manager.ts | 14 +++++++-- packages/notebook/src/browser/style/index.css | 5 ++-- .../src/browser/view-model/notebook-model.ts | 30 +++++++++++-------- .../browser/view/notebook-main-toolbar.tsx | 23 +++++++++----- 8 files changed, 79 insertions(+), 28 deletions(-) diff --git a/packages/core/src/common/event.ts b/packages/core/src/common/event.ts index 99fdef2ea5fde..00d6e0d00eab5 100644 --- a/packages/core/src/common/event.ts +++ b/packages/core/src/common/event.ts @@ -467,3 +467,21 @@ export class AsyncEmitter extends Emitter { } } + +export class QueueableEmitter extends Emitter { + + currentQueue?: T[]; + + queue(...arg: T[]): void { + if (!this.currentQueue) { + this.currentQueue = []; + } + this.currentQueue.push(...arg); + } + + override fire(): void { + super.fire(this.currentQueue || []); + this.currentQueue = undefined; + } + +} diff --git a/packages/notebook/src/browser/contributions/notebook-actions-contribution.ts b/packages/notebook/src/browser/contributions/notebook-actions-contribution.ts index 50f3ebf73a9ef..c0fc2fb10f2d0 100644 --- a/packages/notebook/src/browser/contributions/notebook-actions-contribution.ts +++ b/packages/notebook/src/browser/contributions/notebook-actions-contribution.ts @@ -24,7 +24,7 @@ import { NotebookKernelQuickPickService } from '../service/notebook-kernel-quick import { NotebookExecutionService } from '../service/notebook-execution-service'; import { NotebookEditorWidget } from '../notebook-editor-widget'; import { NotebookEditorWidgetService } from '../service/notebook-editor-widget-service'; -import { NOTEBOOK_CELL_FOCUSED, NOTEBOOK_EDITOR_FOCUSED } from './notebook-context-keys'; +import { NOTEBOOK_CELL_FOCUSED, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_HAS_OUTPUTS } from './notebook-context-keys'; export namespace NotebookCommands { export const ADD_NEW_CELL_COMMAND = Command.toDefaultLocalizedCommand({ @@ -141,7 +141,10 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon )); commands.registerCommand(NotebookCommands.CLEAR_ALL_OUTPUTS_COMMAND, this.editableCommandHandler( - notebookModel => notebookModel.cells.forEach(cell => cell.spliceNotebookCellOutputs({ start: 0, deleteCount: cell.outputs.length, newOutputs: [] })) + notebookModel => notebookModel.applyEdits(notebookModel.cells.map(cell => ({ + editType: CellEditType.Output, + handle: cell.handle, deleteCount: cell.outputs.length, outputs: [] + })), false) )); commands.registerCommand(NotebookCommands.CHANGE_SELECTED_CELL, @@ -217,7 +220,8 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon commandId: NotebookCommands.CLEAR_ALL_OUTPUTS_COMMAND.id, label: nls.localizeByDefault('Clear All Outputs'), icon: codicon('clear-all'), - order: '30' + order: '30', + when: NOTEBOOK_HAS_OUTPUTS }); // other items } diff --git a/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts b/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts index 37766a87c0bb5..507b5051f3fd5 100644 --- a/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts +++ b/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts @@ -301,7 +301,10 @@ export class NotebookCellActionContribution implements MenuContribution, Command } }); commands.registerCommand(NotebookCellCommands.CLEAR_OUTPUTS_COMMAND, this.editableCellCommandHandler( - (_, cell) => cell.spliceNotebookCellOutputs({ start: 0, deleteCount: (cell ?? this.getSelectedCell()).outputs.length, newOutputs: [] }) + (notebook, cell) => notebook.applyEdits([{ + editType: CellEditType.Output, + handle: cell.handle, outputs: [], deleteCount: cell.outputs.length, append: false + }], true) )); commands.registerCommand(NotebookCellCommands.CHANGE_OUTPUT_PRESENTATION_COMMAND, this.editableCellCommandHandler( (_, __, output) => output?.requestOutputPresentationUpdate() diff --git a/packages/notebook/src/browser/notebook-types.ts b/packages/notebook/src/browser/notebook-types.ts index 1f82f071755e7..f1bb70b5eab11 100644 --- a/packages/notebook/src/browser/notebook-types.ts +++ b/packages/notebook/src/browser/notebook-types.ts @@ -111,6 +111,7 @@ export interface CellOutputEdit { editType: CellEditType.Output; index: number; outputs: CellOutput[]; + deleteCount?: number; append?: boolean; } @@ -118,6 +119,7 @@ export interface CellOutputEditByHandle { editType: CellEditType.Output; handle: number; outputs: CellOutput[]; + deleteCount?: number; append?: boolean; } diff --git a/packages/notebook/src/browser/service/notebook-context-manager.ts b/packages/notebook/src/browser/service/notebook-context-manager.ts index 7980fcfa82090..76d417822eb9c 100644 --- a/packages/notebook/src/browser/service/notebook-context-manager.ts +++ b/packages/notebook/src/browser/service/notebook-context-manager.ts @@ -22,13 +22,14 @@ import { NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_FOCUSED, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, - NOTEBOOK_CELL_TYPE, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_SELECTED, + NOTEBOOK_CELL_TYPE, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_VIEW_TYPE } from '../contributions/notebook-context-keys'; import { NotebookEditorWidget } from '../notebook-editor-widget'; import { NotebookCellModel } from '../view-model/notebook-cell-model'; -import { CellKind } from '../../common'; +import { CellKind, NotebookCellsChangeType } from '../../common'; import { NotebookExecutionStateService } from './notebook-execution-state-service'; +import debounce = require('@theia/core/shared/lodash.debounce'); @injectable() export class NotebookContextManager { @@ -73,6 +74,15 @@ export class NotebookContextManager { } })); + widget.model?.onDidChangeContent(events => { + if (events.some(e => e.kind === NotebookCellsChangeType.ModelChange || e.kind === NotebookCellsChangeType.Output)) { + this.scopedStore.setContext(NOTEBOOK_HAS_OUTPUTS, widget.model?.cells.some(cell => cell.outputs.length > 0)); + this.onDidChangeContextEmitter.fire(this.createContextKeyChangedEvent([NOTEBOOK_HAS_OUTPUTS])); + } + }); + + this.scopedStore.setContext(NOTEBOOK_HAS_OUTPUTS, !!widget.model?.cells.find(cell => cell.outputs.length > 0)); + // Cell Selection realted keys this.scopedStore.setContext(NOTEBOOK_CELL_FOCUSED, !!widget.model?.selectedCell); widget.model?.onDidChangeSelectedCell(e => { diff --git a/packages/notebook/src/browser/style/index.css b/packages/notebook/src/browser/style/index.css index 9f1733a3fb3b6..a106da20e730c 100644 --- a/packages/notebook/src/browser/style/index.css +++ b/packages/notebook/src/browser/style/index.css @@ -213,8 +213,9 @@ cursor: pointer; } -.theia-notebook-main-toolbar-item:hover { - background-color: var(--theia-toolbar-hoverBackground); +.theia-notebook-main-toolbar-item.theia-mod-disabled:hover { + background-color: transparent; + cursor: default; } .theia-notebook-main-toolbar-item-text { diff --git a/packages/notebook/src/browser/view-model/notebook-model.ts b/packages/notebook/src/browser/view-model/notebook-model.ts index 41df43d0fab82..e7e1f33b281dc 100644 --- a/packages/notebook/src/browser/view-model/notebook-model.ts +++ b/packages/notebook/src/browser/view-model/notebook-model.ts @@ -14,7 +14,7 @@ // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 // ***************************************************************************** -import { Disposable, Emitter, Event, Resource, URI } from '@theia/core'; +import { Disposable, Emitter, Event, QueueableEmitter, Resource, URI } from '@theia/core'; import { Saveable, SaveOptions } from '@theia/core/lib/browser'; import { CellData, CellEditType, CellUri, NotebookCellInternalMetadata, @@ -65,7 +65,7 @@ export class NotebookModel implements Saveable, Disposable { protected readonly onDidAddOrRemoveCellEmitter = new Emitter(); readonly onDidAddOrRemoveCell = this.onDidAddOrRemoveCellEmitter.event; - protected readonly onDidChangeContentEmitter = new Emitter(); + protected readonly onDidChangeContentEmitter = new QueueableEmitter(); readonly onDidChangeContent = this.onDidChangeContentEmitter.event; protected readonly onDidChangeSelectedCellEmitter = new Emitter(); @@ -281,13 +281,18 @@ export class NotebookModel implements Saveable, Disposable { } else { // could definitely be more efficient. See vscode __spliceNotebookCellOutputs2 // For now, just replace the whole existing output with the new output - cell.spliceNotebookCellOutputs({ start: 0, deleteCount: cell.outputs.length, newOutputs: edit.outputs }); + cell.spliceNotebookCellOutputs({ start: 0, deleteCount: edit.deleteCount ?? cell.outputs.length, newOutputs: edit.outputs }); } - + this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.Output, index: cellIndex, outputs: cell.outputs, append: edit.append ?? false }); break; } case CellEditType.OutputItems: cell.changeOutputItems(edit.outputId, !!edit.append, edit.items); + this.onDidChangeContentEmitter.queue({ + kind: NotebookCellsChangeType.OutputItem, index: cellIndex, outputItems: edit.items, + outputId: edit.outputId, append: edit.append ?? false + }); + break; case CellEditType.Metadata: this.changeCellMetadata(this.cells[cellIndex], edit.metadata, computeUndoRedo); @@ -308,12 +313,15 @@ export class NotebookModel implements Saveable, Disposable { this.moveCellToIndex(cellIndex, edit.length, edit.newIdx, computeUndoRedo); break; } + // if selected cell is affected update it because it can potentially have been replaced if (cell === this.selectedCell) { this.setSelectedCell(this.cells[cellIndex]); } } + this.onDidChangeContentEmitter.fire(); + } protected async replaceCells(start: number, deleteCount: number, newCells: CellData[], computeUndoRedo: boolean): Promise { @@ -352,7 +360,7 @@ export class NotebookModel implements Saveable, Disposable { await Promise.all(cells.map(cell => cell.resolveTextModel())); this.onDidAddOrRemoveCellEmitter.fire({ rawEvent: { kind: NotebookCellsChangeType.ModelChange, changes }, newCellIds: cells.map(cell => cell.handle) }); - this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ModelChange, changes }]); + this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ModelChange, changes }); if (cells.length > 0) { this.setSelectedCell(cells[cells.length - 1]); cells[cells.length - 1].requestEdit(); @@ -370,9 +378,7 @@ export class NotebookModel implements Saveable, Disposable { } cell.internalMetadata = newInternalMetadata; - this.onDidChangeContentEmitter.fire([ - { kind: NotebookCellsChangeType.ChangeCellInternalMetadata, index: this.cells.indexOf(cell), internalMetadata: newInternalMetadata } - ]); + this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeCellInternalMetadata, index: this.cells.indexOf(cell), internalMetadata: newInternalMetadata }); } protected updateNotebookMetadata(metadata: NotebookDocumentMetadata, computeUndoRedo: boolean): void { @@ -385,7 +391,7 @@ export class NotebookModel implements Saveable, Disposable { } this.metadata = metadata; - this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ChangeDocumentMetadata, metadata: this.metadata }]); + this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeDocumentMetadata, metadata: this.metadata }); } protected changeCellMetadataPartial(cell: NotebookCellModel, metadata: NullablePartialNotebookCellMetadata, computeUndoRedo: boolean): void { @@ -417,7 +423,7 @@ export class NotebookModel implements Saveable, Disposable { } cell.metadata = metadata; - this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ChangeCellMetadata, index: this.cells.indexOf(cell), metadata: cell.metadata }]); + this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeCellMetadata, index: this.cells.indexOf(cell), metadata: cell.metadata }); } protected changeCellLanguage(cell: NotebookCellModel, languageId: string, computeUndoRedo: boolean): void { @@ -427,7 +433,7 @@ export class NotebookModel implements Saveable, Disposable { cell.language = languageId; - this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ChangeCellLanguage, index: this.cells.indexOf(cell), language: languageId }]); + this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeCellLanguage, index: this.cells.indexOf(cell), language: languageId }); } protected moveCellToIndex(fromIndex: number, length: number, toIndex: number, computeUndoRedo: boolean): boolean { @@ -440,7 +446,7 @@ export class NotebookModel implements Saveable, Disposable { const cells = this.cells.splice(fromIndex, length); this.cells.splice(toIndex, 0, ...cells); - this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.Move, index: fromIndex, length, newIdx: toIndex, cells }]); + this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.Move, index: fromIndex, length, newIdx: toIndex, cells }); return true; } diff --git a/packages/notebook/src/browser/view/notebook-main-toolbar.tsx b/packages/notebook/src/browser/view/notebook-main-toolbar.tsx index 12dff11891552..56ee76d0b850c 100644 --- a/packages/notebook/src/browser/view/notebook-main-toolbar.tsx +++ b/packages/notebook/src/browser/view/notebook-main-toolbar.tsx @@ -57,6 +57,10 @@ export class NotebookMainToolbar extends React.Component; } - protected renderMenuItem(item: MenuNode): React.ReactNode { + protected renderMenuItem(item: MenuNode, submenu?: string): React.ReactNode { if (item.role === CompoundMenuNodeRole.Group) { - const itemNodes = ArrayUtils.coalesce(item.children?.map(child => this.renderMenuItem(child)) ?? []); + const itemNodes = ArrayUtils.coalesce(item.children?.map(child => this.renderMenuItem(child, item.id)) ?? []); return {itemNodes} {itemNodes && itemNodes.length > 0 && } ; - } else if (!item.when || this.props.contextKeyService.match(item.when, this.props.editorNode)) { + } else if ((this.nativeSubmenus.includes(submenu ?? '')) || !item.when || this.props.contextKeyService.match(item.when, this.props.editorNode)) { const visibleCommand = Boolean(this.props.commandRegistry.getVisibleHandler(item.command ?? '', this.props.notebookModel)); if (!visibleCommand) { return undefined; } const title = (this.props.commandRegistry.getCommand(item.command ?? '') as NotebookCommand)?.tooltip ?? item.label; - return
{ - if (item.command) { + if (item.command && (!item.when || this.props.contextKeyService.match(item.when, this.props.editorNode))) { this.props.commandRegistry.executeCommand(item.command, this.props.notebookModel); } }}> @@ -133,15 +137,18 @@ export class NotebookMainToolbar extends React.Component): void { + protected getAdditionalClasses(item: MenuNode): string { + return !item.when || this.props.contextKeyService.match(item.when, this.props.editorNode) ? '' : ' theia-mod-disabled'; + } + + protected getAllContextKeys(menus: readonly MenuNode[], keySet: Set): void { menus.filter(item => item.when) .forEach(item => this.props.contextKeyService.parseKeys(item.when!)?.forEach(key => keySet.add(key))); From ccb3b3d9e3f020bcf595eab94236cf45ccb050df Mon Sep 17 00:00:00 2001 From: Jonah Iden Date: Fri, 5 Apr 2024 11:40:12 +0200 Subject: [PATCH 3/4] fixed build Signed-off-by: Jonah Iden --- .../notebook/src/browser/service/notebook-context-manager.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/notebook/src/browser/service/notebook-context-manager.ts b/packages/notebook/src/browser/service/notebook-context-manager.ts index 76d417822eb9c..e9ece94ae316a 100644 --- a/packages/notebook/src/browser/service/notebook-context-manager.ts +++ b/packages/notebook/src/browser/service/notebook-context-manager.ts @@ -29,7 +29,6 @@ import { NotebookEditorWidget } from '../notebook-editor-widget'; import { NotebookCellModel } from '../view-model/notebook-cell-model'; import { CellKind, NotebookCellsChangeType } from '../../common'; import { NotebookExecutionStateService } from './notebook-execution-state-service'; -import debounce = require('@theia/core/shared/lodash.debounce'); @injectable() export class NotebookContextManager { From 2dfc07f9834bc58891c7e03016e360530d9d8789 Mon Sep 17 00:00:00 2001 From: Jonah Iden Date: Fri, 5 Apr 2024 14:19:06 +0200 Subject: [PATCH 4/4] formatting Co-authored-by: Mark Sujew --- .../contributions/notebook-cell-actions-contribution.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts b/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts index 507b5051f3fd5..1364bb9effdea 100644 --- a/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts +++ b/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts @@ -303,7 +303,10 @@ export class NotebookCellActionContribution implements MenuContribution, Command commands.registerCommand(NotebookCellCommands.CLEAR_OUTPUTS_COMMAND, this.editableCellCommandHandler( (notebook, cell) => notebook.applyEdits([{ editType: CellEditType.Output, - handle: cell.handle, outputs: [], deleteCount: cell.outputs.length, append: false + handle: cell.handle, + outputs: [], + deleteCount: cell.outputs.length, + append: false }], true) )); commands.registerCommand(NotebookCellCommands.CHANGE_OUTPUT_PRESENTATION_COMMAND, this.editableCellCommandHandler(