From 65fb22cc7c36db9c53af1ed2fdbdf48f66c682be Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Wed, 28 Jul 2021 09:07:43 -0700 Subject: [PATCH] Fix SQL/Notebook editors opening as plaintext (#16442) (#16456) * Register overrides at startup * Always wait for extensions * Fix compile errors (cherry picked from commit 2d8e0d648aeae043e342b3a3ca500285bfcde659) --- .../notebook/browser/notebook.contribution.ts | 77 ++++++++++--------- .../query/browser/query.contribution.ts | 74 +++++++++--------- .../editor/browser/editorOverrideService.ts | 14 ++-- 3 files changed, 85 insertions(+), 80 deletions(-) diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts index e900286243..c757ebc9c6 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -677,47 +677,48 @@ export class NotebookEditorOverrideContribution extends Disposable implements IW @IModeService private _modeService: IModeService ) { super(); - this.registerEditorOverride(); - } - - private registerEditorOverride(): void { + this.registerEditorOverrides(); // Refresh the editor overrides whenever the languages change so we ensure we always have // the latest up to date list of extensions for each language this._modeService.onLanguagesMaybeChanged(() => { - this._registeredOverrides.clear(); - // List of language IDs to associate the query editor for. These are case sensitive. - NotebookEditorInputAssociation.languages.map(lang => { - const langExtensions = this._modeService.getExtensions(lang); - if (langExtensions.length === 0) { - return; + 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 => { + 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.registerContributionPoint( + 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 }; } - // 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.registerContributionPoint( - 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 }; - } - )); - }); + )); }); } @@ -733,4 +734,4 @@ export class NotebookEditorOverrideContribution extends Disposable implements IW } Registry.as(WorkbenchExtensions.Workbench) - .registerWorkbenchContribution(NotebookEditorOverrideContribution, LifecyclePhase.Restored); + .registerWorkbenchContribution(NotebookEditorOverrideContribution, LifecyclePhase.Starting); diff --git a/src/sql/workbench/contrib/query/browser/query.contribution.ts b/src/sql/workbench/contrib/query/browser/query.contribution.ts index 70f0c55eee..54f6087904 100644 --- a/src/sql/workbench/contrib/query/browser/query.contribution.ts +++ b/src/sql/workbench/contrib/query/browser/query.contribution.ts @@ -510,47 +510,47 @@ export class QueryEditorOverrideContribution extends Disposable implements IWork ) { super(); this.registerEditorOverrides(); - } - - private registerEditorOverrides(): void { // Refresh the editor overrides whenever the languages change so we ensure we always have // the latest up to date list of extensions for each language this._modeService.onLanguagesMaybeChanged(() => { - this._registeredOverrides.clear(); - // List of language IDs to associate the query editor for. These are case sensitive. - QueryEditorLanguageAssociation.languages.map(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 - // query editor (filtering out any languages which didn't have any extensions registered yet) - const selector = `*{${langExtensions.join(',')}}`; - this._registeredOverrides.add(this._editorOverrideService.registerContributionPoint( - selector, - { - id: QueryEditor.ID, - label: QueryEditor.LABEL, - describes: (currentEditor) => currentEditor instanceof FileQueryEditorInput, - priority: ContributedEditorPriority.builtin - }, - {}, - (resource, options, group) => { - const fileInput = this._editorService.createEditorInput({ - resource: resource - }) as FileEditorInput; - const langAssociation = languageAssociationRegistry.getAssociationForLanguage(lang); - const queryEditorInput = langAssociation?.syncConvertinput?.(fileInput); - if (!queryEditorInput) { - this._logService.warn('Unable to create input for overriding editor ', resource); - return undefined; - } - return { editor: queryEditorInput, options: options, group: group }; + this.registerEditorOverrides(); + }); + } + + private registerEditorOverrides(): void { + this._registeredOverrides.clear(); + // List of language IDs to associate the query editor for. These are case sensitive. + QueryEditorLanguageAssociation.languages.map(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 + // query editor (filtering out any languages which didn't have any extensions registered yet) + const selector = `*{${langExtensions.join(',')}}`; + this._registeredOverrides.add(this._editorOverrideService.registerContributionPoint( + selector, + { + id: QueryEditor.ID, + label: QueryEditor.LABEL, + describes: (currentEditor) => currentEditor instanceof FileQueryEditorInput, + priority: ContributedEditorPriority.builtin + }, + {}, + (resource, options, group) => { + const fileInput = this._editorService.createEditorInput({ + resource: resource + }) as FileEditorInput; + const langAssociation = languageAssociationRegistry.getAssociationForLanguage(lang); + const queryEditorInput = langAssociation?.syncConvertinput?.(fileInput); + if (!queryEditorInput) { + this._logService.warn('Unable to create input for overriding editor ', resource); + return undefined; } - )); - }); + return { editor: queryEditorInput, options: options, group: group }; + } + )); }); } } - -workbenchRegistry.registerWorkbenchContribution(QueryEditorOverrideContribution, LifecyclePhase.Restored); +workbenchRegistry.registerWorkbenchContribution(QueryEditorOverrideContribution, LifecyclePhase.Starting); diff --git a/src/vs/workbench/services/editor/browser/editorOverrideService.ts b/src/vs/workbench/services/editor/browser/editorOverrideService.ts index 4c4c860182..5f91483a16 100644 --- a/src/vs/workbench/services/editor/browser/editorOverrideService.ts +++ b/src/vs/workbench/services/editor/browser/editorOverrideService.ts @@ -43,7 +43,7 @@ export class EditorOverrideService extends Disposable implements IEditorOverride private _contributionPoints: Map = new Map(); private static readonly overrideCacheStorageID = 'editorOverrideService.cache'; - private cache: Set | undefined; + // private cache: Set | undefined; {{SQL CARBON EDIT}} Remove unused constructor( @IEditorGroupsService private readonly editorGroupService: IEditorGroupsService, @@ -56,7 +56,7 @@ export class EditorOverrideService extends Disposable implements IEditorOverride ) { super(); // Read in the cache on statup - this.cache = new Set(JSON.parse(this.storageService.get(EditorOverrideService.overrideCacheStorageID, StorageScope.GLOBAL, JSON.stringify([])))); + // this.cache = new Set(JSON.parse(this.storageService.get(EditorOverrideService.overrideCacheStorageID, StorageScope.GLOBAL, JSON.stringify([])))); {{SQL CARBON EDIT}} Remove unused this.storageService.remove(EditorOverrideService.overrideCacheStorageID, StorageScope.GLOBAL); this._register(this.storageService.onWillSaveState(() => { @@ -65,16 +65,18 @@ export class EditorOverrideService extends Disposable implements IEditorOverride })); // When extensions have registered we no longer need the cache + /* {{SQL CARBON EDIT}} Remove unused this.extensionService.onDidRegisterExtensions(() => { this.cache = undefined; }); + */ } async resolveEditorOverride(editor: IEditorInput, options: IEditorOptions | ITextEditorOptions | undefined, group: IEditorGroup): Promise { // If it was an override before we await for the extensions to activate and then proceed with overriding or else they won't be registered - if (this.cache && editor.resource && this.resourceMatchesCache(editor.resource)) { - await this.extensionService.whenInstalledExtensionsRegistered(); - } + //if (this.cache && editor.resource && this.resourceMatchesCache(editor.resource)) { // {{SQL CARBON EDIT}} Always wait for extensions so that our language-based overrides (SQL/Notebooks) will always have those registered + await this.extensionService.whenInstalledExtensionsRegistered(); + //} if (options?.override === EditorOverride.DISABLED) { throw new Error(`Calling resolve editor override when override is explicitly disabled!`); @@ -547,6 +549,7 @@ export class EditorOverrideService extends Disposable implements IEditorOverride this.storageService.store(EditorOverrideService.overrideCacheStorageID, JSON.stringify(Array.from(cacheStorage)), StorageScope.GLOBAL, StorageTarget.MACHINE); } + /* {{SQL CARBON EDIT}} Remove unused private resourceMatchesCache(resource: URI): boolean { if (!this.cache) { return false; @@ -559,6 +562,7 @@ export class EditorOverrideService extends Disposable implements IEditorOverride } return false; } + */ } registerSingleton(IEditorOverrideService, EditorOverrideService);