Update the method for determining kernel language #354
Conversation
|
|
shreve
changed the title
|
I was pursuing some additional changes around languages for both the notebook and editor formatters, but realized it was a lot to contribute without even asking. @krassowski @ryantam626 Would you welcome larger code changes? In particular, I added some type definitions, extracted some repeated code, and replaced the hard-coded MIME-type map with the CodeMirror MIME service. |
| @@ -108,6 +108,7 @@ class JupyterLabCodeFormatter | |||
| state: DocumentRegistry.SaveState | |||
| ) { | |||
| if (state === 'started' && this.config.formatOnSave) { | |||
| await context.sessionContext.ready; | |||
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.
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
Outdated
| @@ -123,6 +123,16 @@ export class JupyterlabNotebookCodeFormatter extends JupyterlabCodeFormatter { | |||
| } | |||
| } | |||
|
|
|||
| // finally, try to get the language from the current session's kernel spec | |||
There was a problem hiding this comment.
Is it intended that this block will not be reached when metadata are not defined?
There was a problem hiding this comment.
Great question. I see the problem you're asking about. I think I wanted to change as few lines as possible, but I realized that's just not the right way to do this. I've pushed another update. Thanks for all your effort in reviewing this.
`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.
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 benulland errors arise.Now the
onSave()handler waits for thesessionContextto be ready, thengetNotebookType()gets the same kernel spec info from the session rather than the loaded notebook.