From 921c1f9580a4c02897353e4234335704e85260f1 Mon Sep 17 00:00:00 2001 From: Jonah Iden Date: Wed, 27 Sep 2023 13:34:54 +0200 Subject: [PATCH] more review comments Signed-off-by: Jonah Iden --- packages/core/src/common/uri.ts | 4 ---- .../notebook-cell-actions-contribution.ts | 16 ++++++++-------- .../service/notebook-editor-widget-service.ts | 4 ++-- .../notebook-kernel-quick-pick-service.ts | 4 ++-- .../src/browser/service/notebook-service.ts | 2 +- .../src/browser/view-model/notebook-model.ts | 2 +- .../plugin-ext/src/plugin/notebook/notebooks.ts | 3 +-- 7 files changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/core/src/common/uri.ts b/packages/core/src/common/uri.ts index ead51e974d20d..fe24bc2e54339 100644 --- a/packages/core/src/common/uri.ts +++ b/packages/core/src/common/uri.ts @@ -27,10 +27,6 @@ export class URI { return new URI(Uri.file(path)); } - public static isUri(uri: unknown): uri is URI { - return Uri.isUri(uri); - } - private readonly codeUri: Uri; private _path: Path | undefined; 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 908da9d70587f..39071c329623d 100644 --- a/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts +++ b/packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts @@ -26,42 +26,42 @@ import { NotebookCellOutputModel } from '../view-model/notebook-cell-output-mode import { CellEditType } from '../../common'; export namespace NotebookCellCommands { - // Parameters: notebookModel: NotebookModel | undefined, cell: NotebookCellModel + /** 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 + /** 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 + /** Parameters: notebookModel: NotebookModel, cell: NotebookCellModel */ export const DELETE_COMMAND = Command.toDefaultLocalizedCommand({ id: 'notebook.cell.delete', iconClass: codicon('trash') }); - // Parameters: notebookModel: NotebookModel, cell: NotebookCellModel + /** 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 + /** 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 + /** 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 + /** 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 + /** 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', diff --git a/packages/notebook/src/browser/service/notebook-editor-widget-service.ts b/packages/notebook/src/browser/service/notebook-editor-widget-service.ts index 7e233d6ad29d9..f8307cdb9bd6f 100644 --- a/packages/notebook/src/browser/service/notebook-editor-widget-service.ts +++ b/packages/notebook/src/browser/service/notebook-editor-widget-service.ts @@ -67,7 +67,7 @@ export class NotebookEditorWidgetService implements Disposable { addNotebookEditor(editor: NotebookEditorWidget): void { if (this.notebookEditors.has(editor.id)) { - console.log('WARN: notebook editor already added previously: ' + editor.id); + console.warn('Attempting to add duplicated notebook editor: ' + editor.id); } this.notebookEditors.set(editor.id, editor); this.onNotebookEditorAddEmitter.fire(editor); @@ -78,7 +78,7 @@ export class NotebookEditorWidgetService implements Disposable { this.notebookEditors.delete(editor.id); this.onNotebookEditorRemoveEmitter.fire(editor); } else { - console.log('WARN: trying to remove not registered editor: ' + editor.id); + console.warn('Attempting to remove not registered editor: ' + editor.id); } } diff --git a/packages/notebook/src/browser/service/notebook-kernel-quick-pick-service.ts b/packages/notebook/src/browser/service/notebook-kernel-quick-pick-service.ts index c96d266d82b5e..c1acd73bfa76b 100644 --- a/packages/notebook/src/browser/service/notebook-kernel-quick-pick-service.ts +++ b/packages/notebook/src/browser/service/notebook-kernel-quick-pick-service.ts @@ -18,7 +18,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ - +import { URI as Uri } from '@theia/core/shared/vscode-uri'; import { ArrayUtils, Command, CommandService, DisposableCollection, Event, nls, QuickInputButton, QuickInputService, QuickPickInput, QuickPickItem, URI, } from '@theia/core'; import { inject, injectable } from '@theia/core/shared/inversify'; import { NotebookKernelService, NotebookKernel, NotebookKernelMatchResult, SourceCommand } from './notebook-kernel-service'; @@ -316,7 +316,7 @@ export class KernelPickerMRUStrategy extends NotebookKernelQuickPickServiceImpl quickPick.onDidTriggerItemButton(async e => { if (isKernelSourceQuickPickItem(e.item) && e.item.documentation !== undefined) { - const uri: URI | undefined = URI.isUri(e.item.documentation) ? new URI(e.item.documentation) : await this.commandService.executeCommand(e.item.documentation); + const uri: URI | undefined = Uri.isUri(e.item.documentation) ? new URI(e.item.documentation) : await this.commandService.executeCommand(e.item.documentation); if (uri) { (await this.openerService.getOpener(uri, { openExternal: true })).open(uri, { openExternal: true }); } diff --git a/packages/notebook/src/browser/service/notebook-service.ts b/packages/notebook/src/browser/service/notebook-service.ts index 76faf40079614..f8a8d48c3b21d 100644 --- a/packages/notebook/src/browser/service/notebook-service.ts +++ b/packages/notebook/src/browser/service/notebook-service.ts @@ -135,7 +135,7 @@ export class NotebookService implements Disposable { if (this.notebookProviders.has(type)) { return this.notebookProviders.get(type); } - await this.willUseNotebookSerializerEmitter.sequence(async listener => listener(type)); + await Promise.all(this.willUseNotebookSerializerEmitter.fire(type)); const deferred = new Deferred(); // 20 seconds of timeout const timeoutDuration = 20_000; diff --git a/packages/notebook/src/browser/view-model/notebook-model.ts b/packages/notebook/src/browser/view-model/notebook-model.ts index 05c0cd3b3d819..cf1f2359d6b1f 100644 --- a/packages/notebook/src/browser/view-model/notebook-model.ts +++ b/packages/notebook/src/browser/view-model/notebook-model.ts @@ -75,7 +75,7 @@ export class NotebookModel implements Saveable, Disposable { protected props: NotebookModelProps; @inject(MonacoTextModelService) - modelService: MonacoTextModelService; + protected modelService: MonacoTextModelService; @inject(NotebookCellModelFactory) protected cellModelFactory: NotebookCellModelFactory; diff --git a/packages/plugin-ext/src/plugin/notebook/notebooks.ts b/packages/plugin-ext/src/plugin/notebook/notebooks.ts index 17e80a8bd844b..773c9c3ba081b 100644 --- a/packages/plugin-ext/src/plugin/notebook/notebooks.ts +++ b/packages/plugin-ext/src/plugin/notebook/notebooks.ts @@ -346,8 +346,7 @@ export class NotebooksExtImpl implements NotebooksExt { } async showNotebookDocument(notebookOrUri: theia.NotebookDocument | TheiaURI, options?: theia.NotebookDocumentShowOptions): Promise { - - if (URI.isUri(notebookOrUri)) { + if (TheiaURI.isUri(notebookOrUri)) { notebookOrUri = await this.openNotebookDocument(notebookOrUri as TheiaURI); }