Skip to content

Commit

Permalink
Fix issue with kernel preselection being overridden by view state (#1…
Browse files Browse the repository at this point in the history
…54968)

* Fix view state overriding selected kernel

* Add test to verify correct kernel is used
  • Loading branch information
rchiodo authored Jul 13, 2022
1 parent f649124 commit 052d5b0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ async function createInteractiveWindow(kernel: Kernel) {
// Keep focus on the owning file if there is one
{ viewColumn: vscode.ViewColumn.Beside, preserveFocus: false },
undefined,
kernel.controller.id,
`vscode.vscode-api-tests/${kernel.controller.id}`,
undefined
)) as unknown as INativeInteractiveWindow;

Expand Down Expand Up @@ -45,11 +45,13 @@ async function addCellAndRun(code: string, notebook: vscode.NotebookDocument, i:

const testDisposables: vscode.Disposable[] = [];
let defaultKernel: Kernel;
let secondKernel: Kernel;

setup(async function () {
// there should be ONE default kernel in this suite
defaultKernel = new Kernel('mainKernel', 'Notebook Default Kernel', 'interactive');
secondKernel = new Kernel('secondKernel', 'Notebook Secondary Kernel', 'interactive');
testDisposables.push(defaultKernel.controller);
testDisposables.push(secondKernel.controller);
await saveAllFilesAndCloseAll();
});

Expand Down Expand Up @@ -85,4 +87,22 @@ async function addCellAndRun(code: string, notebook: vscode.NotebookDocument, i:
assert.strictEqual(notebookEditor.visibleRanges[notebookEditor.visibleRanges.length - 1].end, notebookEditor.notebook.cellCount, `Last cell is not visible`);

});

test('Interactive window has the correct kernel', async () => {
assert.ok(vscode.workspace.workspaceFolders);
const notebookEditor = await createInteractiveWindow(defaultKernel);
assert.ok(notebookEditor);

await vscode.commands.executeCommand('workbench.action.closeActiveEditor');

// Create a new interactive window with a different kernel
const notebookEditor2 = await createInteractiveWindow(secondKernel);
assert.ok(notebookEditor2);

// Verify the kernel is the secondary one
await addCellAndRun(`print`, notebookEditor2.notebook, 0);

assert.strictEqual(secondKernel.associatedNotebooks.has(notebookEditor2.notebook.uri.toString()), true, `Secondary kernel was not set as the kernel for the interactive window`);

});
});
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,11 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD

private _restoreSelectedKernel(viewState: INotebookEditorViewState | undefined): void {
if (viewState?.selectedKernelId && this.textModel) {
const kernel = this.notebookKernelService.getMatchingKernel(this.textModel).all.find(k => k.id === viewState.selectedKernelId);
if (kernel) {
const matching = this.notebookKernelService.getMatchingKernel(this.textModel);
const kernel = matching.all.find(k => k.id === viewState.selectedKernelId);
// Selected kernel may have already been picked prior to the view state loading
// If so, don't overwrite it with the saved kernel.
if (kernel && !matching.selected) {
this.notebookKernelService.selectKernelForNotebook(kernel, this.textModel);
}
}
Expand Down

0 comments on commit 052d5b0

Please sign in to comment.