Notebook API post review improvements

packages/core/src/browser/saveable.ts

@@ -50,7 +50,7 @@ export interface SaveableSource {
     readonly saveable: Saveable;
 }
 
-export class SaveableDelegate implements Saveable {
+export class DelegatingSaveable implements Saveable {
     dirty = false;
     protected readonly onDirtyChangedEmitter = new Emitter<void>();
 
@@ -60,20 +60,20 @@ export class SaveableDelegate implements Saveable {
     autoSave: 'off' | 'afterDelay' | 'onFocusChange' | 'onWindowChange' = 'off';
 
     async save(options?: SaveOptions): Promise<void> {
-        await this.delegate?.save(options);
+        await this._delegate?.save(options);
     }
 
     revert?(options?: Saveable.RevertOptions): Promise<void>;
     createSnapshot?(): Saveable.Snapshot;
     applySnapshot?(snapshot: object): void;
 
-    protected delegate?: Saveable;
+    protected _delegate?: Saveable;
     protected toDispose?: Disposable;
 
-    set(delegate: Saveable): void {
+    set delegate(delegate: Saveable) {
         this.toDispose?.dispose();
-        this.delegate = delegate;
-        this.toDispose = this.delegate.onDirtyChanged(() => {
+        this._delegate = delegate;
+        this.toDispose = delegate.onDirtyChanged(() => {
             this.dirty = delegate.dirty;
             this.onDirtyChangedEmitter.fire();
         });

packages/core/src/common/uri.ts

@@ -27,10 +27,6 @@ export class URI {
         return new URI(Uri.file(path));
     }
 
-    public static isUri(uri: unknown): boolean {
-        return Uri.isUri(uri);
-    }
-
     private readonly codeUri: Uri;
     private _path: Path | undefined;
 

packages/monaco/src/browser/monaco-languages.ts

@@ -85,10 +85,6 @@ export class MonacoLanguages implements LanguageService {
         return this.getLanguage(languageId)?.extensions.values().next().value;
     }
 
-    getLanguageIdByLanguageName(languageName: string): string | undefined {
-        return monaco.languages.getLanguages().find(language => language.aliases?.includes(languageName))?.id;
-    }
-
     protected mergeLanguages(registered: monaco.languages.ILanguageExtensionPoint[]): Map<string, Mutable<Language>> {
         const languages = new Map<string, Mutable<Language>>();
         for (const { id, aliases, extensions, filenames } of registered) {

packages/monaco/src/browser/monaco-code-editor.ts → packages/monaco/src/browser/simple-monaco-editor.ts

@@ -26,7 +26,7 @@ import { Dimension, EditorMouseEvent, MouseTarget, Position, TextDocumentChangeE
 import * as monaco from '@theia/monaco-editor-core';
 import { ElementExt } from '@theia/core/shared/@phosphor/domutils';
 
-export class MonacoCodeEditor extends MonacoEditorServices implements Disposable {
+export class SimpleMonacoEditor extends MonacoEditorServices implements Disposable {
 
     protected editor: CodeEditorWidget;
     protected readonly toDispose = new DisposableCollection();

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

@@ -26,35 +26,42 @@ import { NotebookCellOutputModel } from '../view-model/notebook-cell-output-mode
 import { CellEditType } from '../../common';
 
 export namespace NotebookCellCommands {
+    /** Parameters: notebookModel: NotebookModel | undefined, cell: NotebookCellModel */
     export const EDIT_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.edit',
         iconClass: codicon('edit')
     });
+    /** Parameters: notebookModel: NotebookModel | undefined, cell: NotebookCellModel */
     export const STOP_EDIT_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.stop-edit',
         iconClass: codicon('check')
     });
+    /** Parameters: notebookModel: NotebookModel, cell: NotebookCellModel */
     export const DELETE_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.delete',
         iconClass: codicon('trash')
     });
+    /** Parameters: notebookModel: NotebookModel, cell: NotebookCellModel */
     export const SPLIT_CELL_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.split-cell',
         iconClass: codicon('split-vertical'),
     });
+    /** Parameters: notebookModel: NotebookModel, cell: NotebookCellModel */
     export const EXECUTE_SINGLE_CELL_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.execute-cell',
         iconClass: codicon('play'),
     });
+    /** Parameters: notebookModel: NotebookModel, cell: NotebookCellModel */
     export const STOP_CELL_EXECUTION_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.stop-cell-execution',
         iconClass: codicon('stop'),
     });
-
+    /** Parameters: notebookModel: NotebookModel | undefined, cell: NotebookCellModel */
     export const CLEAR_OUTPUTS_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.clear-outputs',
         label: 'Clear Cell Outputs',
     });
+    /** Parameters: notebookModel: NotebookModel | undefined, cell: NotebookCellModel | undefined, output: NotebookCellOutputModel */
     export const CHANGE_OUTPUT_PRESENTATION_COMMAND = Command.toDefaultLocalizedCommand({
         id: 'notebook.cell.change-presentation',
         label: 'Change Presentation',
@@ -184,7 +191,7 @@ export class NotebookCellActionContribution implements MenuContribution, Command
             execute: (notebookModel: NotebookModel, cell: NotebookCellModel) => this.notebookExecutionService.cancelNotebookCells(notebookModel, [cell])
         });
         commands.registerCommand(NotebookCellCommands.CLEAR_OUTPUTS_COMMAND, {
-            execute: (notebookModel: NotebookModel, cell: NotebookCellModel) => cell.spliceNotebookCellOutputs({ start: 0, deleteCount: cell.outputs.length, newOutputs: [] })
+            execute: (_, cell: NotebookCellModel) => cell.spliceNotebookCellOutputs({ start: 0, deleteCount: cell.outputs.length, newOutputs: [] })
         });
         commands.registerCommand(NotebookCellCommands.CHANGE_OUTPUT_PRESENTATION_COMMAND, {
             execute: (_, __, output: NotebookCellOutputModel) => output.requestOutputPresentationUpdate()

packages/notebook/src/browser/contributions/notebook-context-keys.ts

@@ -13,11 +13,18 @@
 //
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
 // *****************************************************************************
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
 
 import { ContextKeyService } from '@theia/core/lib/browser/context-key-service';
 
 export type NotebookCellExecutionStateContext = 'idle' | 'pending' | 'executing' | 'succeeded' | 'failed';
 
+/**
+ * Context Keys for the Notebook Editor as defined by vscode in https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts
+ */
 export const HAS_OPENED_NOTEBOOK = 'userHasOpenedNotebook';
 export const KEYBINDING_CONTEXT_NOTEBOOK_FIND_WIDGET_FOCUSED = 'notebookFindWidgetFocused';
 export const NOTEBOOK_EDITOR_FOCUSED = 'notebookEditorFocused';

packages/notebook/src/browser/index.ts

@@ -18,7 +18,7 @@ export * from './notebook-type-registry';
 export * from './notebook-renderer-registry';
 export * from './notebook-editor-widget';
 export * from './service/notebook-service';
-export * from './service/notebook-editor-service';
+export * from './service/notebook-editor-widget-service';
 export * from './service/notebook-kernel-service';
 export * from './service/notebook-execution-state-service';
 export * from './service/notebook-model-resolver-service';

packages/notebook/src/browser/notebook-cell-resource-resolver.ts

@@ -14,7 +14,7 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import { Emitter, Resource, ResourceReadOptions, ResourceResolver, ResourceVersion, URI } from '@theia/core';
+import { Emitter, Resource, ResourceReadOptions, ResourceResolver, URI } from '@theia/core';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { CellUri } from '../common';
 import { NotebookService } from './service/notebook-service';
@@ -25,10 +25,6 @@ export class NotebookCellResource implements Resource {
     protected readonly didChangeContentsEmitter = new Emitter<void>();
     readonly onDidChangeContents = this.didChangeContentsEmitter.event;
 
-    version?: ResourceVersion | undefined;
-    encoding?: string | undefined;
-    isReadonly?: boolean | undefined;
-
     private cell: NotebookCellModel;
 
     constructor(public uri: URI, cell: NotebookCellModel) {

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

@@ -17,7 +17,7 @@
 import { URI } from '@theia/core';
 import { WidgetFactory, NavigatableWidgetOptions, LabelProvider } from '@theia/core/lib/browser';
 import { inject, injectable } from '@theia/core/shared/inversify';
-import { NotebookEditorWidget, NotebookEditorContainerFactory, NotebookEditorProps } from './notebook-editor-widget';
+import { NotebookEditorWidget, NotebookEditorWidgetContainerFactory, NotebookEditorProps } from './notebook-editor-widget';
 import { NotebookService } from './service/notebook-service';
 import { NotebookModelResolverService } from './service/notebook-model-resolver-service';
 
@@ -38,7 +38,7 @@ export class NotebookEditorWidgetFactory implements WidgetFactory {
     @inject(LabelProvider)
     protected readonly labelProvider: LabelProvider;
 
-    @inject(NotebookEditorContainerFactory)
+    @inject(NotebookEditorWidgetContainerFactory)
     protected readonly createNotebookEditorWidget: (props: NotebookEditorProps) => NotebookEditorWidget;
 
     async createWidget(options?: NotebookEditorWidgetOptions): Promise<NotebookEditorWidget> {

packages/notebook/src/browser/notebook-editor-widget.tsx

@@ -16,20 +16,20 @@
 
 import * as React from '@theia/core/shared/react';
 import { CommandRegistry, MenuModelRegistry, URI } from '@theia/core';
-import { ReactWidget, Navigatable, SaveableSource, Message, SaveableDelegate } from '@theia/core/lib/browser';
+import { ReactWidget, Navigatable, SaveableSource, Message, DelegatingSaveable } from '@theia/core/lib/browser';
 import { ReactNode } from '@theia/core/shared/react';
 import { CellKind } from '../common';
 import { CellRenderer as CellRenderer, NotebookCellListView } from './view/notebook-cell-list-view';
 import { NotebookCodeCellRenderer } from './view/notebook-code-cell-view';
 import { NotebookMarkdownCellRenderer } from './view/notebook-markdown-cell-view';
 import { NotebookModel } from './view-model/notebook-model';
 import { NotebookCellToolbarFactory } from './view/notebook-cell-toolbar-factory';
-import { inject, injectable, interfaces } from '@theia/core/shared/inversify';
+import { inject, injectable, interfaces, postConstruct } from '@theia/core/shared/inversify';
 import { Emitter } from '@theia/core/shared/vscode-languageserver-protocol';
-import { NotebookEditorWidgetService } from './service/notebook-editor-service';
+import { NotebookEditorWidgetService } from './service/notebook-editor-widget-service';
 import { NotebookMainToolbarRenderer } from './view/notebook-main-toolbar';
 
-export const NotebookEditorContainerFactory = Symbol('NotebookModelFactory');
+export const NotebookEditorWidgetContainerFactory = Symbol('NotebookEditorWidgetContainerFactory');
 
 export function createNotebookEditorWidgetContainer(parent: interfaces.Container, props: NotebookEditorProps): interfaces.Container {
     const child = parent.createChild();
@@ -53,7 +53,7 @@ export const NOTEBOOK_EDITOR_ID_PREFIX = 'notebook:';
 export class NotebookEditorWidget extends ReactWidget implements Navigatable, SaveableSource {
     static readonly ID = 'notebook';
 
-    readonly saveable = new SaveableDelegate();
+    readonly saveable = new DelegatingSaveable();
 
     @inject(NotebookCellToolbarFactory)
     protected readonly cellToolbarFactory: NotebookCellToolbarFactory;
@@ -70,6 +70,13 @@ export class NotebookEditorWidget extends ReactWidget implements Navigatable, Sa
     @inject(NotebookMainToolbarRenderer)
     protected notebookMainToolbarRenderer: NotebookMainToolbarRenderer;
 
+    @inject(NotebookCodeCellRenderer)
+    protected codeCellRenderer: NotebookCodeCellRenderer;
+    @inject(NotebookMarkdownCellRenderer)
+    protected markdownCellRenderer: NotebookMarkdownCellRenderer;
+    @inject(NotebookEditorProps)
+    protected readonly props: NotebookEditorProps;
+
     protected readonly onDidChangeModelEmitter = new Emitter<void>();
     readonly onDidChangeModel = this.onDidChangeModelEmitter.event;
 
@@ -84,11 +91,8 @@ export class NotebookEditorWidget extends ReactWidget implements Navigatable, Sa
         return this._model;
     }
 
-    constructor(
-        @inject(NotebookCodeCellRenderer) codeCellRenderer: NotebookCodeCellRenderer,
-        @inject(NotebookMarkdownCellRenderer) markdownCellRenderer: NotebookMarkdownCellRenderer,
-        @inject(NotebookEditorProps) private readonly props: NotebookEditorProps) {
-        super();
+    @postConstruct()
+    protected init(): void {
         this.id = NOTEBOOK_EDITOR_ID_PREFIX + this.props.uri.toString();
         this.node.tabIndex = -1;
 
@@ -97,14 +101,15 @@ export class NotebookEditorWidget extends ReactWidget implements Navigatable, Sa
 
         this.toDispose.push(this.onDidChangeModelEmitter);
 
-        this.renderers.set(CellKind.Markup, markdownCellRenderer);
-        this.renderers.set(CellKind.Code, codeCellRenderer);
+        this.renderers.set(CellKind.Markup, this.markdownCellRenderer);
+        this.renderers.set(CellKind.Code, this.codeCellRenderer);
         this.waitForData();
+
     }
 
     protected async waitForData(): Promise<void> {
         this._model = await this.props.notebookData;
-        this.saveable.set(this._model);
+        this.saveable.delegate = this._model;
         this.toDispose.push(this._model);
         // Ensure that the model is loaded before adding the editor
         this.notebookEditorService.addNotebookEditor(this);

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

@@ -30,7 +30,7 @@ import { NotebookCellActionContribution } from './contributions/notebook-cell-ac
 import { NotebookCellToolbarFactory } from './view/notebook-cell-toolbar-factory';
 import { createNotebookModelContainer, NotebookModel, NotebookModelFactory, NotebookModelProps } from './view-model/notebook-model';
 import { createNotebookCellModelContainer, NotebookCellModel, NotebookCellModelFactory, NotebookCellModelProps } from './view-model/notebook-cell-model';
-import { createNotebookEditorWidgetContainer, NotebookEditorContainerFactory, NotebookEditorProps, NotebookEditorWidget } from './notebook-editor-widget';
+import { createNotebookEditorWidgetContainer, NotebookEditorWidgetContainerFactory, NotebookEditorProps, NotebookEditorWidget } from './notebook-editor-widget';
 import { NotebookCodeCellRenderer } from './view/notebook-code-cell-view';
 import { NotebookMarkdownCellRenderer } from './view/notebook-markdown-cell-view';
 import { NotebookActionsContribution } from './contributions/notebook-actions-contribution';
@@ -39,7 +39,7 @@ import { NotebookExecutionStateService } from './service/notebook-execution-stat
 import { NotebookKernelService } from './service/notebook-kernel-service';
 import { KernelPickerMRUStrategy, NotebookKernelQuickPickService } from './service/notebook-kernel-quick-pick-service';
 import { NotebookKernelHistoryService } from './service/notebook-kernel-history-service';
-import { NotebookEditorWidgetService } from './service/notebook-editor-service';
+import { NotebookEditorWidgetService } from './service/notebook-editor-widget-service';
 import { NotebookRendererMessagingService } from './service/notebook-renderer-messaging-service';
 import { NotebookColorContribution } from './contributions/notebook-color-contribution';
 import { NotebookCellContextManager } from './service/notebook-cell-context-manager';
@@ -83,7 +83,7 @@ export default new ContainerModule(bind => {
     bind(NotebookMarkdownCellRenderer).toSelf().inSingletonScope();
     bind(NotebookMainToolbarRenderer).toSelf().inSingletonScope();
 
-    bind(NotebookEditorContainerFactory).toFactory(ctx => (props: NotebookEditorProps) =>
+    bind(NotebookEditorWidgetContainerFactory).toFactory(ctx => (props: NotebookEditorProps) =>
         createNotebookEditorWidgetContainer(ctx.container, props).get(NotebookEditorWidget)
     );
     bind(NotebookModelFactory).toFactory(ctx => (props: NotebookModelProps) =>

packages/notebook/src/browser/notebook-open-handler.ts

@@ -28,9 +28,8 @@ export class NotebookOpenHandler extends NavigatableWidgetOpenHandler<NotebookEd
 
     id: string = 'notebook';
 
-    constructor(@inject(NotebookTypeRegistry) private notebookTypeRegistry: NotebookTypeRegistry) {
-        super();
-    }
+    @inject(NotebookTypeRegistry)
+    protected notebookTypeRegistry: NotebookTypeRegistry;
 
     canHandle(uri: URI, options?: WidgetOpenerOptions | undefined): MaybePromise<number> {
         const priorities = this.notebookTypeRegistry.notebookTypes
@@ -59,7 +58,7 @@ export class NotebookOpenHandler extends NavigatableWidgetOpenHandler<NotebookEd
 
     protected calculatePriority(notebookType: NotebookTypeDescriptor | undefined): number {
         if (!notebookType) {
-            return -1;
+            return 0;
         }
         return notebookType.priority === 'option' ? 100 : 200;
     }

packages/notebook/src/browser/notebook-renderer-registry.ts

@@ -33,7 +33,11 @@ export interface NotebookRendererInfo {
 @injectable()
 export class NotebookRendererRegistry {
 
-    readonly notebookRenderers: NotebookRendererInfo[] = [];
+    private readonly _notebookRenderers: NotebookRendererInfo[] = [];
+
+    get notebookRenderers(): readonly NotebookRendererInfo[] {
+        return this._notebookRenderers;
+    }
 
     registerNotebookRenderer(type: NotebookRendererDescriptor, basePath: string): Disposable {
         let entrypoint;
@@ -48,14 +52,14 @@ export class NotebookRendererRegistry {
             };
         }
 
-        this.notebookRenderers.push({
+        this._notebookRenderers.push({
             ...type,
             mimeTypes: type.mimeTypes || [],
             requiresMessaging: type.requiresMessaging === 'always' || type.requiresMessaging === 'optional',
             entrypoint
         });
         return Disposable.create(() => {
-            this.notebookRenderers.splice(this.notebookRenderers.findIndex(renderer => renderer.id === type.id), 1);
+            this._notebookRenderers.splice(this._notebookRenderers.findIndex(renderer => renderer.id === type.id), 1);
         });
     }
 }

packages/notebook/src/browser/notebook-type-registry.ts

@@ -19,12 +19,16 @@ import { NotebookTypeDescriptor } from '../common/notebook-protocol';
 
 @injectable()
 export class NotebookTypeRegistry {
-    readonly notebookTypes: NotebookTypeDescriptor[] = [];
+    private readonly _notebookTypes: NotebookTypeDescriptor[] = [];
+
+    get notebookTypes(): readonly NotebookTypeDescriptor[] {
+        return this._notebookTypes;
+    }
 
     registerNotebookType(type: NotebookTypeDescriptor): Disposable {
-        this.notebookTypes.push(type);
+        this._notebookTypes.push(type);
         return Disposable.create(() => {
-            this.notebookTypes.splice(this.notebookTypes.indexOf(type), 1);
+            this._notebookTypes.splice(this._notebookTypes.indexOf(type), 1);
         });
     }
 }

packages/notebook/src/browser/renderers/cell-output-webview.ts

@@ -25,7 +25,7 @@ export interface CellOutputWebview extends Disposable {
 
     readonly id: string;
 
-    render(): React.JSX.Element;
+    render(): React.ReactNode;
 
     attachWebview(): void;
     isAttached(): boolean