Skip to content
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

Remove editorOpenWith #116856

Merged
merged 23 commits into from
Feb 22, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/vs/platform/editor/common/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,17 @@ export enum EditorActivation {
PRESERVE
}

export enum OverrideOptions {
export enum EditorOverride {

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

/**
* Disables overrides
*/
DISABLED,
DISABLED
}

export enum EditorOpenContext {
Expand Down Expand Up @@ -216,9 +218,9 @@ export interface IEditorOptions {
* Allows to override the editor that should be used to display the input:
* - `undefined`: let the editor decide for itself
* - `string`: specific override by id
* - `OverrideOptions`: Various options which can be given to dictate how overrides are handled
* - `EditorOverride`: specific override handling
*/
readonly override?: string | OverrideOptions;
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, OverrideOptions } 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: OverrideOptions.DISABLED
override: EditorOverride.DISABLED
};

const input: IResourceEditorInput = {
Expand Down
6 changes: 3 additions & 3 deletions src/vs/workbench/api/browser/mainThreadNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +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, OverrideOptions } from 'vs/platform/editor/common/editor';
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 Down Expand Up @@ -660,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: OverrideOptions.DISABLED,
override: EditorOverride.DISABLED,
};

const columnArg = viewColumnToEditorGroup(this._editorGroupsService, options.position);
Expand All @@ -682,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._editorService.openEditor(input, { override: 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, OverrideOptions } 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' ? OverrideOptions.DISABLED : undefined
override: typeof options.override === 'boolean' ? EditorOverride.DISABLED : undefined
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/browser/parts/editor/editorActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +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 { OverrideOptions } from 'vs/platform/editor/common/editor';
import { EditorOverride } from 'vs/platform/editor/common/editor';

export class ExecuteCommandAction extends Action {

Expand Down Expand Up @@ -1911,7 +1911,7 @@ export class ReopenResourcesAction extends Action {

const options = activeEditorPane.options;
const group = activeEditorPane.group;
await this.editorService.openEditor(activeInput, { ...options, override: OverrideOptions.PICKER }, group);
await this.editorService.openEditor(activeInput, { ...options, override: EditorOverride.PICK }, group);
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/vs/workbench/browser/parts/editor/editorCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,8 @@ function registerOpenEditorAPICommands(): void {
group = editorGroupsService.getGroup(viewColumnToEditorGroup(editorGroupsService, columnArg)) ?? editorGroupsService.activeGroup;
}

const textOptions: ITextEditorOptions = optionsArg ? optionsArg : {};

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

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/browser/parts/editor/editorDropTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +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 { OverrideOptions } from 'vs/platform/editor/common/editor';
import { EditorOverride } from 'vs/platform/editor/common/editor';

interface IDropOperation {
splitDirection?: GroupDirection;
Expand Down Expand Up @@ -285,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: OverrideOptions.DISABLED, // 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
10 changes: 4 additions & 6 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, OverrideOptions } 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 @@ -1007,9 +1007,9 @@ 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
* - `string`: specific override by id
* - `OverrideOptions`: Various options which can be given to dictate how overrides are handled
* - `EditorOverride`: specific override handling
*/
override?: string | OverrideOptions;
override: string | EditorOverride | undefined;

/**
* A optional hint to signal in which context the editor opens.
Expand Down Expand Up @@ -1642,12 +1642,10 @@ export function editorGroupToViewColumn(editorGroupService: IEditorGroupsService

export const customEditorsAssociationsSettingId = 'workbench.editorAssociations';

export const builtinProviderDisplayName = localize('builtinProviderDisplayName', "Built-in");

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

export type CustomEditorAssociation = {
Expand Down
14 changes: 7 additions & 7 deletions src/vs/workbench/contrib/customEditor/browser/customEditors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { RedoCommand, UndoCommand } from 'vs/editor/browser/editorExtensions';
import * as nls from 'vs/nls';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { EditorActivation, IEditorOptions, ITextEditorOptions, OverrideOptions } from 'vs/platform/editor/common/editor';
import { EditorActivation, IEditorOptions, ITextEditorOptions, EditorOverride } from 'vs/platform/editor/common/editor';
import { FileOperation, IFileService } from 'vs/platform/files/common/files';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { IQuickInputService, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput';
Expand Down Expand Up @@ -217,7 +217,7 @@ export class CustomEditorService extends Disposable implements ICustomEditorServ
): Promise<IEditorPane | undefined> {
if (viewType === defaultCustomEditor.id) {
const fileEditorInput = this.editorService.createEditorInput({ resource, forceFile: true });
return this.openEditorForResource(resource, fileEditorInput, { ...options, override: OverrideOptions.DISABLED }, group);
return this.openEditorForResource(resource, fileEditorInput, { ...options, override: EditorOverride.DISABLED }, group);
}

if (!this._contributedEditors.get(viewType)) {
Expand Down Expand Up @@ -402,7 +402,7 @@ export class CustomEditorService extends Disposable implements ICustomEditorServ
}

const targetGroup = group || this.editorGroupService.activeGroup;
const newEditor = await this.openEditorForResource(resource, editorToUse.editor, { ...options, override: OverrideOptions.DISABLED }, targetGroup);
const newEditor = await this.openEditorForResource(resource, editorToUse.editor, { ...options, override: EditorOverride.DISABLED }, targetGroup);
if (targetGroup.id !== editorToUse.group.id) {
editorToUse.group.closeEditor(editorToUse.editor);
}
Expand Down Expand Up @@ -507,7 +507,7 @@ export class CustomEditorContribution extends Disposable implements IWorkbenchCo

if (id) {
return {
override: this.customEditorService.openWith(resource, id, { ...options, override: OverrideOptions.DISABLED }, group)
override: this.customEditorService.openWith(resource, id, { ...options, override: EditorOverride.DISABLED }, group)
};
}

Expand Down Expand Up @@ -544,7 +544,7 @@ export class CustomEditorContribution extends Disposable implements IWorkbenchCo
return {
override: this.editorService.openEditor(existingEditorForResource, {
...options,
override: OverrideOptions.DISABLED,
override: EditorOverride.DISABLED,
activation: options?.preserveFocus ? EditorActivation.RESTORE : undefined,
}, group)
};
Expand Down Expand Up @@ -575,7 +575,7 @@ export class CustomEditorContribution extends Disposable implements IWorkbenchCo
// Open VS Code's standard editor but prompt user to see if they wish to use a custom one instead
return {
override: (async () => {
const standardEditor = await this.editorService.openEditor(editor, { ...options, override: OverrideOptions.DISABLED }, group);
const standardEditor = await this.editorService.openEditor(editor, { ...options, override: EditorOverride.DISABLED }, group);
// Give a moment to make sure the editor is showing.
// Otherwise the focus shift can cause the prompt to be dismissed right away.
await new Promise(resolve => setTimeout(resolve, 20));
Expand Down Expand Up @@ -643,7 +643,7 @@ export class CustomEditorContribution extends Disposable implements IWorkbenchCo
return {
override: (async () => {
const input = this.instantiationService.createInstance(DiffEditorInput, editor.getName(), editor.getDescription(), originalOverride || editor.originalInput, modifiedOverride || editor.modifiedInput, true);
return this.editorService.openEditor(input, { ...options, override: OverrideOptions.DISABLED }, group);
return this.editorService.openEditor(input, { ...options, override: EditorOverride.DISABLED }, group);
})(),
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { URI } from 'vs/base/common/uri';
import * as nls from 'vs/nls';
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
import { builtinProviderDisplayName } from 'vs/workbench/common/editor';
import { Memento } from 'vs/workbench/common/memento';
import { CustomEditorDescriptor, CustomEditorInfo, CustomEditorPriority } from 'vs/workbench/contrib/customEditor/common/customEditor';
import { customEditorsExtensionPoint, ICustomEditorsExtensionPoint } from 'vs/workbench/contrib/customEditor/common/extensionPoint';
Expand All @@ -18,7 +17,7 @@ import { IExtensionPointUser } from 'vs/workbench/services/extensions/common/ext
export const defaultCustomEditor = new CustomEditorInfo({
id: 'default',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Why does this constant need to be inlined?

Copy link
Member Author

@lramos15 lramos15 Feb 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It breaks layering, as everywhere else this is used is in browser

displayName: nls.localize('promptOpenWith.defaultEditor.displayName', "Text Editor"),
providerDisplayName: builtinProviderDisplayName,
providerDisplayName: nls.localize('builtinProviderDisplayName', "Built-in"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Try extracting this string since it it used in two places

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is probably on me, I wasn't sure about the benefit of sharing NLS strings here. If we have a concept of built-in custom editors, shouldn't we rather have a flag to indicate as such and then maybe put the label in one central place?

selector: [
{ filenamePattern: '*' }
],
Expand Down Expand Up @@ -59,7 +58,7 @@ export class ContributedCustomEditors extends Disposable {
this.add(new CustomEditorInfo({
id: webviewEditorContribution.viewType,
displayName: webviewEditorContribution.displayName,
providerDisplayName: extension.description.isBuiltin ? builtinProviderDisplayName : extension.description.displayName || extension.description.identifier.value,
providerDisplayName: extension.description.isBuiltin ? nls.localize('builtinProviderDisplayName', "Built-in") : extension.description.displayName || extension.description.identifier.value,
selector: webviewEditorContribution.selector || [],
priority: getPriorityFromContribution(webviewEditorContribution, extension.description),
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { IStorageService } from 'vs/platform/storage/common/storage';
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
import { OverrideOptions } from 'vs/platform/editor/common/editor';
import { EditorOverride } from 'vs/platform/editor/common/editor';

/**
* An implementation of editor for binary files that cannot be displayed.
Expand Down Expand Up @@ -51,7 +51,7 @@ export class BinaryFileEditor extends BaseBinaryResourceEditor {
input.setForceOpenAsText();

// If more editors are installed that can handle this input, show a picker
await this.editorService.openEditor(input, { override: OverrideOptions.PICKER, ...options }, this.group);
await this.editorService.openEditor(input, { ...options, override: EditorOverride.PICK, }, this.group);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/vs/workbench/contrib/files/browser/fileActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/ur
import { ResourceFileEdit } from 'vs/editor/browser/services/bulkEditService';
import { IExplorerService } from 'vs/workbench/contrib/files/browser/files';
import { listenStream } from 'vs/base/common/stream';
import { OverrideOptions } from 'vs/platform/editor/common/editor';
import { EditorOverride } from 'vs/platform/editor/common/editor';

export const NEW_FILE_COMMAND_ID = 'explorer.newFile';
export const NEW_FILE_LABEL = nls.localize('newFile', "New File");
Expand Down Expand Up @@ -467,7 +467,7 @@ export class GlobalCompareResourcesAction extends Action {
override: this.editorService.openEditor({
leftResource: activeResource,
rightResource: resource,
options: { override: OverrideOptions.DISABLED, pinned: true }
options: { override: EditorOverride.DISABLED, pinned: true }
})
};
}
Expand All @@ -477,7 +477,7 @@ export class GlobalCompareResourcesAction extends Action {
return {
override: this.editorService.openEditor({
resource: activeResource,
options: { override: OverrideOptions.DISABLED, pinned: true }
options: { override: EditorOverride.DISABLED, pinned: true }
})
};
}
Expand Down
8 changes: 5 additions & 3 deletions src/vs/workbench/contrib/files/browser/fileCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import { ITextFileService } from 'vs/workbench/services/textfile/common/textfile
import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity';
import { isPromiseCanceledError } from 'vs/base/common/errors';
import { toAction } from 'vs/base/common/actions';
import { OverrideOptions } from 'vs/platform/editor/common/editor';
import { EditorOverride } from 'vs/platform/editor/common/editor';

// Commands

Expand Down Expand Up @@ -358,9 +358,11 @@ CommandsRegistry.registerCommand({
const uri = getResourceForCommand(resource, accessor.get(IListService), accessor.get(IEditorService));
if (uri) {
const input = editorService.createEditorInput({ resource: uri });
return editorService.openEditor(input, { override: OverrideOptions.PICKER });

return editorService.openEditor(input, { override: EditorOverride.PICK });
}
return;

return undefined;
}
});

Expand Down
Loading