Skip to content

Commit

Permalink
Remove editorOpenWith (#116856)
Browse files Browse the repository at this point in the history
* Move editorOpenWith to the editorService

* Remove custom editor input handling

* Fix open with API command

* adopt master properly

* Remove dnagling comment

* Address some feedback

* Cleanup region comment.

Co-authored-by: Benjamin Pasero <benjamin.pasero@gmail.com>

* Cleanup region comment.

Co-authored-by: Benjamin Pasero <benjamin.pasero@gmail.com>

* Update override typings

* Fix missing picker

* Fix bug with reopen with

* Remove duplicate import

* First round of feedback via changes
- OverrideOptions => EditorOverride
- It is safe to destructure (...) undefined
- Ensure when destructuring, override is always winning
- Not a big fan of shared builtinProviderDisplayName import

* editorservice - static DEFAULT_EDITOR_OVERRIDE_ID

* editors - introduce a editor associations registry

This moves the relevant code out of the editor service.

* cleanup editor picking

* cleanup editor delegate

* final 💄

Co-authored-by: Benjamin Pasero <benjpas@microsoft.com>
Co-authored-by: Benjamin Pasero <benjamin.pasero@gmail.com>
  • Loading branch information
3 people authored Feb 22, 2021
1 parent cdd7066 commit 413963c
Show file tree
Hide file tree
Showing 25 changed files with 484 additions and 515 deletions.
17 changes: 15 additions & 2 deletions src/vs/platform/editor/common/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,19 @@ export enum EditorActivation {
PRESERVE
}

export enum EditorOverride {

/**
* Displays a picker and allows the user to decide which editor to use
*/
PICK = 1,

/**
* Disables overrides
*/
DISABLED
}

export enum EditorOpenContext {

/**
Expand Down Expand Up @@ -204,10 +217,10 @@ export interface IEditorOptions {
/**
* Allows to override the editor that should be used to display the input:
* - `undefined`: let the editor decide for itself
* - `false`: disable overrides
* - `string`: specific override by id
* - `EditorOverride`: specific override handling
*/
readonly override?: false | string;
readonly override?: string | EditorOverride;

/**
* A optional hint to signal in which context the editor opens.
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/browser/mainThreadEditors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ISelection } from 'vs/editor/common/core/selection';
import { IDecorationOptions, IDecorationRenderOptions, ILineChange } from 'vs/editor/common/editorCommon';
import { ISingleEditOperation } from 'vs/editor/common/model';
import { CommandsRegistry } from 'vs/platform/commands/common/commands';
import { ITextEditorOptions, IResourceEditorInput, EditorActivation } from 'vs/platform/editor/common/editor';
import { ITextEditorOptions, IResourceEditorInput, EditorActivation, EditorOverride } from 'vs/platform/editor/common/editor';
import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
import { MainThreadDocumentsAndEditors } from 'vs/workbench/api/browser/mainThreadDocumentsAndEditors';
import { MainThreadTextEditor } from 'vs/workbench/api/browser/mainThreadEditor';
Expand Down Expand Up @@ -142,7 +142,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: false
override: EditorOverride.DISABLED
};

const input: IResourceEditorInput = {
Expand Down
11 changes: 4 additions & 7 deletions src/vs/workbench/api/browser/mainThreadNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ import { Schemas } from 'vs/base/common/network';
import { isEqual } from 'vs/base/common/resources';
import { URI, UriComponents } from 'vs/base/common/uri';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { EditorActivation, ITextEditorOptions } from 'vs/platform/editor/common/editor';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { EditorActivation, ITextEditorOptions, EditorOverride } from 'vs/platform/editor/common/editor';
import { ILogService } from 'vs/platform/log/common/log';
import { BoundModelReferenceCollection } from 'vs/workbench/api/browser/mainThreadDocuments';
import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
Expand All @@ -27,7 +26,6 @@ import { ICellEditOperation, ICellRange, IEditor, IMainCellDto, INotebookDecorat
import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService';
import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
import { IEditorGroup, IEditorGroupsService, preferredSideBySideGroupDirection } from 'vs/workbench/services/editor/common/editorGroupsService';
import { openEditorWith } from 'vs/workbench/services/editor/common/editorOpenWith';
import { IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService';
import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity';
import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
Expand Down Expand Up @@ -128,8 +126,7 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
@ILogService private readonly _logService: ILogService,
@INotebookCellStatusBarService private readonly _cellStatusBarService: INotebookCellStatusBarService,
@INotebookEditorModelResolverService private readonly _notebookModelResolverService: INotebookEditorModelResolverService,
@IUriIdentityService private readonly _uriIdentityService: IUriIdentityService,
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@IUriIdentityService private readonly _uriIdentityService: IUriIdentityService
) {
super();
this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook);
Expand Down Expand Up @@ -663,7 +660,7 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
// 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: false,
override: EditorOverride.DISABLED,
};

const columnArg = viewColumnToEditorGroup(this._editorGroupsService, options.position);
Expand All @@ -685,7 +682,7 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
const input = this._editorService.createEditorInput({ resource: URI.revive(resource), options: editorOptions });

// TODO: handle options.selection
const editorPane = await this._instantiationService.invokeFunction(openEditorWith, input, viewType, options, group);
const editorPane = await this._editorService.openEditor(input, { ...options, override: viewType }, group);
const notebookEditor = (editorPane as unknown as { isNotebookEditor?: boolean })?.isNotebookEditor ? (editorPane!.getControl() as INotebookEditor) : undefined;

if (notebookEditor) {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/common/extHostTypeConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import * as modes from 'vs/editor/common/modes';
import * as types from './extHostTypes';
import * as search from 'vs/workbench/contrib/search/common/search';
import { ITextEditorOptions } from 'vs/platform/editor/common/editor';
import { ITextEditorOptions, EditorOverride } from 'vs/platform/editor/common/editor';
import { IDecorationOptions, IThemeDecorationRenderOptions, IDecorationRenderOptions, IContentDecorationRenderOptions } from 'vs/editor/common/editorCommon';
import { EndOfLineSequence, TrackedRangeStickiness } from 'vs/editor/common/model';
import type * as vscode from 'vscode';
Expand Down Expand Up @@ -1344,7 +1344,7 @@ export namespace TextEditorOpenOptions {
inactive: options.background,
preserveFocus: options.preserveFocus,
selection: typeof options.selection === 'object' ? Range.from(options.selection) : undefined,
override: typeof options.override === 'boolean' ? false : undefined
override: typeof options.override === 'boolean' ? EditorOverride.DISABLED : undefined
};
}

Expand Down
155 changes: 152 additions & 3 deletions src/vs/workbench/browser/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,36 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { localize } from 'vs/nls';
import { Event } from 'vs/base/common/event';
import { EditorInput } from 'vs/workbench/common/editor';
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { Registry } from 'vs/platform/registry/common/platform';
import { EditorPane } from 'vs/workbench/browser/parts/editor/editorPane';
import { IConstructorSignature0, IInstantiationService, BrandedService } from 'vs/platform/instantiation/common/instantiation';
import { insert } from 'vs/base/common/arrays';
import { IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { IJSONSchema } from 'vs/base/common/jsonSchema';
import { workbenchConfigurationNodeBase } from 'vs/workbench/common/configuration';
import { Extensions as ConfigurationExtensions, IConfigurationNode, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry';

export const Extensions = {
Editors: 'workbench.contributions.editors',
Associations: 'workbench.editors.associations'
};

//#region Editors Registry

export interface IEditorDescriptor {

/**
* The unique identifier of the editor
*/
getId(): string;

/**
* The display name of the editor
*/
getName(): string;

instantiate(instantiationService: IInstantiationService): EditorPane;
Expand Down Expand Up @@ -174,8 +194,137 @@ class EditorRegistry implements IEditorRegistry {
}
}

export const Extensions = {
Editors: 'workbench.contributions.editors'
Registry.add(Extensions.Editors, new EditorRegistry());

//#endregion


//#region Editor Associations

export const editorsAssociationsSettingId = 'workbench.editorAssociations';

export const DEFAULT_EDITOR_ASSOCIATION: IEditorType = {
id: 'default',
displayName: localize('promptOpenWith.defaultEditor.displayName', "Text Editor"),
providerDisplayName: localize('builtinProviderDisplayName', "Built-in")
};

Registry.add(Extensions.Editors, new EditorRegistry());
export type EditorAssociation = {
readonly editorType: string;
readonly filenamePattern?: string;
};

export type EditorsAssociations = readonly EditorAssociation[];

const configurationRegistry = Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration);

const editorTypeSchemaAddition: IJSONSchema = {
type: 'string',
enum: []
};

const editorAssociationsConfigurationNode: IConfigurationNode = {
...workbenchConfigurationNodeBase,
properties: {
'workbench.editorAssociations': {
type: 'array',
markdownDescription: localize('editor.editorAssociations', "Configure which editor to use for specific file types."),
items: {
type: 'object',
defaultSnippets: [{
body: {
'viewType': '$1',
'filenamePattern': '$2'
}
}],
properties: {
'viewType': {
anyOf: [
{
type: 'string',
description: localize('editor.editorAssociations.viewType', "The unique id of the editor to use."),
},
editorTypeSchemaAddition
]
},
'filenamePattern': {
type: 'string',
description: localize('editor.editorAssociations.filenamePattern', "Glob pattern specifying which files the editor should be used for."),
}
}
}
}
}
};

export interface IEditorType {
readonly id: string;
readonly displayName: string;
readonly providerDisplayName: string;
}

export interface IEditorTypesHandler {
readonly onDidChangeEditorTypes: Event<void>;

getEditorTypes(): IEditorType[];
}

export interface IEditorAssociationsRegistry {

/**
* Register handlers for editor types
*/
registerEditorTypesHandler(id: string, handler: IEditorTypesHandler): IDisposable;
}

class EditorAssociationsRegistry implements IEditorAssociationsRegistry {

private readonly editorTypesHandlers = new Map<string, IEditorTypesHandler>();

registerEditorTypesHandler(id: string, handler: IEditorTypesHandler): IDisposable {
if (this.editorTypesHandlers.has(id)) {
throw new Error(`An editor type handler with ${id} was already registered.`);
}

this.editorTypesHandlers.set(id, handler);
this.updateEditorAssociationsSchema();

const editorTypeChangeEvent = handler.onDidChangeEditorTypes(() => {
this.updateEditorAssociationsSchema();
});

return {
dispose: () => {
editorTypeChangeEvent.dispose();
this.editorTypesHandlers.delete(id);
this.updateEditorAssociationsSchema();
}
};
}

private updateEditorAssociationsSchema() {
const enumValues: string[] = [];
const enumDescriptions: string[] = [];

const editorTypes: IEditorType[] = [DEFAULT_EDITOR_ASSOCIATION];

for (const [, handler] of this.editorTypesHandlers) {
editorTypes.push(...handler.getEditorTypes());
}

for (const { id, providerDisplayName } of editorTypes) {
enumValues.push(id);
enumDescriptions.push(localize('editorAssociations.editorType.sourceDescription', "Source: {0}", providerDisplayName));
}

editorTypeSchemaAddition.enum = enumValues;
editorTypeSchemaAddition.enumDescriptions = enumDescriptions;

configurationRegistry.notifyConfigurationSchemaUpdated(editorAssociationsConfigurationNode);
}
}

Registry.add(Extensions.Associations, new EditorAssociationsRegistry());
configurationRegistry.registerConfiguration(editorAssociationsConfigurationNode);

//#endregion
10 changes: 4 additions & 6 deletions src/vs/workbench/browser/parts/editor/editorActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import { ItemActivation, IQuickInputService } from 'vs/platform/quickinput/commo
import { AllEditorsByMostRecentlyUsedQuickAccess, ActiveGroupEditorsByMostRecentlyUsedQuickAccess, AllEditorsByAppearanceQuickAccess } from 'vs/workbench/browser/parts/editor/editorQuickAccess';
import { Codicon } from 'vs/base/common/codicons';
import { IFilesConfigurationService, AutoSaveMode } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService';
import { openEditorWith, getAllAvailableEditors } from 'vs/workbench/services/editor/common/editorOpenWith';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { EditorOverride } from 'vs/platform/editor/common/editor';

export class ExecuteCommandAction extends Action {

Expand Down Expand Up @@ -1896,8 +1895,7 @@ export class ReopenResourcesAction extends Action {
constructor(
id: string,
label: string,
@IEditorService private readonly editorService: IEditorService,
@IInstantiationService private readonly instantiationService: IInstantiationService,
@IEditorService private readonly editorService: IEditorService
) {
super(id, label);
}
Expand All @@ -1915,7 +1913,7 @@ export class ReopenResourcesAction extends Action {

const options = activeEditorPane.options;
const group = activeEditorPane.group;
await this.instantiationService.invokeFunction(openEditorWith, activeInput, undefined, options, group);
await this.editorService.openEditor(activeInput, { ...options, override: EditorOverride.PICK }, group);
}
}

Expand Down Expand Up @@ -1946,7 +1944,7 @@ export class ToggleEditorTypeAction extends Action {
const options = activeEditorPane.options;
const group = activeEditorPane.group;

const overrides = getAllAvailableEditors(activeEditorResource, undefined, options, group, this.editorService);
const overrides = this.editorService.getEditorOverrides(activeEditorResource, options, group);
const firstNonActiveOverride = overrides.find(([_, entry]) => !entry.active);
if (!firstNonActiveOverride) {
return;
Expand Down
5 changes: 1 addition & 4 deletions src/vs/workbench/browser/parts/editor/editorCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import { MenuRegistry, MenuId } from 'vs/platform/actions/common/actions';
import { ActiveGroupEditorsByMostRecentlyUsedQuickAccess } from 'vs/workbench/browser/parts/editor/editorQuickAccess';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { ITextEditorOptions } from 'vs/platform/editor/common/editor';
import { openEditorWith } from 'vs/workbench/services/editor/common/editorOpenWith';

export const CLOSE_SAVED_EDITORS_COMMAND_ID = 'workbench.action.closeUnmodifiedEditors';
export const CLOSE_EDITORS_IN_GROUP_COMMAND_ID = 'workbench.action.closeEditorsInGroup';
Expand Down Expand Up @@ -499,10 +498,8 @@ function registerOpenEditorAPICommands(): void {
group = editorGroupsService.getGroup(viewColumnToEditorGroup(editorGroupsService, columnArg)) ?? editorGroupsService.activeGroup;
}

const textOptions: ITextEditorOptions = optionsArg ? { ...optionsArg, override: false } : { override: false };

const input = editorService.createEditorInput({ resource: URI.revive(resource) });
return openEditorWith(accessor, input, id, textOptions, group);
return editorService.openEditor(input, { ...optionsArg, override: id }, group);
});
}

Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/browser/parts/editor/editorDropTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { assertIsDefined, assertAllDefined } from 'vs/base/common/types';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { localize } from 'vs/nls';
import { ByteSize } from 'vs/platform/files/common/files';
import { EditorOverride } from 'vs/platform/editor/common/editor';

interface IDropOperation {
splitDirection?: GroupDirection;
Expand Down Expand Up @@ -284,7 +285,7 @@ class DropOverlay extends Themable {
const options = getActiveTextEditorOptions(sourceGroup, draggedEditor.editor, EditorOptions.create({
pinned: true, // always pin dropped editor
sticky: sourceGroup.isSticky(draggedEditor.editor), // preserve sticky state
override: false, // Use `draggedEditor.editor` as is. If it is already a custom editor, it will stay so.
override: EditorOverride.DISABLED // preserve editor type
}));
const copyEditor = this.isCopyOperation(event, draggedEditor);
targetGroup.openEditor(draggedEditor.editor, options, copyEditor ? OpenEditorContext.COPY_EDITOR : OpenEditorContext.MOVE_EDITOR);
Expand Down
8 changes: 4 additions & 4 deletions src/vs/workbench/common/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { withNullAsUndefined, assertIsDefined } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
import { IDisposable, Disposable, toDisposable } from 'vs/base/common/lifecycle';
import { IEditor, IEditorViewState, ScrollType, IDiffEditor } from 'vs/editor/common/editorCommon';
import { IEditorModel, IEditorOptions, ITextEditorOptions, IBaseResourceEditorInput, IResourceEditorInput, EditorActivation, EditorOpenContext, ITextEditorSelection, TextEditorSelectionRevealType } from 'vs/platform/editor/common/editor';
import { IEditorModel, IEditorOptions, ITextEditorOptions, IBaseResourceEditorInput, IResourceEditorInput, EditorActivation, EditorOpenContext, ITextEditorSelection, TextEditorSelectionRevealType, EditorOverride } from 'vs/platform/editor/common/editor';
import { IInstantiationService, IConstructorSignature0, ServicesAccessor, BrandedService } from 'vs/platform/instantiation/common/instantiation';
import { IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey';
import { Registry } from 'vs/platform/registry/common/platform';
Expand Down Expand Up @@ -1006,10 +1006,10 @@ export class EditorOptions implements IEditorOptions {
/**
* Allows to override the editor that should be used to display the input:
* - `undefined`: let the editor decide for itself
* - `false`: disable overrides
* - `string`: specific override by id
* - `EditorOverride`: specific override handling
*/
override?: false | string;
override: string | EditorOverride | undefined;

/**
* A optional hint to signal in which context the editor opens.
Expand Down Expand Up @@ -1067,7 +1067,7 @@ export class EditorOptions implements IEditorOptions {
this.index = options.index;
}

if (typeof options.override === 'string' || options.override === false) {
if (options.override !== undefined) {
this.override = options.override;
}

Expand Down
Loading

0 comments on commit 413963c

Please sign in to comment.