Skip to content

Commit

Permalink
Allow users to open VSC notebook even when using our own Notebook Edi…
Browse files Browse the repository at this point in the history
…tors (microsoft#12402)

For #12400
* Side by side nb editors
  • Loading branch information
DonJayamanne committed Jun 17, 2020
1 parent a03edb5 commit 3d829fc
Show file tree
Hide file tree
Showing 20 changed files with 384 additions and 183 deletions.
5 changes: 0 additions & 5 deletions build/ci/templates/steps/compile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ steps:
compile: 'false'
installVSCEorNPX: 'false'

- task: Gulp@0
displayName: 'Validate package.json'
inputs:
targets: 'validate-packagejson'

- task: Gulp@0
displayName: 'Compile and check for errors'
inputs:
Expand Down
13 changes: 0 additions & 13 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,10 @@ gulp.task('checkNativeDependencies', (done) => {
done();
});

gulp.task('validate-packagejson', () => validatePackageJson());
gulp.task('compile-ipywidgets', () => buildIPyWidgets());

const webpackEnv = { NODE_OPTIONS: '--max_old_space_size=9096' };

async function validatePackageJson() {
const json = require('./package.json');
if (json.enableProposedApi) {
throw new Error('package.json has enableProposedApi setting enabled');
}
if (json.contributes.notebookOutputRenderer) {
throw new Error('Package.json contains entry for contributes.notebookOutputRenderer');
}
if (json.contributes.notebookProvider) {
throw new Error('Package.json contains entry for contributes.notebookProvider');
}
}

async function buildIPyWidgets() {
// if the output ipywidgest file exists, then no need to re-build.
Expand Down
40 changes: 38 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
},
"languageServerVersion": "0.5.30",
"publisher": "ms-python",
"enableProposedApi": false,
"enableProposedApi": true,
"author": {
"name": "Microsoft Corporation"
},
Expand Down Expand Up @@ -3028,7 +3028,43 @@
"when": "testsDiscovered"
}
]
}
},
"notebookOutputRenderer": [
{
"viewType": "jupyter-notebook-renderer",
"displayName": "Jupyter Notebook Renderer",
"mimeTypes": [
"application/geo+json",
"application/vdom.v1+json",
"application/vnd.dataresource+json",
"application/vnd.plotly.v1+json",
"application/vnd.vega.v2+json",
"application/vnd.vega.v3+json",
"application/vnd.vega.v4+json",
"application/vnd.vega.v5+json",
"application/vnd.vegalite.v1+json",
"application/vnd.vegalite.v2+json",
"application/vnd.vegalite.v3+json",
"application/vnd.vegalite.v4+json",
"application/x-nteract-model-debug+json",
"image/gif",
"text/latex",
"text/vnd.plotly.v1+html"
]
}
],
"notebookProvider": [
{
"viewType": "jupyter-notebook",
"displayName": "Jupyter Notebook (preview)",
"selector": [
{
"filenamePattern": "*.ipynb"
}
],
"priority": "option"
}
]
},
"scripts": {
"package": "gulp clean && gulp prePublishBundle && vsce package -o ms-python-insiders.vsix",
Expand Down
4 changes: 3 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -529,5 +529,7 @@
"DataScience.rawKernelSessionFailed": "Unable to start session for kernel {0}. Select another kernel to launch with.",
"DataScience.rawKernelConnectingSession": "Connecting to kernel.",
"DataScience.reloadCustomEditor": "Please reload VS Code to use the custom editor API",
"DataScience.step": "Run next line (F10)"
"DataScience.reloadVSCodeNotebookEditor": "Please reload VS Code to use the Notebook Editor",
"DataScience.step": "Run next line (F10)",
"DataScience.usingPreviewNotebookWithOtherNotebookWarning": "Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks."
}
2 changes: 1 addition & 1 deletion src/client/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export function isUnitTestExecution(): boolean {

// Temporary constant, used to indicate whether we're using custom editor api or not.
export const UseCustomEditorApi = Symbol('USE_CUSTOM_EDITOR');
export const UseNativeEditorApi = Symbol('USE_NATIVEEDITOR');
export const UseVSCodeNotebookEditorApi = Symbol('USE_NATIVEEDITOR');
export const UseProposedApi = Symbol('USE_VSC_PROPOSED_API');

export * from '../constants';
8 changes: 8 additions & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,14 @@ export namespace DataScience {
'DataScience.reloadCustomEditor',
'Please reload VS Code to use the custom editor API'
);
export const reloadVSCodeNotebookEditor = localize(
'DataScience.reloadVSCodeNotebookEditor',
'Please reload VS Code to use the Notebook Editor'
);
export const usingPreviewNotebookWithOtherNotebookWarning = localize(
'DataScience.usingPreviewNotebookWithOtherNotebookWarning',
'Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.'
);
}

export namespace StartPage {
Expand Down
3 changes: 3 additions & 0 deletions src/client/datascience/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,6 @@ export namespace LiveShareCommands {
export const rawKernelSupported = 'rawKernelSupported';
export const createRawNotebook = 'createRawNotebook';
}

export const VSCodeNotebookProvider = 'VSCodeNotebookProvider';
export const OurNotebookProvider = 'OurNotebookProvider';
19 changes: 3 additions & 16 deletions src/client/datascience/context/activeEditorContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { inject, injectable } from 'inversify';
import { TextEditor } from 'vscode';
import { IExtensionSingleActivationService } from '../../activation/types';
import { ICommandManager, IDocumentManager, IVSCodeNotebook } from '../../common/application/types';
import { ICommandManager, IDocumentManager } from '../../common/application/types';
import { PYTHON_LANGUAGE } from '../../common/constants';
import { ContextKey } from '../../common/contextKey';
import { NotebookEditorSupport } from '../../common/experiments/groups';
Expand All @@ -31,7 +31,6 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
@inject(IDocumentManager) private readonly docManager: IDocumentManager,
@inject(ICommandManager) private readonly commandManager: ICommandManager,
@inject(IDisposableRegistry) disposables: IDisposableRegistry,
@inject(IVSCodeNotebook) private readonly vscodeNotebook: IVSCodeNotebook,
@inject(IExperimentsManager) private readonly experiments: IExperimentsManager
) {
disposables.push(this);
Expand Down Expand Up @@ -72,28 +71,16 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
if (this.docManager.activeTextEditor?.document.languageId === PYTHON_LANGUAGE) {
this.onDidChangeActiveTextEditor(this.docManager.activeTextEditor);
}
if (this.experiments.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
this.vscodeNotebook.onDidChangeNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
this.vscodeNotebook.onDidCloseNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
this.vscodeNotebook.onDidOpenNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
this.docManager.onDidChangeActiveTextEditor(this.onDidChangeVSCodeNotebook, this, this.disposables);
}
}

private udpateNativeNotebookCellContext() {
if (!this.experiments.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
return;
}
this.hasNativeNotebookCells
.set((this.vscodeNotebook.activeNotebookEditor?.document?.cells?.length || 0) > 0)
.set((this.notebookEditorProvider.activeEditor?.model?.cells?.length || 0) > 0)
.ignoreErrors();
}
private onDidChangeVSCodeNotebook() {
this.isPythonFileActive = !this.vscodeNotebook.activeNotebookEditor;
this.nativeContext.set(!!this.vscodeNotebook.activeNotebookEditor).ignoreErrors();
this.udpateNativeNotebookCellContext();
this.updateMergedContexts();
}
private onDidChangeActiveInteractiveWindow(e?: IInteractiveWindow) {
this.interactiveContext.set(!!e).ignoreErrors();
this.updateMergedContexts();
Expand All @@ -104,7 +91,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
}
private onDidChangeActiveTextEditor(e?: TextEditor) {
this.isPythonFileActive =
e?.document.languageId === PYTHON_LANGUAGE && !this.vscodeNotebook.activeNotebookEditor;
e?.document.languageId === PYTHON_LANGUAGE && !this.notebookEditorProvider.activeEditor;
this.udpateNativeNotebookCellContext();
this.updateMergedContexts();
}
Expand Down
17 changes: 14 additions & 3 deletions src/client/datascience/interactive-ipynb/nativeEditorStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { CellState, ICell, IJupyterExecution, IJupyterKernelSpec, INotebookModel
import detectIndent = require('detect-indent');
// tslint:disable-next-line:no-require-imports no-var-requires
import cloneDeep = require('lodash/cloneDeep');
import { UseNativeEditorApi } from '../../common/constants';
import { UseVSCodeNotebookEditorApi } from '../../common/constants';
import { isFileNotFoundError } from '../../common/platform/errors';
import { sendTelemetryEvent } from '../../telemetry';
import { pruneCell } from '../common';
Expand Down Expand Up @@ -107,7 +107,7 @@ export class NativeEditorNotebookModel implements INotebookModel {
private _id = uuid();

constructor(
private readonly useNativeEditorApi: boolean,
public useNativeEditorApi: boolean,
file: Uri,
cells: ICell[],
json: Partial<nbformat.INotebookContent> = {},
Expand Down Expand Up @@ -494,6 +494,17 @@ export class NativeEditorNotebookModel implements INotebookModel {
}
}

/**
* Temporary hack to ensure we can use VS Code notebooks along with our standard notbooked editors.
*/
export function updateModelForUseWithVSCodeNotebook(model: INotebookModel) {
if (!(model instanceof NativeEditorNotebookModel)) {
throw new Error('Unsupported NotebookModel');
}
const rawModel = model as NativeEditorNotebookModel;
rawModel.useNativeEditorApi = true;
}

@injectable()
export class NativeEditorStorage implements INotebookStorage {
public get onSavedAs(): Event<{ new: Uri; old: Uri }> {
Expand All @@ -513,7 +524,7 @@ export class NativeEditorStorage implements INotebookStorage {
@inject(IExtensionContext) private context: IExtensionContext,
@inject(IMemento) @named(GLOBAL_MEMENTO) private globalStorage: Memento,
@inject(IMemento) @named(WORKSPACE_MEMENTO) private localStorage: Memento,
@inject(UseNativeEditorApi) private readonly useNativeEditorApi: boolean
@inject(UseVSCodeNotebookEditorApi) private readonly useNativeEditorApi: boolean
) {}
private static isUntitledFile(file: Uri) {
return isUntitledFile(file);
Expand Down
9 changes: 2 additions & 7 deletions src/client/datascience/notebook/cellEditSyncService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import type { NotebookCell, NotebookDocument } from '../../../../typings/vscode-
import { splitMultilineString } from '../../../datascience-ui/common';
import { IExtensionSingleActivationService } from '../../activation/types';
import { IDocumentManager, IVSCodeNotebook } from '../../common/application/types';
import { NotebookEditorSupport } from '../../common/experiments/groups';
import { IDisposable, IDisposableRegistry, IExperimentsManager } from '../../common/types';
import { IDisposable, IDisposableRegistry } from '../../common/types';
import { isNotebookCell } from '../../common/utils/misc';
import { traceError } from '../../logging';
import { INotebookEditorProvider, INotebookModel } from '../types';
Expand All @@ -23,8 +22,7 @@ export class CellEditSyncService implements IExtensionSingleActivationService, I
@inject(IDocumentManager) private readonly documentManager: IDocumentManager,
@inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry,
@inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook,
@inject(INotebookEditorProvider) private readonly editorProvider: INotebookEditorProvider,
@inject(IExperimentsManager) private readonly experiment: IExperimentsManager
@inject(INotebookEditorProvider) private readonly editorProvider: INotebookEditorProvider
) {
disposableRegistry.push(this);
}
Expand All @@ -34,9 +32,6 @@ export class CellEditSyncService implements IExtensionSingleActivationService, I
}
}
public async activate(): Promise<void> {
if (!this.experiment.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
return;
}
this.documentManager.onDidChangeTextDocument(this.onDidChangeTextDocument, this, this.disposables);
}

Expand Down
32 changes: 31 additions & 1 deletion src/client/datascience/notebook/contentProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,16 @@ import type {
NotebookDocumentOpenContext
} from 'vscode-proposed';
import { ICommandManager } from '../../common/application/types';
import { MARKDOWN_LANGUAGE, UseVSCodeNotebookEditorApi } from '../../common/constants';
import { DataScience } from '../../common/utils/localize';
import { captureTelemetry } from '../../telemetry';
import { Telemetry } from '../constants';
import { updateModelForUseWithVSCodeNotebook } from '../interactive-ipynb/nativeEditorStorage';
import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider';
import { notebookModelToVSCNotebookData } from './helpers/helpers';
import { NotebookEditorCompatibilitySupport } from './notebookEditorCompatibilitySupport';
// tslint:disable-next-line: no-var-requires no-require-imports
const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');

/**
* This class is responsible for reading a notebook file (ipynb or other files) and returning VS Code with the NotebookData.
Expand All @@ -36,10 +42,34 @@ export class NotebookContentProvider implements VSCodeNotebookContentProvider {
}
constructor(
@inject(INotebookStorageProvider) private readonly notebookStorage: INotebookStorageProvider,
@inject(ICommandManager) private readonly commandManager: ICommandManager
@inject(ICommandManager) private readonly commandManager: ICommandManager,
@inject(UseVSCodeNotebookEditorApi) private readonly useVSCodeNotebookEditorApi: boolean,
@inject(NotebookEditorCompatibilitySupport)
private readonly compatibilitySupport: NotebookEditorCompatibilitySupport
) {}
public async openNotebook(uri: Uri, openContext: NotebookDocumentOpenContext): Promise<NotebookData> {
if (!this.compatibilitySupport.canOpenWithVSCodeNotebookEditor(uri)) {
// If not supported, return a notebook with error displayed.
// We cannot, not display a notebook.
return {
cells: [
{
cellKind: vscodeNotebookEnums.CellKind.Markdown,
language: MARKDOWN_LANGUAGE,
source: `# ${DataScience.usingPreviewNotebookWithOtherNotebookWarning()}`,
metadata: { editable: false, runnable: false },
outputs: []
}
],
languages: [],
metadata: { cellEditable: false, editable: false, runnable: false }
};
}
const model = await this.notebookStorage.load(uri, undefined, !openContext.backupId);
// If experiment is not enabled, then this method was invoked as user opted to try and open using the new API.
if (!this.useVSCodeNotebookEditorApi) {
updateModelForUseWithVSCodeNotebook(model);
}
return notebookModelToVSCNotebookData(model);
}
@captureTelemetry(Telemetry.Save, undefined, true)
Expand Down
64 changes: 12 additions & 52 deletions src/client/datascience/notebook/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ICommandManager, IVSCodeNotebook } from '../../common/application/types
import { NotebookEditorSupport } from '../../common/experiments/groups';
import { IFileSystem } from '../../common/platform/types';
import { IDisposableRegistry, IExperimentsManager, IExtensionContext } from '../../common/types';
import { DataScience } from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { JupyterNotebookView } from './constants';
import { NotebookContentProvider } from './contentProvider';
Expand Down Expand Up @@ -36,60 +37,19 @@ export class NotebookIntegration implements IExtensionSingleActivationService {
// This condition is temporary.
// If user belongs to the experiment, then make the necessary changes to package.json.
// Once the API is final, we won't need to modify the package.json.
if (!this.experiment.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
return;
}

const packageJsonFile = path.join(this.context.extensionPath, 'package.json');
const content = JSON.parse(this.fs.readFileSync(packageJsonFile));
if (this.experiment.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
const packageJsonFile = path.join(this.context.extensionPath, 'package.json');
const content = JSON.parse(this.fs.readFileSync(packageJsonFile));

// This code is temporary.
if (
!content.enableProposedApi ||
!Array.isArray(content.contributes.notebookOutputRenderer) ||
!Array.isArray(content.contributes.notebookProvider)
) {
content.enableProposedApi = true;
content.contributes.notebookOutputRenderer = [
{
viewType: 'jupyter-notebook-renderer',
displayName: 'Jupyter Notebook Renderer',
mimeTypes: [
'application/geo+json',
'application/vdom.v1+json',
'application/vnd.dataresource+json',
'application/vnd.plotly.v1+json',
'application/vnd.vega.v2+json',
'application/vnd.vega.v3+json',
'application/vnd.vega.v4+json',
'application/vnd.vega.v5+json',
'application/vnd.vegalite.v1+json',
'application/vnd.vegalite.v2+json',
'application/vnd.vegalite.v3+json',
'application/vnd.vegalite.v4+json',
'application/x-nteract-model-debug+json',
'image/gif',
'text/latex',
'text/vnd.plotly.v1+html'
]
}
];
content.contributes.notebookProvider = [
{
viewType: JupyterNotebookView,
displayName: 'Jupyter Notebook',
selector: [
{
filenamePattern: '*.ipynb'
}
]
}
];
// This code is temporary.
if (content.contributes.notebookProvider[0].priority !== 'default') {
content.contributes.notebookProvider[0].priority = 'default';

await this.fs.writeFile(packageJsonFile, JSON.stringify(content, undefined, 4));
await this.commandManager
.executeCommand('python.reloadVSCode', 'Please reload VS Code to use the new VS Code Notebook API')
.then(noop, noop);
await this.fs.writeFile(packageJsonFile, JSON.stringify(content, undefined, 4));
await this.commandManager
.executeCommand('python.reloadVSCode', DataScience.reloadVSCodeNotebookEditor())
.then(noop, noop);
}
}

this.disposables.push(
Expand Down
Loading

0 comments on commit 3d829fc

Please sign in to comment.