Repository navigation
Include notebook renderers in extension #12793
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ | |
| }, | ||
| "languageServerVersion": "0.5.30", | ||
| "publisher": "ms-python", | ||
| "enableProposedApi": false, | ||
| "enableProposedApi": true, | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipping in VSIX & used only in insiders. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this mess up using without the notebook editor? Or did VS code fix that problem?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes they did, and the fix will be in the next stable release (I think today or tomorrow). Hence the need to wait for their new release. |
||
| "author": { | ||
| "name": "Microsoft Corporation" | ||
| }, | ||
|
|
@@ -3085,7 +3085,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", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,18 +2,17 @@ | |
| // Licensed under the MIT License. | ||
| 'use strict'; | ||
| import { inject, injectable } from 'inversify'; | ||
| import * as path from 'path'; | ||
| import { ConfigurationTarget } from 'vscode'; | ||
| import { IExtensionSingleActivationService } from '../../activation/types'; | ||
| import { | ||
| IApplicationEnvironment, | ||
| IApplicationShell, | ||
| ICommandManager, | ||
| IVSCodeNotebook | ||
| IVSCodeNotebook, | ||
| IWorkspaceService | ||
| } from '../../common/application/types'; | ||
| import { NotebookEditorSupport } from '../../common/experiments/groups'; | ||
| import { traceError } from '../../common/logger'; | ||
| import { IFileSystem } from '../../common/platform/types'; | ||
| import { IDisposableRegistry, IExperimentsManager, IExtensionContext } from '../../common/types'; | ||
| import { IDisposableRegistry, IExperimentsManager } from '../../common/types'; | ||
| import { DataScience } from '../../common/utils/localize'; | ||
| import { noop } from '../../common/utils/misc'; | ||
| import { JupyterNotebookView } from './constants'; | ||
|
|
@@ -33,36 +32,18 @@ export class NotebookIntegration implements IExtensionSingleActivationService { | |
| @inject(IExperimentsManager) private readonly experiment: IExperimentsManager, | ||
| @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, | ||
| @inject(INotebookContentProvider) private readonly notebookContentProvider: INotebookContentProvider, | ||
| @inject(IExtensionContext) private readonly context: IExtensionContext, | ||
| @inject(IFileSystem) private readonly fs: IFileSystem, | ||
| @inject(ICommandManager) private readonly commandManager: ICommandManager, | ||
| @inject(NotebookKernel) private readonly notebookKernel: NotebookKernel, | ||
| @inject(NotebookOutputRenderer) private readonly renderer: NotebookOutputRenderer, | ||
| @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment, | ||
| @inject(IApplicationShell) private readonly shell: IApplicationShell | ||
| @inject(IApplicationShell) private readonly shell: IApplicationShell, | ||
| @inject(IWorkspaceService) private readonly workspace: IWorkspaceService | ||
| ) {} | ||
| public get isEnabled() { | ||
| const packageJsonFile = path.join(this.context.extensionPath, 'package.json'); | ||
| const content = JSON.parse(this.fs.readFileSync(packageJsonFile)); | ||
|
|
||
| // This code is temporary. | ||
| return ( | ||
| content.enableProposedApi && | ||
| Array.isArray(content.contributes.notebookOutputRenderer) && | ||
| (content.contributes.notebookOutputRenderer as []).length > 0 && | ||
| Array.isArray(content.contributes.notebookProvider) && | ||
| (content.contributes.notebookProvider as []).length > 0 | ||
| ); | ||
| } | ||
| public async enableSideBySideUsage() { | ||
| await this.enableNotebooks(false); | ||
| } | ||
| public async activate(): Promise<void> { | ||
| // 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)) { | ||
| await this.enableNotebooks(true); | ||
| await this.enableNotebooks(); | ||
| } | ||
| if (this.env.channel !== 'insiders') { | ||
| return; | ||
|
|
@@ -108,62 +89,29 @@ export class NotebookIntegration implements IExtensionSingleActivationService { | |
| } | ||
| } | ||
| } | ||
| private async enableNotebooks(useVSCodeNotebookAsDefaultEditor: boolean) { | ||
| private async enableNotebooks() { | ||
| if (this.env.channel === 'stable') { | ||
| this.shell.showErrorMessage(DataScience.previewNotebookOnlySupportedInVSCInsiders()).then(noop, noop); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Existing code ensures notebooks gets used only in insiders. |
||
| return; | ||
| } | ||
| const packageJsonFile = path.join(this.context.extensionPath, 'package.json'); | ||
| const content = JSON.parse(this.fs.readFileSync(packageJsonFile)); | ||
|
|
||
| // This code is temporary. | ||
| const settings = this.workspace.getConfiguration('workbench', undefined); | ||
| const editorAssociations = settings.get('editorAssociations') as { | ||
| viewType: string; | ||
| filenamePattern: string; | ||
| }[]; | ||
|
|
||
| if ( | ||
| !content.enableProposedApi || | ||
| !Array.isArray(content.contributes.notebookOutputRenderer) || | ||
| !Array.isArray(content.contributes.notebookProvider) | ||
| !Array.isArray(editorAssociations) || | ||
| editorAssociations.length === 0 || | ||
| !editorAssociations.find((item) => item.viewType === JupyterNotebookView) | ||
| ) { | ||
| 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 (preview)', | ||
| selector: [ | ||
| { | ||
| filenamePattern: '*.ipynb' | ||
| } | ||
| ], | ||
| priority: useVSCodeNotebookAsDefaultEditor ? 'default' : 'option' | ||
| } | ||
| ]; | ||
|
|
||
| await this.fs.writeFile(packageJsonFile, JSON.stringify(content, undefined, 4)); | ||
| await this.commandManager | ||
| .executeCommand('python.reloadVSCode', DataScience.reloadVSCodeNotebookEditor()) | ||
| .then(noop, noop); | ||
| editorAssociations.push({ | ||
| viewType: 'jupyter-notebook', | ||
| filenamePattern: '*.ipynb' | ||
| }); | ||
| await settings.update('editorAssociations', editorAssociations, ConfigurationTarget.Global); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As the package.json contains the necessary bits, we just need to update the users settings.json file to pick our notebook viewer as the default viewer for ipynb file. |
||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shipping renderers in extension.
This is a fallback until we get Renderers extension published.