From 62b57c3c53ebcba9af5ec0ac0f8abcb079ce0337 Mon Sep 17 00:00:00 2001 From: Violet Shreve Date: Wed, 7 Aug 2024 19:02:27 -0400 Subject: [PATCH 1/3] Update the method for determining kernel language The previous algorithm for `getNotebookType()` was based on getting the kernel spec out of the current notebook. This is a problem when format is triggered by, for example, `jupyterlab-autosave-on-focus-change`, immediately after notebook creation. In that case, the language is determined to be `null` and errors arise. Now the `onSave()` handler waits for the `sessionContext` to be ready, then `getNotebookType()` gets the same kernel spec info from the session rather than the loaded notebook. --- src/formatter.ts | 12 +++++++++++- src/index.ts | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/formatter.ts b/src/formatter.ts index 758fe48..272485b 100644 --- a/src/formatter.ts +++ b/src/formatter.ts @@ -92,7 +92,7 @@ export class JupyterlabNotebookCodeFormatter extends JupyterlabCodeFormatter { return codeCells; } - private getNotebookType() { + private getNotebookType(): string | null { if (!this.notebookTracker.currentWidget) { return null; } @@ -123,6 +123,16 @@ export class JupyterlabNotebookCodeFormatter extends JupyterlabCodeFormatter { } } + // finally, try to get the language from the current session's kernel spec + const sessionContext = this.notebookTracker.currentWidget?.sessionContext; + const kernelName = sessionContext?.session?.kernel?.name; + if (kernelName) { + const specs = sessionContext.specsManager.specs?.kernelspecs; + if (specs && kernelName in specs) { + return specs[kernelName]!.language; + } + } + return null; } diff --git a/src/index.ts b/src/index.ts index b05e430..86541d4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -108,6 +108,7 @@ class JupyterLabCodeFormatter state: DocumentRegistry.SaveState ) { if (state === 'started' && this.config.formatOnSave) { + await context.sessionContext.ready; await this.notebookCodeFormatter.formatAllCodeCells( this.config, { saving: true }, From 92c805d5405f507f09ec4f5106473fc17aa03dc5 Mon Sep 17 00:00:00 2001 From: Violet Shreve Date: Mon, 12 Aug 2024 15:55:41 -0400 Subject: [PATCH 2/3] Make metadata access safer at runtime The `!` operator is a type assertion that the attribute is not null, but does nothing at runtime to ensure this is the case. I haven't yet found reproduction steps for this failure, but I encountered a case where accessing `model!.sharedModel` threw an error. This commit adds optional chaining operators, which do provide protection at runtime. --- src/formatter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/formatter.ts b/src/formatter.ts index 272485b..f67bf9c 100644 --- a/src/formatter.ts +++ b/src/formatter.ts @@ -98,7 +98,7 @@ export class JupyterlabNotebookCodeFormatter extends JupyterlabCodeFormatter { } const metadata = - this.notebookTracker.currentWidget.content.model!.sharedModel.metadata; + this.notebookTracker.currentWidget.content.model?.sharedModel?.metadata; if (!metadata) { return null; From 50e591041ab09bf1f321dbadef5001489d79e08b Mon Sep 17 00:00:00 2001 From: Violet Shreve Date: Mon, 12 Aug 2024 17:09:37 -0400 Subject: [PATCH 3/3] Fall back to session check if metadata is missing `getNotebookType()` has two ways to determine the language. If the metadata check fails, it falls through to the current session check. Previously, if metadata was `null`, the method would return early. --- src/formatter.ts | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/formatter.ts b/src/formatter.ts index f67bf9c..58c4e79 100644 --- a/src/formatter.ts +++ b/src/formatter.ts @@ -93,38 +93,38 @@ export class JupyterlabNotebookCodeFormatter extends JupyterlabCodeFormatter { } private getNotebookType(): string | null { + // If there is no current notebook, there is nothing to do if (!this.notebookTracker.currentWidget) { return null; } + // first, check the notebook's metadata for language info const metadata = this.notebookTracker.currentWidget.content.model?.sharedModel?.metadata; - if (!metadata) { - return null; - } - - // prefer kernelspec language - if ( - metadata.kernelspec && - metadata.kernelspec.language && - typeof metadata.kernelspec.language === 'string' - ) { - return metadata.kernelspec.language.toLowerCase(); - } + if (metadata) { + // prefer kernelspec language + if ( + metadata.kernelspec && + metadata.kernelspec.language && + typeof metadata.kernelspec.language === 'string' + ) { + return metadata.kernelspec.language.toLowerCase(); + } - // otherwise, check language info code mirror mode - if (metadata.language_info && metadata.language_info.codemirror_mode) { - const mode = metadata.language_info.codemirror_mode; - if (typeof mode === 'string') { - return mode.toLowerCase(); - } else if (typeof mode.name === 'string') { - return mode.name.toLowerCase(); + // otherwise, check language info code mirror mode + if (metadata.language_info && metadata.language_info.codemirror_mode) { + const mode = metadata.language_info.codemirror_mode; + if (typeof mode === 'string') { + return mode.toLowerCase(); + } else if (typeof mode.name === 'string') { + return mode.name.toLowerCase(); + } } } - // finally, try to get the language from the current session's kernel spec - const sessionContext = this.notebookTracker.currentWidget?.sessionContext; + // in the absence of metadata, look in the current session's kernel spec + const sessionContext = this.notebookTracker.currentWidget.sessionContext; const kernelName = sessionContext?.session?.kernel?.name; if (kernelName) { const specs = sessionContext.specsManager.specs?.kernelspecs;