diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index 716ae5c7f7..336f2cb695 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -690,7 +690,7 @@ { "provider": "jupyter", "fileExtensions": [ - "IPYNB" + ".ipynb" ], "standardKernels": [ { diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts index 2ab1f56727..56f7cc61d3 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -53,7 +53,7 @@ import { ImageMimeTypes, TextCellEditModes } from 'sql/workbench/services/notebo import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { NotebookInput } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; -import { IExecuteManager } from 'sql/workbench/services/notebook/browser/notebookService'; +import { DEFAULT_NOTEBOOK_FILETYPE, IExecuteManager, SQL_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; import { NotebookExplorerViewletViewsContribution } from 'sql/workbench/contrib/notebook/browser/notebookExplorer/notebookExplorerViewlet'; import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import { ContributedEditorPriority, IEditorOverrideService } from 'vs/workbench/services/editor/common/editorOverrideService'; @@ -62,7 +62,8 @@ import { IModeService } from 'vs/editor/common/services/modeService'; import { ILogService } from 'vs/platform/log/common/log'; import { DiffEditorInput } from 'vs/workbench/common/editor/diffEditorInput'; import { useNewMarkdownRendererKey } from 'sql/workbench/contrib/notebook/common/notebookCommon'; -import { JUPYTER_PROVIDER_ID } from 'sql/workbench/common/constants'; +import { JUPYTER_PROVIDER_ID, NotebookLanguage } from 'sql/workbench/common/constants'; +import { INotebookProviderRegistry, NotebookProviderRegistryId } from 'sql/workbench/services/notebook/common/notebookRegistry'; Registry.as(EditorExtensions.EditorInputFactories) .registerEditorInputSerializer(FileNotebookInput.ID, FileNoteBookEditorInputSerializer); @@ -687,10 +688,12 @@ configurationRegistry.registerConfiguration({ }); const languageAssociationRegistry = Registry.as(LanguageAssociationExtensions.LanguageAssociations); +const notebookRegistry = Registry.as(NotebookProviderRegistryId); export class NotebookEditorOverrideContribution extends Disposable implements IWorkbenchContribution { private _registeredOverrides = new DisposableStore(); + private _newFileExtensions: string[] = []; constructor( @ILogService private _logService: ILogService, @@ -705,47 +708,59 @@ export class NotebookEditorOverrideContribution extends Disposable implements IW this._modeService.onLanguagesMaybeChanged(() => { this.registerEditorOverrides(); }); + notebookRegistry.onNewDescriptionRegistration(({ id, registration }) => { + if (id !== JUPYTER_PROVIDER_ID && id !== SQL_NOTEBOOK_PROVIDER && registration?.fileExtensions?.length > 0) { + let extensions = registration.fileExtensions + .filter(ext => ext?.length > 0 && ext.toLowerCase() !== DEFAULT_NOTEBOOK_FILETYPE); + this._newFileExtensions = this._newFileExtensions.concat(extensions); + this.registerEditorOverrides(); + } + }); } private registerEditorOverrides(): void { this._registeredOverrides.clear(); - // List of language IDs to associate the query editor for. These are case sensitive. - NotebookEditorInputAssociation.languages.map(lang => { + let allExtensions: string[] = []; + + // List of built-in language IDs to associate the query editor for. These are case sensitive. + NotebookEditorInputAssociation.languages.forEach(lang => { const langExtensions = this._modeService.getExtensions(lang); - if (langExtensions.length === 0) { - return; - } - // Create the selector from the list of all the language extensions we want to associate with the - // notebook editor (filtering out any languages which didn't have any extensions registered yet) - const selector = `*{${langExtensions.join(',')}}`; - this._registeredOverrides.add(this._editorOverrideService.registerEditor( - selector, - { - id: NotebookEditor.ID, - label: NotebookEditor.LABEL, - describes: (currentEditor) => currentEditor instanceof FileNotebookInput, - priority: ContributedEditorPriority.builtin - }, - {}, - (resource, options, group) => { - const fileInput = this._editorService.createEditorInput({ - resource: resource - }) as FileEditorInput; - // Try to convert the input, falling back to just a plain file input if we're unable to - const newInput = this.tryConvertInput(fileInput, lang) ?? fileInput; - return { editor: newInput, options: options, group: group }; - }, - (diffEditorInput, options, group) => { - // Try to convert the input, falling back to the original input if we're unable to - const newInput = this.tryConvertInput(diffEditorInput, lang) ?? diffEditorInput; - return { editor: newInput, options: options, group: group }; - } - )); + allExtensions = allExtensions.concat(langExtensions); }); + + // Add newly registered file extensions + allExtensions = allExtensions.concat(this._newFileExtensions); + + // Create the selector from the list of all the language extensions we want to associate with the + // notebook editor + const selector = `*{${allExtensions.join(',')}}`; + this._registeredOverrides.add(this._editorOverrideService.registerEditor( + selector, + { + id: NotebookEditor.ID, + label: NotebookEditor.LABEL, + describes: (currentEditor) => currentEditor instanceof FileNotebookInput, + priority: ContributedEditorPriority.builtin + }, + {}, + (resource, options, group) => { + const fileInput = this._editorService.createEditorInput({ + resource: resource + }) as FileEditorInput; + // Try to convert the input, falling back to just a plain file input if we're unable to + const newInput = this.tryConvertInput(fileInput) ?? fileInput; + return { editor: newInput, options: options, group: group }; + }, + (diffEditorInput, options, group) => { + // Try to convert the input, falling back to the original input if we're unable to + const newInput = this.tryConvertInput(diffEditorInput) ?? diffEditorInput; + return { editor: newInput, options: options, group: group }; + } + )); } - private tryConvertInput(input: IEditorInput, lang: string): IEditorInput | undefined { - const langAssociation = languageAssociationRegistry.getAssociationForLanguage(lang); + private tryConvertInput(input: IEditorInput): IEditorInput | undefined { + const langAssociation = languageAssociationRegistry.getAssociationForLanguage(NotebookLanguage.Ipynb); const notebookEditorInput = langAssociation?.syncConvertInput?.(input); if (!notebookEditorInput) { this._logService.warn('Unable to create input for overriding editor ', input instanceof DiffEditorInput ? `${input.primary.resource.toString()} <-> ${input.secondary.resource.toString()}` : input.resource.toString()); diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts index 1f513cfa98..11493ead30 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts @@ -221,17 +221,17 @@ suite.skip('NotebookService:', function (): void { assert.strictEqual(notebookService.languageMagics.length, 0, 'No language magics should exist after creation'); assert.strictEqual(notebookService.listNotebookEditors().length, 0, 'No notebook editors should be listed'); assert.strictEqual(notebookService.getMimeRegistry().mimeTypes.length, 15, 'MIME Types need to have appropriate tests when added or removed'); - assert.deepStrictEqual(notebookService.getProvidersForFileType('ipynb'), ['sql'], 'sql provider should be registered for ipynb extension'); + assert.deepStrictEqual(notebookService.getProvidersForFileType('.ipynb'), ['sql'], 'sql provider should be registered for ipynb extension'); assert.strictEqual(notebookService.getStandardKernelsForProvider('sql').length, 1, 'SQL kernel should be provided by default'); assert.strictEqual(notebookService.getStandardKernelsForProvider('otherProvider'), undefined, 'Other provider should not have kernels since it has not been added as a provider'); - assert.deepStrictEqual(notebookService.getSupportedFileExtensions(), ['IPYNB'], 'IPYNB file extension should be supported by default'); + assert.deepStrictEqual(notebookService.getSupportedFileExtensions(), ['.ipynb'], 'IPYNB file extension should be supported by default'); await notebookService.registrationComplete; assert.ok(notebookService.isRegistrationComplete, `notebookService.isRegistrationComplete should be true once its registrationComplete promise is resolved`); }); test('Validate another provider added successfully', async function (): Promise { await notebookService.registrationComplete; - assert.deepStrictEqual(notebookService.getProvidersForFileType('ipynb'), ['sql'], 'sql provider should be registered for ipynb extension'); + assert.deepStrictEqual(notebookService.getProvidersForFileType('.ipynb'), ['sql'], 'sql provider should be registered for ipynb extension'); const otherProviderRegistration: ProviderDescriptionRegistration = { fileExtensions: ['ipynb'], @@ -246,8 +246,8 @@ suite.skip('NotebookService:', function (): void { const notebookRegistry = Registry.as(NotebookProviderRegistryId); notebookRegistry.registerProviderDescription(otherProviderRegistration); - assert.deepStrictEqual(notebookService.getProvidersForFileType('ipynb'), ['sql', 'otherProvider'], 'otherProvider should also be registered for ipynb extension'); - assert.deepStrictEqual(notebookService.getSupportedFileExtensions(), ['IPYNB'], 'Only IPYNB should be registered as supported file extension'); + assert.deepStrictEqual(notebookService.getProvidersForFileType('.ipynb'), ['sql', 'otherProvider'], 'otherProvider should also be registered for ipynb extension'); + assert.deepStrictEqual(notebookService.getSupportedFileExtensions(), ['.ipynb'], 'Only IPYNB should be registered as supported file extension'); assert.strictEqual(notebookService.getStandardKernelsForProvider('otherProvider').length, 1, 'otherProvider kernel info could not be found'); assert.deepStrictEqual(notebookService.getStandardKernelsForProvider('otherProvider')[0], otherProviderRegistration.standardKernels[0], 'otherProviderRegistration standard kernels does not match'); }); diff --git a/src/sql/workbench/services/notebook/browser/models/notebookUtils.ts b/src/sql/workbench/services/notebook/browser/models/notebookUtils.ts index d87aaa0a3f..d1bb009c39 100644 --- a/src/sql/workbench/services/notebook/browser/models/notebookUtils.ts +++ b/src/sql/workbench/services/notebook/browser/models/notebookUtils.ts @@ -21,8 +21,7 @@ export function getProvidersForFileName(fileName: string, notebookService: INote let fileExt = path.extname(fileName); let providers: string[]; // First try to get provider for actual file type - if (fileExt && fileExt.startsWith('.')) { - fileExt = fileExt.slice(1, fileExt.length); + if (fileExt) { providers = notebookService.getProvidersForFileType(fileExt); } // Fallback to provider for default file type (assume this is a global handler) diff --git a/src/sql/workbench/services/notebook/browser/notebookService.ts b/src/sql/workbench/services/notebook/browser/notebookService.ts index 417df773a5..d20166bdfa 100644 --- a/src/sql/workbench/services/notebook/browser/notebookService.ts +++ b/src/sql/workbench/services/notebook/browser/notebookService.ts @@ -26,7 +26,7 @@ export const SERVICE_ID = 'sqlNotebookService'; export const INotebookService = createDecorator(SERVICE_ID); export const DEFAULT_NOTEBOOK_PROVIDER = 'builtin'; -export const DEFAULT_NOTEBOOK_FILETYPE = 'IPYNB'; +export const DEFAULT_NOTEBOOK_FILETYPE = '.ipynb'; export const SQL_NOTEBOOK_PROVIDER = 'sql'; export const OVERRIDE_EDITOR_THEMING_SETTING = 'notebook.overrideEditorTheming'; diff --git a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts index 6a9567f918..e0773e6468 100644 --- a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts +++ b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts @@ -311,7 +311,7 @@ export class NotebookService extends Disposable implements INotebookService { if (!this._serializationProviders.has(p.id)) { // Only add a new provider descriptor if the provider // supports file extensions beyond the default ipynb - let addNewProvider = extensions.some(ext => ext?.length > 0 && ext.toUpperCase() !== DEFAULT_NOTEBOOK_FILETYPE); + let addNewProvider = extensions.some(ext => ext?.length > 0 && ext.toLowerCase() !== DEFAULT_NOTEBOOK_FILETYPE); if (addNewProvider) { this._serializationProviders.set(p.id, new SerializationProviderDescriptor(p.id)); } @@ -378,12 +378,12 @@ export class NotebookService extends Disposable implements INotebookService { } private addFileProvider(fileType: string, provider: ProviderDescriptionRegistration) { - let providers = this._fileToProviderDescriptions.get(fileType.toUpperCase()); + let providers = this._fileToProviderDescriptions.get(fileType.toLowerCase()); if (!providers) { providers = []; } providers.push(provider); - this._fileToProviderDescriptions.set(fileType.toUpperCase(), providers); + this._fileToProviderDescriptions.set(fileType.toLowerCase(), providers); } // Standard kernels are contributed where a list of kernels are defined that can be shown @@ -411,7 +411,7 @@ export class NotebookService extends Disposable implements INotebookService { } getProvidersForFileType(fileType: string): string[] | undefined { - let providers = this._fileToProviderDescriptions.get(fileType.toUpperCase()); + let providers = this._fileToProviderDescriptions.get(fileType.toLowerCase()); return providers?.map(provider => provider.provider); } diff --git a/src/vs/workbench/api/browser/mainThreadEditors.ts b/src/vs/workbench/api/browser/mainThreadEditors.ts index c51d5cfc3c..b3f180dc74 100644 --- a/src/vs/workbench/api/browser/mainThreadEditors.ts +++ b/src/vs/workbench/api/browser/mainThreadEditors.ts @@ -147,7 +147,7 @@ export class MainThreadTextEditors implements MainThreadTextEditorsShape { // preserve pre 1.38 behaviour to not make group active when preserveFocus: true // but make sure to restore the editor to fix https://github.com/microsoft/vscode/issues/79633 activation: options.preserveFocus ? EditorActivation.RESTORE : undefined, - override: notebookFileTypes?.some(ext => uri?.fsPath?.toLowerCase().endsWith(ext)) || uri?.fsPath?.toLowerCase().endsWith('sql') ? undefined : EditorOverride.DISABLED // {{SQL CARBON EDIT}} + override: notebookFileTypes?.some(ext => uri?.fsPath?.toLowerCase().endsWith(ext)) || uri?.fsPath?.toLowerCase().endsWith('.sql') ? undefined : EditorOverride.DISABLED // {{SQL CARBON EDIT}} }; const input: IResourceEditorInput = { diff --git a/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts b/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts index f5fdf747f9..af18bd61b3 100644 --- a/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts +++ b/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts @@ -103,7 +103,7 @@ suite('MainThreadDocumentsAndEditors', () => { }, new TestPathService(), { // {{SQL CARBON EDIT}} - getSupportedFileExtensions: () => ['ipynb'] + getSupportedFileExtensions: () => ['.ipynb'] } ); });