Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use openWith command to open notebooks #12104

Merged
merged 1 commit into from
Jun 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/client/common/application/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ interface ICommandNameWithoutArgumentTypeMapping {
* @extends {ICommandNameWithoutArgumentTypeMapping}
*/
export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgumentTypeMapping {
['vscode.openWith']: [Uri, string];
['workbench.action.quickOpen']: [string];
['workbench.extensions.installExtension']: [Uri | 'ms-python.python'];
['setContext']: [string, boolean];
Expand Down
4 changes: 2 additions & 2 deletions src/client/common/application/customEditorService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ export class CustomEditorService implements ICustomEditorService {
}
}

public async openEditor(file: vscode.Uri): Promise<void> {
public async openEditor(file: vscode.Uri, viewType: string): Promise<void> {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rchiodo I've tested this with customEditor changes (new API) & it works.
Also works with untitled notebooks.

if (this.useCustomEditorApi) {
await this.commandManager.executeCommand('vscode.open', file);
await this.commandManager.executeCommand('vscode.openWith', file, viewType);
}
}
}
2 changes: 1 addition & 1 deletion src/client/common/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@ export interface ICustomEditorService {
/**
* Opens a file with a custom editor
*/
openEditor(file: Uri): Promise<void>;
openEditor(file: Uri, viewType: string): Promise<void>;
}

export const IClipboard = Symbol('IClipboard');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export class NativeEditorProvider
disposable = this._onDidOpenNotebookEditor.event(handler);

// Send an open command.
this.customEditorService.openEditor(file).ignoreErrors();
this.customEditorService.openEditor(file, NativeEditorProvider.customEditorViewType).ignoreErrors();

// Promise should resolve when the file opens.
return deferred.promise;
Expand Down
4 changes: 4 additions & 0 deletions src/client/datascience/notebook/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

export const JupyterNotebookView = 'jupyter-notebook';
3 changes: 2 additions & 1 deletion src/client/datascience/notebook/notebookEditorProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
monitorModelCellOutputChangesAndUpdateNotebookDocument,
updateCellModelWithChangesToVSCCell
} from './cellUpdateHelpers';
import { JupyterNotebookView } from './constants';
import { NotebookEditor } from './notebookEditor';
import { INotebookExecutionService } from './types';

Expand Down Expand Up @@ -147,7 +148,7 @@ export class NotebookEditorProvider implements INotebookEditorProvider {

// Tell VSC to open the notebook, at which point it will fire a callback when a notebook document has been opened.
// Then our promise will get resolved.
await this.commandManager.executeCommand('vscode.open', file);
await this.commandManager.executeCommand('vscode.openWith', file, JupyterNotebookView);

// This gets resolved when we have handled the opening of the notebook.
return deferred.promise;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ suite('DataScience - Native Editor Provider', () => {
});

customEditorService
.setup((c) => c.openEditor(typemoq.It.isAny()))
.setup((c) => c.openEditor(typemoq.It.isAny(), typemoq.It.isAny()))
.returns(async (f) => {
const doc = typemoq.Mock.ofType<CustomDocument>();
doc.setup((d) => d.uri).returns(() => f);
Expand Down
2 changes: 1 addition & 1 deletion src/test/datascience/mockCustomEditorService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class MockCustomEditorService implements ICustomEditorService {

return { dispose: noop };
}
public async openEditor(file: Uri): Promise<void> {
public async openEditor(file: Uri, _viewType: string): Promise<void> {
if (!this.provider) {
throw new Error('Opening before registering');
}
Expand Down
11 changes: 6 additions & 5 deletions src/test/datascience/notebook/notebookEditorProvider.ds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as sinon from 'sinon';
import { commands, Uri } from 'vscode';
import { ICommandManager, IVSCodeNotebook } from '../../../client/common/application/types';
import { IDisposable } from '../../../client/common/types';
import { JupyterNotebookView } from '../../../client/datascience/notebook/constants';
import { NotebookEditor } from '../../../client/datascience/notebook/notebookEditor';
import { INotebookEditorProvider } from '../../../client/datascience/types';
import { createEventHandler, IExtensionTestApi, waitForCondition } from '../../common';
Expand Down Expand Up @@ -125,7 +126,7 @@ suite('DataScience - VSCode Notebook', function () {
await modelDisposed.assertFired();
});
test('Opening an nb multiple times will result in a single (our) INotebookEditor being created', async () => {
await commandManager.executeCommand('vscode.open', Uri.file(templateIPynb));
await commandManager.executeCommand('vscode.openWith', Uri.file(templateIPynb), JupyterNotebookView);
await waitForCondition(async () => !!editorProvider.activeEditor, 2_000, 'Editor not created');

// Open a duplicate editor.
Expand All @@ -137,7 +138,7 @@ suite('DataScience - VSCode Notebook', function () {
assert.lengthOf(editorProvider.editors, 1);
});
test('Closing one of the duplicate notebooks will not dispose (our) INotebookEditor until all VSC Editors are closed', async () => {
await commandManager.executeCommand('vscode.open', Uri.file(templateIPynb));
await commandManager.executeCommand('vscode.openWith', Uri.file(templateIPynb), JupyterNotebookView);
await waitForCondition(async () => !!editorProvider.activeEditor, 2_000, 'Editor not created');

const editorDisposed = createEventHandler(editorProvider.activeEditor!, 'closed', disposables);
Expand Down Expand Up @@ -221,7 +222,7 @@ suite('DataScience - VSCode Notebook', function () {
assert.isUndefined(editorProvider.activeEditor);
assert.equal(editorProvider.editors.length, 0);

await commandManager.executeCommand('vscode.open', testIPynb);
await commandManager.executeCommand('vscode.openWith', testIPynb, JupyterNotebookView);

assert.equal(editorProvider.editors.length, 1);
assert.isOk(vscodeNotebook.activeNotebookEditor);
Expand All @@ -232,7 +233,7 @@ suite('DataScience - VSCode Notebook', function () {
assert.isUndefined(editorProvider.activeEditor);
assert.equal(editorProvider.editors.length, 0);

await commandManager.executeCommand('vscode.open', testIPynb);
await commandManager.executeCommand('vscode.openWith', testIPynb, JupyterNotebookView);

assert.equal(editorProvider.editors.length, 1);
assert.isOk(vscodeNotebook.activeNotebookEditor);
Expand Down Expand Up @@ -295,7 +296,7 @@ suite('DataScience - VSCode Notebook', function () {
await activeNotebookChanged.assertFiredExactly(1);

// Open another notebook.
await commandManager.executeCommand('vscode.open', testIPynb);
await commandManager.executeCommand('vscode.openWith', testIPynb, JupyterNotebookView);

await notebookOpened.assertFiredExactly(2);
await activeNotebookChanged.assertFiredExactly(2);
Expand Down