Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for creating new notebooks #13696

Merged
merged 2 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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([{
Expand All @@ -138,7 +139,7 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon
count: 0,
cells: [{
cellKind,
language: firstCodeCell?.language ?? 'markdown',
language: cellLanguage,
source: '',
outputs: [],
metadata: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -28,6 +28,7 @@ import { match } from '@theia/core/lib/common/glob';
export interface UntitledResource {
untitledResource: URI | undefined
}

@injectable()
export class NotebookModelResolverService {

Expand All @@ -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) {
Expand Down Expand Up @@ -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');
Expand All @@ -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 {
Expand Down
35 changes: 19 additions & 16 deletions packages/notebook/src/browser/service/notebook-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

Expand All @@ -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(() => {
Expand All @@ -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;
}
Expand Down
31 changes: 11 additions & 20 deletions packages/notebook/src/browser/view-model/notebook-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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> {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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())));
Expand Down Expand Up @@ -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();
}

Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();

Expand All @@ -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.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
Loading
Loading