more review comments

Signed-off-by: Jonah Iden <[email protected]>
eclipse-theia · Sep 27, 2023 · 921c1f9 · 921c1f9
1 parent 516224c
commit 921c1f9
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<NotebookProviderInfo | undefined>();
         // 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
@@ -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
@@ -346,8 +346,7 @@ export class NotebooksExtImpl implements NotebooksExt {
     }
 
     async showNotebookDocument(notebookOrUri: theia.NotebookDocument | TheiaURI, options?: theia.NotebookDocumentShowOptions): Promise<theia.NotebookEditor> {
-
-        if (URI.isUri(notebookOrUri)) {
+        if (TheiaURI.isUri(notebookOrUri)) {
             notebookOrUri = await this.openNotebookDocument(notebookOrUri as TheiaURI);
         }