Skip to content

Add proposal for custom editor diff/merge priority#315321

Merged
mjbvz merged 1 commit into
microsoft:mainfrom
mjbvz:dev/mjbvz/grieving-toad
May 8, 2026
Merged

Add proposal for custom editor diff/merge priority#315321
mjbvz merged 1 commit into
microsoft:mainfrom
mjbvz:dev/mjbvz/grieving-toad

Conversation

@mjbvz
Copy link
Copy Markdown
Collaborator

@mjbvz mjbvz commented May 8, 2026

For #292379
Also related to work in #315174 and #138525

Let's a custom editor contribution give a different priority for diff/merge editors

Copilot AI review requested due to automatic review settings May 8, 2026 17:31
@mjbvz mjbvz enabled auto-merge May 8, 2026 17:31
@mjbvz mjbvz added this to the 1.120.0 milestone May 8, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a proposed mechanism for custom editors to use different default priorities depending on whether a resource is opened for editing vs diffing vs merging, addressing scenarios where a custom editor should be default for normal editing but not for diffs/merges.

Changes:

  • Adds diffEditorPriority and mergeEditorPriority support to registered editors and custom editor contributions.
  • Updates editor resolution logic to compute an “effective priority” based on editor context (edit/diff/merge).
  • Adds initial unit tests for diff-priority resolution and wires up a new customEditorPriority API proposal placeholder.
Show a summary per file
File Description
src/vscode-dts/vscode.proposed.customEditorPriority.d.ts Adds the proposed API placeholder entry for gating the manifest contribution changes.
src/vs/platform/extensions/common/extensionsApiProposals.ts Registers the customEditorPriority proposal so extensions can opt in via enabledApiProposals.
src/vs/workbench/services/editor/common/editorResolverService.ts Extends RegisteredEditorInfo with optional diff/merge priority fields.
src/vs/workbench/services/editor/browser/editorResolverService.ts Adds merge association type and “effective priority” handling for diff/merge resolution.
src/vs/workbench/services/editor/test/browser/editorResolverService.test.ts Adds tests intended to validate diff priority behavior (currently not asserting the right thing).
src/vs/workbench/contrib/customEditor/common/extensionPoint.ts Extends the contributes.customEditors schema with diffEditorPriority/mergeEditorPriority.
src/vs/workbench/contrib/customEditor/common/customEditor.ts Carries diff/merge priority fields through custom editor descriptor/info types.
src/vs/workbench/contrib/customEditor/common/contributedCustomEditors.ts Reads the new contribution fields when the proposal is enabled and maps them to registered editor priorities.
src/vs/workbench/contrib/customEditor/browser/customEditors.ts Passes diff/merge priorities into editor registration with the resolver.

Copilot's findings

Comments suppressed due to low confidence (1)

src/vs/workbench/services/editor/test/browser/editorResolverService.test.ts:767

  • This test currently asserts only that the resolved editor is a DiffEditorInput, which doesn’t validate the intended behavior (fallback from diffEditorPriority to priority). To make the test meaningful, assert that the correct registered editor’s createDiffEditorInput factory was invoked (e.g. with a call counter) and that the other factory was not invoked.
		// Diff editor should use custom editor since diffEditorPriority falls back to priority: default
		const diffResolution = await service.resolveEditor({
			original: { resource: URI.file('my://resource.test-no-diff-priority') },
			modified: { resource: URI.file('my://resource.test-no-diff-priority') }
		}, part.activeGroup);
		assert.ok(diffResolution);
		assert.notStrictEqual(typeof diffResolution, 'number');
		if (diffResolution !== ResolvedStatus.ABORT && diffResolution !== ResolvedStatus.NONE) {
			assert.strictEqual(diffResolution.editor.typeId, 'workbench.editors.diffEditorInput');
			diffResolution.editor.dispose();
  • Files reviewed: 8/9 changed files
  • Comments generated: 3

Comment on lines 275 to +283
private getAssociationsForResourceByType(resource: URI, associationType: EditorAssociationType): EditorAssociations {
if (associationType === EditorAssociationType.Editor) {
return this.getAssociationsForResource(resource);
if (associationType === EditorAssociationType.DiffEditor || associationType === EditorAssociationType.MergeEditor) {
const diffAssociations = this.getAssociationsForResourceFromSetting(resource, diffEditorsAssociationsSettingId);
if (diffAssociations.length) {
return diffAssociations;
}
}

const diffAssociations = this.getAssociationsForResourceFromSetting(resource, diffEditorsAssociationsSettingId);
return diffAssociations.length ? diffAssociations : this.getAssociationsForResource(resource);
return this.getAssociationsForResource(resource);
Comment on lines +716 to +726
// Diff editor should NOT use custom editor (diffEditorPriority: option)
const diffResolution = await service.resolveEditor({
original: { resource: URI.file('my://resource.test-diff-priority') },
modified: { resource: URI.file('my://resource.test-diff-priority') }
}, part.activeGroup);
assert.ok(diffResolution);
// With diffEditorPriority: option, the custom editor should not be selected as default
if (diffResolution !== ResolvedStatus.ABORT && diffResolution !== ResolvedStatus.NONE) {
assert.notStrictEqual(diffResolution.editor.typeId, CUSTOM_EDITOR_INPUT_ID,
'Custom editor with diffEditorPriority:option should not be used for diffs');
diffResolution.editor.dispose();
Comment on lines +517 to +526
private getEffectivePriority(editorInfo: RegisteredEditorInfo, associationType: EditorAssociationType): RegisteredEditorPriority {
switch (associationType) {
case EditorAssociationType.DiffEditor:
return editorInfo.diffEditorPriority ?? editorInfo.priority;
case EditorAssociationType.MergeEditor:
return editorInfo.mergeEditorPriority ?? editorInfo.priority;
default:
return editorInfo.priority;
}
}
@mjbvz mjbvz merged commit 56edb5e into microsoft:main May 8, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants