notebook api review comments batch 2

packages/notebook/src/browser/contributions/notebook-actions-contribution.ts

@@ -20,7 +20,7 @@ import { ApplicationShell, codicon, CommonCommands } from '@theia/core/lib/brows
 import { NotebookModel } from '../view-model/notebook-model';
 import { NotebookService } from '../service/notebook-service';
 import { CellEditType, CellKind } from '../../common';
-import { KernelPickerMRUStrategy, NotebookKernelQuickPickService } from '../service/notebook-kernel-quick-pick-service';
+import { NotebookKernelQuickPickService } from '../service/notebook-kernel-quick-pick-service';
 import { NotebookExecutionService } from '../service/notebook-execution-service';
 import { NotebookEditorWidget } from '../notebook-editor-widget';
 
@@ -66,7 +66,7 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon
     protected notebookService: NotebookService;
 
     @inject(NotebookKernelQuickPickService)
-    protected notebookKernelQuickPickService: KernelPickerMRUStrategy;
+    protected notebookKernelQuickPickService: NotebookKernelQuickPickService;
 
     @inject(NotebookExecutionService)
     protected notebookExecutionService: NotebookExecutionService;

packages/notebook/src/browser/index.ts

@@ -24,3 +24,4 @@ export * from './service/notebook-execution-state-service';
 export * from './service/notebook-model-resolver-service';
 export * from './service/notebook-renderer-messaging-service';
 export * from './renderers/cell-output-webview';
+export * from './notebook-types';

packages/notebook/src/browser/notebook-frontend-module.ts

@@ -37,7 +37,7 @@ import { NotebookActionsContribution } from './contributions/notebook-actions-co
 import { NotebookExecutionService } from './service/notebook-execution-service';
 import { NotebookExecutionStateService } from './service/notebook-execution-state-service';
 import { NotebookKernelService } from './service/notebook-kernel-service';
-import { KernelPickerMRUStrategy, NotebookKernelQuickPickService } from './service/notebook-kernel-quick-pick-service';
+import { NotebookKernelQuickPickService } from './service/notebook-kernel-quick-pick-service';
 import { NotebookKernelHistoryService } from './service/notebook-kernel-history-service';
 import { NotebookEditorWidgetService } from './service/notebook-editor-widget-service';
 import { NotebookRendererMessagingService } from './service/notebook-renderer-messaging-service';
@@ -65,7 +65,7 @@ export default new ContainerModule(bind => {
     bind(NotebookKernelService).toSelf().inSingletonScope();
     bind(NotebookRendererMessagingService).toSelf().inSingletonScope();
     bind(NotebookKernelHistoryService).toSelf().inSingletonScope();
-    bind(NotebookKernelQuickPickService).to(KernelPickerMRUStrategy).inSingletonScope();
+    bind(NotebookKernelQuickPickService).toSelf().inSingletonScope();
 
     bind(NotebookCellResourceResolver).toSelf().inSingletonScope();
     bind(ResourceResolver).toService(NotebookCellResourceResolver);

packages/notebook/src/browser/notebook-types.ts

@@ -0,0 +1,172 @@
+// *****************************************************************************
+// Copyright (C) 2023 TypeFox and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// http://www.eclipse.org/legal/epl-2.0.
+//
+// This Source Code may also be made available under the following Secondary
+// Licenses when the conditions for such availability set forth in the Eclipse
+// Public License v. 2.0 are satisfied: GNU General Public License, version 2
+// with the GNU Classpath Exception which is available at
+// https://www.gnu.org/software/classpath/license.html.
+//
+// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
+// *****************************************************************************
+
+import {
+    CellData, CellEditType, CellMetadataEdit, CellOutput, CellOutputItem, CellRange, NotebookCellContentChangeEvent,
+    NotebookCellInternalMetadata,
+    NotebookCellsChangeInternalMetadataEvent,
+    NotebookCellsChangeLanguageEvent,
+    NotebookCellsChangeMetadataEvent,
+    NotebookCellsChangeType, NotebookCellTextModelSplice, NotebookDocumentMetadata
+} from '../common';
+import { NotebookCell } from './view-model/notebook-cell-model';
+
+export interface NotebookTextModelChangedEvent {
+    readonly rawEvents: NotebookContentChangedEvent[];
+    // readonly versionId: number;
+    readonly synchronous?: boolean;
+    readonly endSelectionState?: SelectionState;
+};
+
+export type NotebookContentChangedEvent = (NotebookCellsInitializeEvent<NotebookCell> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent |
+    NotebookCellsModelChangedEvent<NotebookCell> | NotebookCellsModelMoveEvent<NotebookCell> | NotebookOutputChangedEvent | NotebookOutputItemChangedEvent |
+    NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent |
+    NotebookCellsChangeInternalMetadataEvent | NotebookDocumentUnknownChangeEvent); // & { transient: boolean };
+
+export interface NotebookCellsInitializeEvent<T> {
+    readonly kind: NotebookCellsChangeType.Initialize;
+    readonly changes: NotebookCellTextModelSplice<T>[];
+}
+
+export interface NotebookDocumentChangeMetadataEvent {
+    readonly kind: NotebookCellsChangeType.ChangeDocumentMetadata;
+    readonly metadata: NotebookDocumentMetadata;
+}
+
+export interface NotebookCellsModelChangedEvent<T> {
+    readonly kind: NotebookCellsChangeType.ModelChange;
+    readonly changes: NotebookCellTextModelSplice<T>[];
+}
+
+export interface NotebookModelWillAddRemoveEvent {
+    readonly rawEvent: NotebookCellsModelChangedEvent<CellData>;
+};
+
+export interface NotebookCellsModelMoveEvent<T> {
+    readonly kind: NotebookCellsChangeType.Move;
+    readonly index: number;
+    readonly length: number;
+    readonly newIdx: number;
+    readonly cells: T[];
+}
+
+export interface NotebookOutputChangedEvent {
+    readonly kind: NotebookCellsChangeType.Output;
+    readonly index: number;
+    readonly outputs: CellOutput[];
+    readonly append: boolean;
+}
+
+export interface NotebookOutputItemChangedEvent {
+    readonly kind: NotebookCellsChangeType.OutputItem;
+    readonly index: number;
+    readonly outputId: string;
+    readonly outputItems: CellOutputItem[];
+    readonly append: boolean;
+}
+
+export interface NotebookDocumentUnknownChangeEvent {
+    readonly kind: NotebookCellsChangeType.Unknown;
+}
+
+export enum SelectionStateType {
+    Handle = 0,
+    Index = 1
+}
+
+export interface SelectionHandleState {
+    kind: SelectionStateType.Handle;
+    primary: number | null;
+    selections: number[];
+}
+
+export interface SelectionIndexState {
+    kind: SelectionStateType.Index;
+    focus: CellRange;
+    selections: CellRange[];
+}
+
+export type SelectionState = SelectionHandleState | SelectionIndexState;
+
+export interface NotebookModelWillAddRemoveEvent {
+    readonly newCellIds?: number[];
+    readonly rawEvent: NotebookCellsModelChangedEvent<CellData>;
+};
+
+export interface CellOutputEdit {
+    editType: CellEditType.Output;
+    index: number;
+    outputs: CellOutput[];
+    append?: boolean;
+}
+
+export interface CellOutputEditByHandle {
+    editType: CellEditType.Output;
+    handle: number;
+    outputs: CellOutput[];
+    append?: boolean;
+}
+
+export interface CellOutputItemEdit {
+    editType: CellEditType.OutputItems;
+    items: CellOutputItem[];
+    outputId: string;
+    append?: boolean;
+}
+
+export interface CellLanguageEdit {
+    editType: CellEditType.CellLanguage;
+    index: number;
+    language: string;
+}
+
+export interface DocumentMetadataEdit {
+    editType: CellEditType.DocumentMetadata;
+    metadata: NotebookDocumentMetadata;
+}
+
+export interface CellMoveEdit {
+    editType: CellEditType.Move;
+    index: number;
+    length: number;
+    newIdx: number;
+}
+
+export interface CellReplaceEdit {
+    editType: CellEditType.Replace;
+    index: number;
+    count: number;
+    cells: CellData[];
+}
+
+export type ImmediateCellEditOperation = CellOutputEditByHandle | CellOutputItemEdit | CellPartialInternalMetadataEditByHandle; // add more later on
+export type CellEditOperation = ImmediateCellEditOperation | CellReplaceEdit | CellOutputEdit |
+    CellMetadataEdit | CellLanguageEdit | DocumentMetadataEdit | CellMoveEdit; // add more later on
+
+export type NullablePartialNotebookCellInternalMetadata = {
+    [Key in keyof Partial<NotebookCellInternalMetadata>]: NotebookCellInternalMetadata[Key] | null
+};
+export interface CellPartialInternalMetadataEditByHandle {
+    editType: CellEditType.PartialInternalMetadata;
+    handle: number;
+    internalMetadata: NullablePartialNotebookCellInternalMetadata;
+}
+
+export interface NotebookCellOutputsSplice {
+    start: number;
+    deleteCount: number;
+    newOutputs: CellOutput[];
+};

packages/notebook/src/browser/service/notebook-cell-context-manager.ts

@@ -15,54 +15,59 @@
 // *****************************************************************************
 
 import { inject, injectable } from '@theia/core/shared/inversify';
-import { ContextKeyService } from '@theia/core/lib/browser/context-key-service';
+import { ContextKeyChangeEvent, ContextKeyService, ScopedValueStore } from '@theia/core/lib/browser/context-key-service';
 import { NotebookCellModel } from '../view-model/notebook-cell-model';
 import { NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_TYPE } from '../contributions/notebook-context-keys';
 import { Disposable, DisposableCollection, Emitter } from '@theia/core';
 import { CellKind } from '../../common';
 import { NotebookExecutionStateService } from '../service/notebook-execution-state-service';
 
 @injectable()
-export class NotebookCellContextManager implements Disposable {
+export class NotebookCellContextManager implements NotebookCellContextManager, Disposable {
     @inject(ContextKeyService) protected contextKeyService: ContextKeyService;
 
     @inject(NotebookExecutionStateService)
     protected readonly executionStateService: NotebookExecutionStateService;
 
     protected readonly toDispose = new DisposableCollection();
 
+    protected currentStore: ScopedValueStore;
     protected currentContext: HTMLLIElement;
 
-    protected readonly onDidChangeContextEmitter = new Emitter<void>();
+    protected readonly onDidChangeContextEmitter = new Emitter<ContextKeyChangeEvent>();
     readonly onDidChangeContext = this.onDidChangeContextEmitter.event;
 
     updateCellContext(cell: NotebookCellModel, newHtmlContext: HTMLLIElement): void {
         if (newHtmlContext !== this.currentContext) {
             this.toDispose.dispose();
 
             this.currentContext = newHtmlContext;
-            const currentStore = this.contextKeyService.createScoped(newHtmlContext);
-            this.toDispose.push(currentStore);
+            this.currentStore = this.contextKeyService.createScoped(newHtmlContext);
 
-            currentStore.setContext(NOTEBOOK_CELL_TYPE, cell.cellKind === CellKind.Code ? 'code' : 'markdown');
+            this.currentStore.setContext(NOTEBOOK_CELL_TYPE, cell.cellKind === CellKind.Code ? 'code' : 'markdown');
+
+            this.toDispose.push(this.contextKeyService.onDidChange(e => {
+                this.onDidChangeContextEmitter.fire(e);
+            }));
 
             this.toDispose.push(cell.onDidRequestCellEditChange(cellEdit => {
-                currentStore?.setContext(NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, cellEdit);
-                this.onDidChangeContextEmitter.fire();
+                this.currentStore?.setContext(NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, cellEdit);
+                this.onDidChangeContextEmitter.fire({ affects: keys => keys.has(NOTEBOOK_CELL_MARKDOWN_EDIT_MODE) });
             }));
             this.toDispose.push(this.executionStateService.onDidChangeExecution(e => {
                 if (e.affectsCell(cell.uri)) {
-                    currentStore?.setContext(NOTEBOOK_CELL_EXECUTING, !!e.changed);
-                    currentStore?.setContext(NOTEBOOK_CELL_EXECUTION_STATE, e.changed?.state ?? 'idle');
-                    this.onDidChangeContextEmitter.fire();
+                    this.currentStore?.setContext(NOTEBOOK_CELL_EXECUTING, !!e.changed);
+                    this.currentStore?.setContext(NOTEBOOK_CELL_EXECUTION_STATE, e.changed?.state ?? 'idle');
+                    this.onDidChangeContextEmitter.fire({ affects: keys => keys.has(NOTEBOOK_CELL_EXECUTING) || keys.has(NOTEBOOK_CELL_EXECUTION_STATE) });
                 }
             }));
-            this.onDidChangeContextEmitter.fire();
+            this.onDidChangeContextEmitter.fire({ affects: keys => true });
         }
     }
 
     dispose(): void {
         this.toDispose.dispose();
+        this.currentStore?.dispose();
         this.onDidChangeContextEmitter.dispose();
     }
 }

packages/notebook/src/browser/service/notebook-editor-widget-service.ts

@@ -86,8 +86,8 @@ export class NotebookEditorWidgetService implements Disposable {
         return this.notebookEditors.get(editorId);
     }
 
-    listNotebookEditors(): readonly NotebookEditorWidget[] {
-        return [...this.notebookEditors].map(e => e[1]);
+    getNotebookEditors(): readonly NotebookEditorWidget[] {
+        return Array.from(this.notebookEditors.values());
     }
 
 }

packages/notebook/src/browser/service/notebook-execution-service.ts

@@ -23,11 +23,10 @@ import { CellExecution, NotebookExecutionStateService } from '../service/noteboo
 import { CellKind, NotebookCellExecutionState } from '../../common';
 import { NotebookCellModel } from '../view-model/notebook-cell-model';
 import { NotebookModel } from '../view-model/notebook-model';
-import { NotebookKernelService, NotebookKernel } from './notebook-kernel-service';
+import { NotebookKernelService } from './notebook-kernel-service';
 import { CommandService, Disposable } from '@theia/core';
-import { NotebookKernelQuickPickService, NotebookKernelQuickPickServiceImpl } from './notebook-kernel-quick-pick-service';
+import { NotebookKernelQuickPickService } from './notebook-kernel-quick-pick-service';
 import { NotebookKernelHistoryService } from './notebook-kernel-history-service';
-import { NotebookCommands } from '../contributions/notebook-actions-contribution';
 
 export interface CellExecutionParticipant {
     onWillExecuteCell(executions: CellExecution[]): Promise<void>;
@@ -49,7 +48,7 @@ export class NotebookExecutionService {
     protected commandService: CommandService;
 
     @inject(NotebookKernelQuickPickService)
-    protected notebookKernelQuickPickService: NotebookKernelQuickPickServiceImpl;
+    protected notebookKernelQuickPickService: NotebookKernelQuickPickService;
 
     private readonly cellExecutionParticipants = new Set<CellExecutionParticipant>();
 
@@ -69,7 +68,7 @@ export class NotebookExecutionService {
             }
         }
 
-        const kernel = await this.resolveKernel(notebook);
+        const kernel = await this.notebookKernelHistoryService.resolveSelectedKernel(notebook);
 
         if (!kernel) {
             // clear all pending cell executions
@@ -125,15 +124,4 @@ export class NotebookExecutionService {
         this.cancelNotebookCellHandles(notebook, Array.from(cells, cell => cell.handle));
     }
 
-    async resolveKernel(notebook: NotebookModel): Promise<NotebookKernel | undefined> {
-        const alreadySelected = this.notebookKernelHistoryService.getKernels(notebook);
-
-        if (alreadySelected.selected) {
-            return alreadySelected.selected;
-        }
-
-        await this.commandService.executeCommand(NotebookCommands.SELECT_KERNEL_COMMAND.id, notebook);
-        const { selected } = this.notebookKernelHistoryService.getKernels(notebook);
-        return selected;
-    }
 }

packages/notebook/src/browser/service/notebook-execution-state-service.ts

@@ -23,8 +23,9 @@ import { inject, injectable } from '@theia/core/shared/inversify';
 import { NotebookService } from './notebook-service';
 import {
     CellEditType, CellExecuteOutputEdit, CellExecuteOutputItemEdit, CellExecutionUpdateType,
-    CellUri, CellPartialInternalMetadataEditByHandle, NotebookCellExecutionState, CellEditOperation, NotebookCellInternalMetadata
+    CellUri, NotebookCellExecutionState, NotebookCellInternalMetadata
 } from '../../common';
+import { CellPartialInternalMetadataEditByHandle, CellEditOperation } from '../notebook-types';
 import { NotebookModel } from '../view-model/notebook-model';
 import { v4 } from 'uuid';
 
@@ -43,10 +44,6 @@ export interface CellExecutionStateUpdate {
     isPaused?: boolean;
 }
 
-export interface ICellExecutionComplete {
-    runEndTime?: number;
-    lastRunSuccess?: boolean;
-}
 export enum NotebookExecutionType {
     cell,
     notebook