Improve support for creating new notebooks (#13696)

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

@@ -120,16 +120,17 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon
                 notebookModel = notebookModel ?? this.notebookEditorWidgetService.focusedEditor?.model;
 
                 let insertIndex: number = 0;
-                if (index && index >= 0) {
-                    insertIndex = index as number;
+                if (typeof index === 'number' && index >= 0) {
+                    insertIndex = index;
                 } else if (notebookModel.selectedCell && typeof index === 'string') {
                     // if index is -1 insert below otherwise at the index of the selected cell which is above the selected.
                     insertIndex = notebookModel.cells.indexOf(notebookModel.selectedCell) + (index === 'below' ? 1 : 0);
                 }
 
-                let firstCodeCell;
+                let cellLanguage: string = 'markdown';
                 if (cellKind === CellKind.Code) {
-                    firstCodeCell = notebookModel.cells.find(cell => cell.cellKind === CellKind.Code);
+                    const firstCodeCell = notebookModel.cells.find(cell => cell.cellKind === CellKind.Code);
+                    cellLanguage = firstCodeCell?.language ?? 'plaintext';
                 }
 
                 notebookModel.applyEdits([{
@@ -138,7 +139,7 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon
                     count: 0,
                     cells: [{
                         cellKind,
-                        language: firstCodeCell?.language ?? 'markdown',
+                        language: cellLanguage,
                         source: '',
                         outputs: [],
                         metadata: {},

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

@@ -61,16 +61,15 @@ export class NotebookEditorWidgetFactory implements WidgetFactory {
         return editor;
     }
 
-    private async createEditor(uri: URI, notebookType: string): Promise<NotebookEditorWidget> {
-
+    protected async createEditor(uri: URI, notebookType: string): Promise<NotebookEditorWidget> {
         return this.createNotebookEditorWidget({
             uri,
             notebookType,
             notebookData: this.notebookModelResolver.resolve(uri, notebookType),
         });
     }
 
-    private setLabels(editor: NotebookEditorWidget, uri: URI): void {
+    protected setLabels(editor: NotebookEditorWidget, uri: URI): void {
         editor.title.caption = uri.path.fsPath();
         if (editor.model?.readOnly) {
             editor.title.caption += ` • ${nls.localizeByDefault('Read-only')}`;

packages/notebook/src/browser/service/notebook-model-resolver-service.ts

@@ -14,11 +14,11 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import { Emitter, Resource, ResourceProvider, URI } from '@theia/core';
+import { Emitter, Resource, ResourceProvider, UNTITLED_SCHEME, URI } from '@theia/core';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { UriComponents } from '@theia/core/lib/common/uri';
 import { FileService } from '@theia/filesystem/lib/browser/file-service';
-import { CellKind, NotebookData } from '../../common';
+import { NotebookData } from '../../common';
 import { NotebookModel } from '../view-model/notebook-model';
 import { NotebookService } from './notebook-service';
 import { NotebookTypeRegistry } from '../notebook-type-registry';
@@ -28,6 +28,7 @@ import { match } from '@theia/core/lib/common/glob';
 export interface UntitledResource {
     untitledResource: URI | undefined
 }
+
 @injectable()
 export class NotebookModelResolverService {
 
@@ -49,14 +50,15 @@ export class NotebookModelResolverService {
     readonly onDidSaveNotebook = this.onDidSaveNotebookEmitter.event;
 
     async resolve(resource: URI, viewType?: string): Promise<NotebookModel> {
-
+        const existingModel = this.notebookService.getNotebookEditorModel(resource);
         if (!viewType) {
-            const existingViewType = this.notebookService.getNotebookEditorModel(resource)?.viewType;
-            if (existingViewType) {
-                viewType = existingViewType;
+            if (existingModel) {
+                return existingModel;
             } else {
                 viewType = this.findViewTypeForResource(resource);
             }
+        } else if (existingModel?.viewType === viewType) {
+            return existingModel;
         }
 
         if (!viewType) {
@@ -86,15 +88,15 @@ export class NotebookModelResolverService {
             const suffix = this.getPossibleFileEnding(notebookTypeInfo.selector ?? []) ?? '';
             for (let counter = 1; ; counter++) {
                 const candidate = new URI()
-                    .withScheme('untitled')
+                    .withScheme(UNTITLED_SCHEME)
                     .withPath(`Untitled-notebook-${counter}${suffix}`)
                     .withQuery(viewType);
                 if (!this.notebookService.getNotebookEditorModel(candidate)) {
                     resource = candidate;
                     break;
                 }
             }
-        } else if (arg.untitledResource.scheme === 'untitled') {
+        } else if (arg.untitledResource.scheme === UNTITLED_SCHEME) {
             resource = arg.untitledResource;
         } else {
             throw new Error('Invalid untitled resource: ' + arg.untitledResource.toString() + ' untitled resources with associated file path are not supported yet');
@@ -108,16 +110,8 @@ export class NotebookModelResolverService {
 
     protected async resolveExistingNotebookData(resource: Resource, viewType: string): Promise<NotebookData> {
         if (resource.uri.scheme === 'untitled') {
-
             return {
-                cells: [
-                    {
-                        cellKind: CellKind.Markup,
-                        language: 'markdown',
-                        outputs: [],
-                        source: ''
-                    }
-                ],
+                cells: [],
                 metadata: {}
             };
         } else {

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

@@ -110,11 +110,8 @@ export class NotebookService implements Disposable {
     }
 
     async createNotebookModel(data: NotebookData, viewType: string, resource: Resource): Promise<NotebookModel> {
-        const serializer = this.notebookProviders.get(viewType)?.serializer;
-        if (!serializer) {
-            throw new Error('no notebook serializer for ' + viewType);
-        }
-
+        const dataProvider = await this.getNotebookDataProvider(viewType);
+        const serializer = dataProvider.serializer;
         const model = this.notebookModelFactory({ data, resource, viewType, serializer });
         this.notebookModels.set(resource.uri.toString(), model);
         // Resolve cell text models right after creating the notebook model
@@ -129,25 +126,24 @@ export class NotebookService implements Disposable {
     }
 
     async getNotebookDataProvider(viewType: string): Promise<NotebookProviderInfo> {
-        await this.ready.promise;
-
-        const result = await this.waitForNotebookProvider(viewType);
-        if (!result) {
+        try {
+            return await this.waitForNotebookProvider(viewType);
+        } catch {
             throw new Error(`No provider registered for view type: '${viewType}'`);
         }
-        return result;
     }
 
     /**
      * When the application starts up, notebook providers from plugins are not registered yet.
      * It takes a few seconds for the plugin host to start so that notebook data providers can be registered.
      * This methods waits until the notebook provider is registered.
      */
-    protected async waitForNotebookProvider(type: string): Promise<NotebookProviderInfo | undefined> {
-        if (this.notebookProviders.has(type)) {
-            return this.notebookProviders.get(type);
+    protected waitForNotebookProvider(type: string): Promise<NotebookProviderInfo> {
+        const existing = this.notebookProviders.get(type);
+        if (existing) {
+            return Promise.resolve(existing);
         }
-        const deferred = new Deferred<NotebookProviderInfo | undefined>();
+        const deferred = new Deferred<NotebookProviderInfo>();
         // 20 seconds of timeout
         const timeoutDuration = 20_000;
 
@@ -161,7 +157,12 @@ export class NotebookService implements Disposable {
             if (viewType === type) {
                 clearTimeout(timeout);
                 disposable.dispose();
-                deferred.resolve(this.notebookProviders.get(type));
+                const newProvider = this.notebookProviders.get(type);
+                if (!newProvider) {
+                    deferred.reject(new Error(`Notebook provider for type ${type} is invalid`));
+                } else {
+                    deferred.resolve(newProvider);
+                }
             }
         });
         timeout = setTimeout(() => {
@@ -170,7 +171,9 @@ export class NotebookService implements Disposable {
             deferred.reject(new Error(`Timed out while waiting for notebook serializer for type ${type} to be registered`));
         }, timeoutDuration);
 
-        await Promise.all(this.willUseNotebookSerializerEmitter.fire(type));
+        this.ready.promise.then(() => {
+            this.willUseNotebookSerializerEmitter.fire(type);
+        });
 
         return deferred.promise;
     }

packages/notebook/src/browser/view-model/notebook-model.ts

@@ -156,7 +156,7 @@ export class NotebookModel implements Saveable, Disposable {
         this.onDidDisposeEmitter.fire();
     }
 
-    async save(options: SaveOptions): Promise<void> {
+    async save(options?: SaveOptions): Promise<void> {
         this.dirtyCells = [];
         this.dirty = false;
 
@@ -186,25 +186,8 @@ export class NotebookModel implements Saveable, Disposable {
         if (!rawData) {
             throw new Error('could not read notebook snapshot');
         }
-        const data = JSON.parse(rawData);
-        const cells = data.cells.map((cell: CellData, index: number) => {
-            const handle = this.nextHandle++;
-            return this.cellModelFactory({
-                uri: CellUri.generate(this.uri, handle),
-                handle: handle,
-                source: cell.source,
-                language: cell.language,
-                cellKind: cell.cellKind,
-                outputs: cell.outputs,
-                metadata: cell.metadata,
-                internalMetadata: cell.internalMetadata,
-                collapseState: cell.collapseState
-            });
-        });
-        this.addCellOutputListeners(cells);
-
-        this.metadata = data.metadata;
-
+        const data = JSON.parse(rawData) as NotebookData;
+        this.setData(data);
     }
 
     async revert(options?: Saveable.RevertOptions): Promise<void> {
@@ -229,6 +212,14 @@ export class NotebookModel implements Saveable, Disposable {
         }
     }
 
+    setData(data: NotebookData): void {
+        // Replace all cells in the model
+        this.replaceCells(0, this.cells.length, data.cells, false);
+        this.metadata = data.metadata;
+        this.dirty = false;
+        this.onDidChangeContentEmitter.fire();
+    }
+
     undo(): void {
         // TODO we probably need to check if a monaco editor is focused and if so, not undo
         this.undoRedoService.undo(this.uri);

packages/plugin-ext/src/main/browser/notebooks/notebook-documents-main.ts

@@ -25,6 +25,7 @@ import { MAIN_RPC_CONTEXT, NotebookCellsChangedEventDto, NotebookDataDto, Notebo
 import { RPCProtocol } from '../../../common/rpc-protocol';
 import { NotebookDto } from './notebook-dto';
 import { MonacoEditorModel } from '@theia/monaco/lib/browser/monaco-editor-model';
+import { NotebookOpenHandler } from '@theia/notebook/lib/browser/notebook-open-handler';
 
 export class NotebookDocumentsMainImpl implements NotebookDocumentsMain {
 
@@ -35,14 +36,16 @@ export class NotebookDocumentsMainImpl implements NotebookDocumentsMain {
 
     protected readonly notebookModelResolverService: NotebookModelResolverService;
 
-    protected notebookMonacoTextModelService: NotebookMonacoTextModelService;
+    protected readonly notebookMonacoTextModelService: NotebookMonacoTextModelService;
+    protected readonly notebookOpenHandler: NotebookOpenHandler;
 
     constructor(
         rpc: RPCProtocol,
         container: interfaces.Container
     ) {
         this.proxy = rpc.getProxy(MAIN_RPC_CONTEXT.NOTEBOOK_DOCUMENTS_EXT);
         this.notebookModelResolverService = container.get(NotebookModelResolverService);
+        this.notebookOpenHandler = container.get(NotebookOpenHandler);
 
         // forward dirty and save events
         this.disposables.push(this.notebookModelResolverService.onDidChangeDirty(model => this.proxy.$acceptDirtyStateChanged(model.uri.toComponents(), model.isDirty())));
@@ -157,10 +160,10 @@ export class NotebookDocumentsMainImpl implements NotebookDocumentsMain {
         this.proxy.$acceptDirtyStateChanged(ref.uri.toComponents(), true);
 
         // apply content changes... slightly HACKY -> this triggers a change event
-        // if (options.content) {
-        //     const data = NotebookDto.fromNotebookDataDto(options.content);
-        //     ref.notebook.reset(data.cells, data.metadata, ref.object.notebook.transientOptions);
-        // }
+        if (options.content) {
+            const data = NotebookDto.fromNotebookDataDto(options.content);
+            ref.setData(data);
+        }
         return ref.uri.toComponents();
     }
 
@@ -171,9 +174,8 @@ export class NotebookDocumentsMainImpl implements NotebookDocumentsMain {
 
     async $trySaveNotebook(uriComponents: UriComponents): Promise<boolean> {
         const uri = URI.fromComponents(uriComponents);
-
         const ref = await this.notebookModelResolverService.resolve(uri);
-        await ref.save({});
+        await ref.save();
         ref.dispose();
         return true;
     }

packages/plugin-ext/src/main/browser/notebooks/notebook-editors-main.ts

@@ -20,16 +20,17 @@
 
 import { UriComponents, URI } from '@theia/core/lib/common/uri';
 import { CellRange } from '@theia/notebook/lib/common';
-import { NotebookEditorWidget } from '@theia/notebook/lib/browser';
+import { NotebookEditorWidget, NotebookService } from '@theia/notebook/lib/browser';
 import { MAIN_RPC_CONTEXT, NotebookDocumentShowOptions, NotebookEditorRevealType, NotebookEditorsExt, NotebookEditorsMain } from '../../../common';
 import { RPCProtocol } from '../../../common/rpc-protocol';
 import { interfaces } from '@theia/core/shared/inversify';
-import { open, OpenerService } from '@theia/core/lib/browser';
+import { NotebookOpenHandler } from '@theia/notebook/lib/browser/notebook-open-handler';
 
 export class NotebookEditorsMainImpl implements NotebookEditorsMain {
 
     protected readonly proxy: NotebookEditorsExt;
-    protected readonly openerService: OpenerService;
+    protected readonly notebookService: NotebookService;
+    protected readonly notebookOpenHandler: NotebookOpenHandler;
 
     protected readonly mainThreadEditors = new Map<string, NotebookEditorWidget>();
 
@@ -38,12 +39,16 @@ export class NotebookEditorsMainImpl implements NotebookEditorsMain {
         container: interfaces.Container
     ) {
         this.proxy = rpc.getProxy(MAIN_RPC_CONTEXT.NOTEBOOK_EDITORS_EXT);
-        this.openerService = container.get(OpenerService);
+        this.notebookService = container.get(NotebookService);
+        this.notebookOpenHandler = container.get(NotebookOpenHandler);
     }
 
     async $tryShowNotebookDocument(uriComponents: UriComponents, viewType: string, options: NotebookDocumentShowOptions): Promise<string> {
-        const editor = await open(this.openerService, URI.fromComponents(uriComponents), {});
-        return (editor as NotebookEditorWidget).id;
+        const editor = await this.notebookOpenHandler.open(URI.fromComponents(uriComponents), {
+            notebookType: viewType
+        });
+        await editor.ready;
+        return editor.id;
     }
     $tryRevealRange(id: string, range: CellRange, revealType: NotebookEditorRevealType): Promise<void> {
         throw new Error('Method not implemented.');

packages/plugin-ext/src/main/browser/notebooks/notebooks-main.ts

@@ -58,7 +58,7 @@ export class NotebooksMainImpl implements NotebooksMain {
         const plugins = container.get(HostedPluginSupport);
 
         this.proxy = rpc.getProxy(MAIN_RPC_CONTEXT.NOTEBOOKS_EXT);
-        this.notebookService.onWillUseNotebookSerializer(async event => plugins.activateByNotebookSerializer(event));
+        this.notebookService.onWillUseNotebookSerializer(event => plugins.activateByNotebookSerializer(event));
         this.notebookService.markReady();
         commands.registerArgumentProcessor({
             processArgument: arg => {