review changes

Signed-off-by: Jonah Iden <[email protected]>
eclipse-theia · Nov 24, 2023 · 995c9a4 · 995c9a4
1 parent d20af95
commit 995c9a4
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 14 deletions.
diff --git a/packages/notebook/src/browser/contributions/notebook-actions-contribution.ts b/packages/notebook/src/browser/contributions/notebook-actions-contribution.ts
@@ -20,7 +20,7 @@ import { ApplicationShell, codicon, CommonCommands } from '@theia/core/lib/brows
 import { NotebookModel } from '../view-model/notebook-model';
 import { NotebookService } from '../service/notebook-service';
 import { CellEditType, CellKind } from '../../common';
-import { NotebookKernelQuickPickService, NotebookKernelQuickPickServiceImpl } from '../service/notebook-kernel-quick-pick-service';
+import { NotebookKernelQuickPickService } from '../service/notebook-kernel-quick-pick-service';
 import { NotebookExecutionService } from '../service/notebook-execution-service';
 import { NotebookEditorWidget } from '../notebook-editor-widget';
 
@@ -66,7 +66,7 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon
     protected notebookService: NotebookService;
 
     @inject(NotebookKernelQuickPickService)
-    protected notebookKernelQuickPickService: NotebookKernelQuickPickServiceImpl;
+    protected notebookKernelQuickPickService: NotebookKernelQuickPickService;
 
     @inject(NotebookExecutionService)
     protected notebookExecutionService: NotebookExecutionService;

diff --git a/packages/notebook/src/browser/notebook-frontend-module.ts b/packages/notebook/src/browser/notebook-frontend-module.ts
@@ -37,7 +37,7 @@ import { NotebookActionsContribution } from './contributions/notebook-actions-co
 import { NotebookExecutionService } from './service/notebook-execution-service';
 import { NotebookExecutionStateService } from './service/notebook-execution-state-service';
 import { NotebookKernelService } from './service/notebook-kernel-service';
-import { NotebookKernelQuickPickService, NotebookKernelQuickPickServiceImpl } from './service/notebook-kernel-quick-pick-service';
+import { NotebookKernelQuickPickService } from './service/notebook-kernel-quick-pick-service';
 import { NotebookKernelHistoryService } from './service/notebook-kernel-history-service';
 import { NotebookEditorWidgetService } from './service/notebook-editor-widget-service';
 import { NotebookRendererMessagingService } from './service/notebook-renderer-messaging-service';
@@ -65,7 +65,7 @@ export default new ContainerModule(bind => {
     bind(NotebookKernelService).toSelf().inSingletonScope();
     bind(NotebookRendererMessagingService).toSelf().inSingletonScope();
     bind(NotebookKernelHistoryService).toSelf().inSingletonScope();
-    bind(NotebookKernelQuickPickService).to(NotebookKernelQuickPickServiceImpl).inSingletonScope();
+    bind(NotebookKernelQuickPickService).toSelf().inSingletonScope();
 
     bind(NotebookCellResourceResolver).toSelf().inSingletonScope();
     bind(ResourceResolver).toService(NotebookCellResourceResolver);

diff --git a/packages/notebook/src/browser/service/notebook-editor-widget-service.ts b/packages/notebook/src/browser/service/notebook-editor-widget-service.ts
@@ -87,7 +87,7 @@ export class NotebookEditorWidgetService implements Disposable {
     }
 
     getNotebookEditors(): readonly NotebookEditorWidget[] {
-        return [...this.notebookEditors].map(e => e[1]);
+        return Array.from(this.notebookEditors.values());
     }
 
 }
diff --git a/packages/notebook/src/browser/service/notebook-execution-service.ts b/packages/notebook/src/browser/service/notebook-execution-service.ts
@@ -25,7 +25,7 @@ import { NotebookCellModel } from '../view-model/notebook-cell-model';
 import { NotebookModel } from '../view-model/notebook-model';
 import { NotebookKernelService } from './notebook-kernel-service';
 import { CommandService, Disposable } from '@theia/core';
-import { NotebookKernelQuickPickService, NotebookKernelQuickPickServiceImpl } from './notebook-kernel-quick-pick-service';
+import { NotebookKernelQuickPickService } from './notebook-kernel-quick-pick-service';
 import { NotebookKernelHistoryService } from './notebook-kernel-history-service';
 
 export interface CellExecutionParticipant {
@@ -48,7 +48,7 @@ export class NotebookExecutionService {
     protected commandService: CommandService;
 
     @inject(NotebookKernelQuickPickService)
-    protected notebookKernelQuickPickService: NotebookKernelQuickPickServiceImpl;
+    protected notebookKernelQuickPickService: NotebookKernelQuickPickService;
 
     private readonly cellExecutionParticipants = new Set<CellExecutionParticipant>();
 

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
@@ -29,8 +29,6 @@ import debounce = require('@theia/core/shared/lodash.debounce');
 
 export const JUPYTER_EXTENSION_ID = 'ms-toolsai.jupyter';
 
-export const NotebookKernelQuickPickService = Symbol('NotebookKernelQuickPickService');
-
 type KernelPick = QuickPickItem & { kernel: NotebookKernel };
 function isKernelPick(item: QuickPickInput<QuickPickItem>): item is KernelPick {
     return 'kernel' in item;
@@ -82,7 +80,7 @@ function toKernelQuickPick(kernel: NotebookKernel, selected: NotebookKernel | un
 }
 
 @injectable()
-export class NotebookKernelQuickPickServiceImpl {
+export class NotebookKernelQuickPickService {
 
     @inject(NotebookKernelService)
     protected readonly notebookKernelService: NotebookKernelService;

diff --git a/packages/notebook/src/browser/view-model/notebook-model.ts b/packages/notebook/src/browser/view-model/notebook-model.ts
@@ -60,7 +60,7 @@ export class NotebookModel implements Saveable, Disposable {
     protected readonly onDidAddOrRemoveCellEmitter = new Emitter<NotebookModelWillAddRemoveEvent>();
     readonly onDidAddOrRemoveCell = this.onDidAddOrRemoveCellEmitter.event;
 
-    private readonly onDidChangeContentEmitter = new Emitter<NotebookContentChangedEvent[]>();
+    protected readonly onDidChangeContentEmitter = new Emitter<NotebookContentChangedEvent[]>();
     readonly onDidChangeContent = this.onDidChangeContentEmitter.event;
 
     @inject(FileService)
@@ -164,7 +164,11 @@ export class NotebookModel implements Saveable, Disposable {
     }
 
     async applySnapshot(snapshot: Saveable.Snapshot): Promise<void> {
-        const data = 'read' in snapshot ? JSON.parse(snapshot.read()!) : JSON.parse(snapshot.value);
+        const rawData = 'read' in snapshot ? snapshot.read() : snapshot.value;
+        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({

diff --git a/packages/notebook/src/browser/view/notebook-cell-list-view.tsx b/packages/notebook/src/browser/view/notebook-cell-list-view.tsx
@@ -105,7 +105,7 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
 
     protected onDragStart(event: React.DragEvent<HTMLLIElement>, index: number): void {
         event.stopPropagation();
-        event.dataTransfer.setData('text/theia-notbook-cell-index', index.toString());
+        event.dataTransfer.setData('text/theia-notebook-cell-index', index.toString());
         event.dataTransfer.setData('text/plain', this.props.notebookModel.cells[index].source);
     }
 
@@ -117,7 +117,7 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
     }
 
     protected onDrop(event: React.DragEvent<HTMLLIElement>, dropElementIndex: number): void {
-        const index = parseInt(event.dataTransfer.getData('text/theia-notbook-cell-index'));
+        const index = parseInt(event.dataTransfer.getData('text/theia-notebook-cell-index'));
         const isTargetBelow = index < dropElementIndex;
         let newIdx = this.state.dragOverIndicator?.position === 'top' ? dropElementIndex : dropElementIndex + 1;
         newIdx = isTargetBelow ? newIdx - 1 : newIdx;