Show binary files in multi-diff editors

@@ -8,13 +8,15 @@ import { Codicon } from '../../../../base/common/codicons.js'; import { BugIndicatingError } from '../../../../base/common/errors.js'; import { DisposableStore, MutableDisposable } from '../../../../base/common/lifecycle.js'; import { autorun, derived, globalTransaction, IObservable, observableValue } from '../../../../base/common/observable.js';+import { localize } from '../../../../nls.js'; import { createActionViewItem } from '../../../../platform/actions/browser/menuEntryActionViewItem.js'; import { MenuWorkbenchToolBar } from '../../../../platform/actions/browser/toolbar.js'; import { MenuId } from '../../../../platform/actions/common/actions.js'; import { IContextKeyService, type IScopedContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { EditorContextKeys } from '../../../common/editorContextKeys.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { ServiceCollection } from '../../../../platform/instantiation/common/serviceCollection.js';+import { defaultButtonStyles } from '../../../../platform/theme/browser/defaultStyles.js'; import { IDiffEditorOptions } from '../../../common/config/editorOptions.js'; import { OffsetRange } from '../../../common/core/ranges/offsetRange.js'; import { observableCodeEditor } from '../../observableCodeEditor.js';@@ -24,6 +26,8 @@ import { ActionRunnerWithContext } from './utils.js'; import { IVirtualizedItemBindingContext, VirtualizedItemBinding, VirtualizedItemTemplate } from './virtualizedItemManager.js'; import { IWorkbenchUIElementFactory, MultiDiffEditorItemLabelKind } from './workbenchUIElementFactory.js'; +export const binaryFilePlaceholderContentHeight = 100;+ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiffItemViewModel, DiffEditorItemBinding> { 	private readonly _viewModel; @@ -46,11 +50,13 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff  	private readonly isModifedFocused; 	private readonly isOriginalFocused;+	private readonly isBinaryFilePlaceholderFocused; 	public readonly isFocused;  	private readonly _resourceLabel;  	private readonly _resourceLabel2;+	private readonly _openBinaryDiffButton: Button | undefined; 	private readonly _verticalStateUpdate = this._register(new MutableDisposable()); 	private _observedEditorContentHeight = 500; 	private _isSettingData = false;@@ -91,6 +97,7 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff 				return { maxScroll: scroll2, width: this._originalWidth.read(reader) }; 			} 		});+		const binaryFileChangedLabel = localize('binaryFileChanged', "Binary file changed"); 		this._elements = h('div.multiDiffEntry', [ 			h('div.header@header', [ 				h('div.header-content', [@@ -108,6 +115,12 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff  			h('div.editorParent', [ 				h('div.editorContainer@editor'),+				h('div.binary-file-placeholder@binaryFilePlaceholder', { role: 'group', 'aria-label': binaryFileChangedLabel }, [+					h('div.binary-file-placeholder-content', [+						h('span', [binaryFileChangedLabel]),+						h('div.binary-file-placeholder-actions@binaryFilePlaceholderActions'),+					]),+				]), 			]) 		]) as Record<string, HTMLElement>; 		this.editor = this._register(this._instantiationService.createInstance(DiffEditorWidget, this._elements.editor, {@@ -125,7 +138,28 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff 		})); 		this.isModifedFocused = observableCodeEditor(this.editor.getModifiedEditor()).isFocused; 		this.isOriginalFocused = observableCodeEditor(this.editor.getOriginalEditor()).isFocused;-		this.isFocused = derived(this, reader => this.isModifedFocused.read(reader) || this.isOriginalFocused.read(reader));+		this.isBinaryFilePlaceholderFocused = observableValue(this, false);+		const binaryFilePlaceholderFocus = this._register(trackFocus(this._elements.binaryFilePlaceholder));+		this._register(binaryFilePlaceholderFocus.onDidFocus(() => this.isBinaryFilePlaceholderFocused.set(true, undefined)));+		this._register(binaryFilePlaceholderFocus.onDidBlur(() => this.isBinaryFilePlaceholderFocused.set(false, undefined)));+		this.isFocused = derived(this, reader =>+			this.isModifedFocused.read(reader)+			|| this.isOriginalFocused.read(reader)+			|| this.isBinaryFilePlaceholderFocused.read(reader)+		);+		this._elements.binaryFilePlaceholder.tabIndex = 0;+		if (this._workbenchUIElementFactory.openDiffEditor) {+			this._openBinaryDiffButton = this._register(new Button(this._elements.binaryFilePlaceholderActions, { ...defaultButtonStyles, secondary: true }));+			this._openBinaryDiffButton.label = localize('openBinaryDiff', "Open Diff");+			this._register(this._openBinaryDiffButton.onDidClick(() => {+				const item = this._viewModel.get();+				if (item?.originalUri && item.modifiedUri) {+					this._workbenchUIElementFactory.openDiffEditor?.(item.originalUri, item.modifiedUri);+				}+			}));+		} else {+			this._openBinaryDiffButton = undefined;+		} 		this._resourceLabel = this._workbenchUIElementFactory.createResourceLabel 			? this._register(this._workbenchUIElementFactory.createResourceLabel(this._elements.primaryPath, MultiDiffEditorItemLabelKind.Primary)) 			: undefined;@@ -202,7 +236,13 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff  		this._register(autorun(reader => { 			const collapsed = this._collapsed.read(reader);-			this._elements.editor.style.display = collapsed ? 'none' : 'block';+			const item = this._viewModel.read(reader);+			const isBinary = item?.isBinary === true;+			const canOpenDiff = !!(item?.originalUri && item.modifiedUri && this._openBinaryDiffButton);+			this._elements.editor.style.display = collapsed || isBinary ? 'none' : 'block';+			this._elements.binaryFilePlaceholder.style.display = !collapsed && isBinary ? 'grid' : 'none';+			this._elements.binaryFilePlaceholder.tabIndex = canOpenDiff ? -1 : 0;+			this._elements.binaryFilePlaceholderActions.style.display = canOpenDiff ? '' : 'none'; 			if (this._workbenchUIElementFactory.headerClickToCollapse) { 				this._elements.header.setAttribute('aria-expanded', String(!collapsed)); 			}@@ -224,7 +264,7 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff 				this._originalContentWidth.set(this.editor.getOriginalEditor().getContentWidth(), tx); 			}); 			const viewModel = this._viewModel.get();-			if (this._isSettingData || !viewModel?.diffEditorViewModel.isDiffUpToDate.get()) {+			if (this._isSettingData || viewModel?.isBinary || !viewModel?.diffEditorViewModel.isDiffUpToDate.get()) { 				return; 			} 			this._observedEditorContentHeight = e.contentHeight;@@ -356,7 +396,9 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff 		}  		const value = item.documentDiffItem;-		const editorContentHeight = Math.max(0, Math.max(initialSize, item.lastTemplateData.get().expandedContentHeight) - this._outerEditorHeight);+		const editorContentHeight = item.isBinary+			? binaryFilePlaceholderContentHeight+			: Math.max(0, Math.max(initialSize, item.lastTemplateData.get().expandedContentHeight) - this._outerEditorHeight); 		this._observedEditorContentHeight = editorContentHeight; 		this._isSettingData = true; 		try {@@ -394,6 +436,9 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff 			this._isSettingData = false; 		} 		this._dataStore.add(autorun(reader => {+			if (item.isBinary) {+				return;+			} 			const viewModel = item.diffEditorViewModel; 			if (!viewModel.isDiffUpToDate.read(reader)) { 				return;@@ -470,6 +515,15 @@ export class DiffEditorItemTemplate extends VirtualizedItemTemplate<DocumentDiff 		this._elements.root.style.visibility = 'hidden'; // Some editor parts are still visible 	} +	public focusBinaryFilePlaceholder(): void {+		const item = this._viewModel.get();+		if (item?.originalUri && item.modifiedUri && this._openBinaryDiffButton) {+			this._openBinaryDiffButton.focus();+		} else {+			this._elements.binaryFilePlaceholder.focus();+		}+	}+ 	public unbind(item: DocumentDiffItemViewModel): void { 		if (this._viewModel.get() !== item) { 			throw new BugIndicatingError('Cannot unbind a diff editor template from a different item');@@ -512,6 +566,14 @@ export class DiffEditorItemBinding extends VirtualizedItemBinding<DocumentDiffIt 		return this._template.getExpandedContentHeight(); 	} +	focus(): void {+		if (this.item.isBinary) {+			this._template.focusBinaryFilePlaceholder();+		} else {+			this.editor.focus();+		}+	}+ 	override dispose(): void { 		if (this._store.isDisposed) { 			return;

@@ -4,6 +4,7 @@  *--------------------------------------------------------------------------------------------*/  import { Event, IValueWithChangeEvent } from '../../../../base/common/event.js';+import { URI } from '../../../../base/common/uri.js'; import { RefCounted } from '../diffEditor/utils.js'; import { IDiffEditorOptions } from '../../../common/config/editorOptions.js'; import { ITextModel } from '../../../common/model.js';@@ -14,16 +15,26 @@ export interface IMultiDiffEditorModel { 	readonly contextKeys?: Record<string, ContextKeyValue>; } +/**+ * A resource participating on one side of a document diff.+ */+export class DiffItemSource {+	constructor(+		public readonly uri: URI,+		public readonly textModel: ITextModel | undefined,+	) { }+}+ export interface IDocumentDiffItem { 	/** 	 * undefined if the file was created. 	 */-	readonly original: ITextModel | undefined;+	readonly original: DiffItemSource | undefined;  	/** 	 * undefined if the file was deleted. 	 */-	readonly modified: ITextModel | undefined;+	readonly modified: DiffItemSource | undefined; 	readonly options?: IDiffEditorOptions; 	readonly onOptionsDidChange?: Event<void>; 	readonly contextKeys?: Record<string, ContextKeyValue>;

@@ -135,6 +135,11 @@ export class DocumentDiffItemViewModel extends Disposable {  	public get originalUri(): URI | undefined { return this.documentDiffItem.original?.uri; } 	public get modifiedUri(): URI | undefined { return this.documentDiffItem.modified?.uri; }+	public get isBinary(): boolean {+		const { original, modified } = this.documentDiffItem;+		return (original !== undefined && original.textModel === undefined)+			|| (modified !== undefined && modified.textModel === undefined);+	}  	public readonly isActive: IObservable<boolean> = derived(this, reader => this._editorViewModel.activeDiffItem.read(reader) === this); 	public readonly isFirst: IObservable<boolean> = derived(this, reader => this._editorViewModel.items.read(reader)[0] === this);@@ -188,8 +193,8 @@ export class DocumentDiffItemViewModel extends Disposable { 		}  		const diffEditorViewModelStore = new DisposableStore();-		const originalTextModel = this.documentDiffItem.original ?? diffEditorViewModelStore.add(this._modelService.createModel('', null));-		const modifiedTextModel = this.documentDiffItem.modified ?? diffEditorViewModelStore.add(this._modelService.createModel('', null));+		const originalTextModel = this.documentDiffItem.original?.textModel ?? diffEditorViewModelStore.add(this._modelService.createModel('', null));+		const modifiedTextModel = this.documentDiffItem.modified?.textModel ?? diffEditorViewModelStore.add(this._modelService.createModel('', null)); 		diffEditorViewModelStore.add(this._documentDiffItemRef.createNewRef(this));  		this.diffEditorViewModelRef = this._register(RefCounted.createWithDisposable(

@@ -127,6 +127,10 @@ export class MultiDiffEditorWidget extends Disposable {  	public readonly onDidChangeActiveControl = Event.fromObservableLight(this._activeControl); +	public focus(): boolean {+		return this._widgetImpl.get().focus();+	}+ 	public getViewState(): IMultiDiffEditorViewState { 		return this._widgetImpl.get().getViewState(); 	}

@@ -23,7 +23,7 @@ import { EditorContextKeys } from '../../../common/editorContextKeys.js'; import { ICodeEditor } from '../../editorBrowser.js'; import { CompressedVirtualizedScrollView, ICompressedVirtualizedScrollItem, ICompressedVirtualizedScrollItemContext } from './compressedVirtualizedScrollView.js'; import { ICompressedVirtualizedScrollLayout } from './compressedVirtualizedScrollLayout.js';-import { DiffEditorItemBinding, DiffEditorItemTemplate } from './diffEditorItemTemplate.js';+import { binaryFilePlaceholderContentHeight, DiffEditorItemBinding, DiffEditorItemTemplate } from './diffEditorItemTemplate.js'; import { IDocumentDiffItem } from './model.js'; import { formatDiffItemKey, formatUri, ILoggedDiffItem, MultiDiffEditorLogger } from './multiDiffEditorLogging.js'; import { DocumentDiffItemViewModel, MultiDiffEditorViewModel } from './multiDiffEditorViewModel.js';@@ -100,9 +100,18 @@ export class MultiDiffEditorWidgetImpl extends Disposable { 				const manager = this._register(new VirtualizedItemManager<DocumentDiffItemViewModel, DiffEditorItemBinding, DiffEditorItemTemplate>(sourceItems, context, { 					getId: item => item, 					getTemplateId: () => 'diffEditor',-					getUnboundSize: item => derived(item, reader => item.collapsed.read(reader)-						? this._workbenchUIElementFactory.diffEditorItemHeaderHeight ?? 40-						: item.lastTemplateData.read(reader).expandedContentHeight),+					getUnboundSize: item => derived(item, reader => {+						const headerHeight = this._workbenchUIElementFactory.diffEditorItemHeaderHeight ?? 40;+						if (item.collapsed.read(reader)) {+							return headerHeight;+						}+						if (item.isBinary) {+							return headerHeight+								+ (this._workbenchUIElementFactory.diffEditorItemContentBottomPadding ?? 0)+								+ binaryFilePlaceholderContentHeight;+						}+						return item.lastTemplateData.read(reader).expandedContentHeight;+					}), 					createTemplate: () => this._instantiationService.createInstance( 						DiffEditorItemTemplate, 						context.contentDomNode,@@ -516,30 +525,40 @@ export class MultiDiffEditorWidgetImpl extends Disposable { 		viewModel.activeDiffItem.setCache(target, undefined);  		if (!this._preserveFocusOnLoad) {-			this._viewItemsInfo.get().getItem(target).template.get()?.editor.focus();+			this._viewItemsInfo.get().getItem(target).binding.get()?.focus(); 		} 		return true; 	}  	public findDocumentDiffItem(resource: URI): IDocumentDiffItem | undefined { 		const item = this._viewItems.get().find(v =>-			v.viewModel.diffEditorViewModel.model.modified.uri.toString() === resource.toString()-			|| v.viewModel.diffEditorViewModel.model.original.uri.toString() === resource.toString()+			v.viewModel.modifiedUri?.toString() === resource.toString()+			|| v.viewModel.originalUri?.toString() === resource.toString() 		); 		return item?.viewModel.documentDiffItem; 	} +	public focus(): boolean {+		const activeDiffItem = this._viewModel.get()?.activeDiffItem.get();+		if (!activeDiffItem) {+			return false;+		}+		const binding = this._viewItemsInfo.get().getItem(activeDiffItem).binding.get();+		binding?.focus();+		return binding !== undefined;+	}+ 	public tryGetCodeEditor(resource: URI): { diffEditor: IDiffEditor; editor: ICodeEditor } | undefined { 		const item = this._viewItems.get().find(v =>-			v.viewModel.diffEditorViewModel.model.modified.uri.toString() === resource.toString()-			|| v.viewModel.diffEditorViewModel.model.original.uri.toString() === resource.toString()+			v.viewModel.modifiedUri?.toString() === resource.toString()+			|| v.viewModel.originalUri?.toString() === resource.toString() 		); 		const editor = item?.template.get()?.editor;-		if (!editor) {+		if (!editor || item.viewModel.isBinary) { 			return undefined; 		} -		if (item.viewModel.diffEditorViewModel.model.modified.uri.toString() === resource.toString()) {+		if (item.viewModel.modifiedUri?.toString() === resource.toString()) { 			return { diffEditor: editor, editor: editor.getModifiedEditor() }; 		} else { 			return { diffEditor: editor, editor: editor.getOriginalEditor() };@@ -617,7 +636,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { 			} 		} 		if (focusEditor) {-			editor?.focus();+			item.binding.get()?.focus(); 		} 	} @@ -786,7 +805,7 @@ class VirtualizedViewItem extends Disposable implements ILoggedDiffItem, ICompre 	}  	public override toString(): string {-		return `VirtualViewItem(${this.viewModel.documentDiffItem.modified?.uri.toString()})`;+		return `VirtualViewItem(${this.viewModel.modifiedUri?.toString() ?? this.viewModel.originalUri?.toString()})`; 	}  	public getKey(): string {

@@ -154,5 +154,24 @@ 		.editorContainer { 			flex: 1; 		}++		.binary-file-placeholder {+			display: none;+			flex: 1;+			place-items: center;+			color: var(--vscode-descriptionForeground);++			&:focus-visible {+				outline: var(--vscode-strokeThickness) solid var(--vscode-focusBorder);+				outline-offset: calc(-1 * var(--vscode-strokeThickness));+			}++			.binary-file-placeholder-content {+				display: flex;+				flex-direction: column;+				align-items: center;+				gap: var(--vscode-spacing-size80);+			}+		} 	} }

@@ -46,6 +46,9 @@ export interface IWorkbenchUIElementFactory { 	/** Handles a middle-click on an entry header. Returns whether the event was handled. */ 	handleHeaderMiddleClick?(resource: URI): boolean; +	/** Opens an entry in a standalone diff editor. */+	openDiffEditor?(original: URI, modified: URI): void;+ 	/** 	 * Optional override for how individual actions render in the per-file header 	 * toolbar (`MenuId.MultiDiffEditorFileToolbar`). Return `undefined` to fall

@@ -6,6 +6,7 @@ import assert from 'assert'; import sinon from 'sinon'; import { Dimension } from '../../../../base/browser/dom.js';+import { Button } from '../../../../base/browser/ui/button/button.js'; import { Event, ValueWithChangeEvent } from '../../../../base/common/event.js'; import { autorun, waitForState } from '../../../../base/common/observable.js'; import { URI } from '../../../../base/common/uri.js';@@ -21,7 +22,7 @@ import { InMemoryStorageService, IStorageService } from '../../../../platform/st import { IDiffProviderFactoryService } from '../../../browser/widget/diffEditor/diffProviderFactoryService.js'; import { DiffEditorWidget } from '../../../browser/widget/diffEditor/diffEditorWidget.js'; import { RefCounted } from '../../../browser/widget/diffEditor/utils.js';-import { IDocumentDiffItem, IMultiDiffEditorModel } from '../../../browser/widget/multiDiffEditor/model.js';+import { DiffItemSource, IDocumentDiffItem, IMultiDiffEditorModel } from '../../../browser/widget/multiDiffEditor/model.js'; import { MultiDiffEditorWidget } from '../../../browser/widget/multiDiffEditor/multiDiffEditorWidget.js'; import { IWorkbenchUIElementFactory } from '../../../browser/widget/multiDiffEditor/workbenchUIElementFactory.js'; import { EditorOption } from '../../../common/config/editorOptions.js';@@ -82,6 +83,97 @@ suite('MultiDiffEditorWidget', () => { 		} 	}); +	test('renders binary files as a placeholder', async () => {+		const services = new ServiceCollection();+		services.set(IAccessibilitySignalService, new class extends mock<IAccessibilitySignalService>() { }());+		services.set(IActionViewItemService, new NullActionViewItemService());+		services.set(IEditorProgressService, new class extends mock<IEditorProgressService>() { }());+		services.set(IDiffProviderFactoryService, new TestDiffProviderFactoryService());+		services.set(IStorageService, disposables.add(new InMemoryStorageService()));+		services.set(IMenuService, new class extends mock<IMenuService>() {+			override createMenu(): IMenu {+				return new class extends mock<IMenu>() {+					override readonly onDidChange = Event.None;+					override getActions() { return []; }+					override dispose(): void { }+				}();+			}+		}());+		const instantiationService = createCodeEditorServices(disposables, services);+		const originalUri = URI.parse('inmemory://original/image.png');+		const modifiedUri = URI.parse('inmemory://modified/image.png');+		const documentItem = RefCounted.createOfNonDisposable<IDocumentDiffItem>({+			original: new DiffItemSource(originalUri, undefined),+			modified: new DiffItemSource(modifiedUri, undefined),+		}, { dispose() { } });+		const model: IMultiDiffEditorModel = {+			documents: ValueWithChangeEvent.const([documentItem]),+		};+		let openedDiff: { original: URI; modified: URI } | undefined;+		const container = document.createElement('div');+		const widget = instantiationService.createInstance(+			MultiDiffEditorWidget,+			container,+			{+				openDiffEditor: (original, modified) => openedDiff = { original, modified },+			} satisfies IWorkbenchUIElementFactory,+			undefined,+		);+		widget.layout(new Dimension(800, 600));+		const viewModel = widget.createViewModel(model);+		await waitForState(viewModel.items, items => items.length === 1);+		widget.setViewModel(viewModel);+		widget.reveal({ original: originalUri, modified: modifiedUri }, { highlight: false });+		await waitForState(widget.getLayoutDebugState(), state => state.items[0]?.hasTemplate === true);++		try {+			const placeholder = widget.getRootElement().querySelector<HTMLElement>('.binary-file-placeholder');+			const editor = widget.getRootElement().querySelector<HTMLElement>('.editorContainer');+			const openDiffButton = placeholder?.querySelector<HTMLElement>('.monaco-button');+			const focusSpy = sinon.spy(Button.prototype, 'focus');+			const canFocusActiveItem = widget.focus();+			openDiffButton?.click();+			assert.deepStrictEqual({+				text: placeholder?.textContent,+				display: placeholder?.style.display,+				tabIndex: placeholder?.tabIndex,+				role: placeholder?.getAttribute('role'),+				ariaLabel: placeholder?.getAttribute('aria-label'),+				openDiffButtonText: openDiffButton?.textContent,+				openDiffButtonSecondary: openDiffButton?.classList.contains('secondary'),+				openDiffButtonFocused: focusSpy.calledOnce,+				openedOriginalUri: openedDiff?.original.toString(),+				openedModifiedUri: openedDiff?.modified.toString(),+				editorDisplay: editor?.style.display,+				itemHeight: widget.getLayoutDebugState().get().items[0].verticalState.contentHeight,+				canFocusActiveItem,+				findsDocumentItem: widget.findDocumentDiffItem(modifiedUri) === documentItem.object,+				hasCodeEditorForBinaryResource: widget.tryGetCodeEditor(modifiedUri) !== undefined,+			}, {+				text: 'Binary file changedOpen Diff',+				display: 'grid',+				tabIndex: -1,+				role: 'group',+				ariaLabel: 'Binary file changed',+				openDiffButtonText: 'Open Diff',+				openDiffButtonSecondary: true,+				openDiffButtonFocused: true,+				openedOriginalUri: originalUri.toString(),+				openedModifiedUri: modifiedUri.toString(),+				editorDisplay: 'none',+				itemHeight: 140,+				canFocusActiveItem: true,+				findsDocumentItem: true,+				hasCodeEditorForBinaryResource: false,+			});+		} finally {+			widget.setViewModel(undefined);+			viewModel.dispose();+			widget.dispose();+			documentItem.dispose();+		}+	});+ 	test('applies document and responsive layout options before attaching the diff model', async () => { 		const services = new ServiceCollection(); 		services.set(IAccessibilitySignalService, new class extends mock<IAccessibilitySignalService>() { }());@@ -105,8 +197,8 @@ suite('MultiDiffEditorWidget', () => { 		const original = disposables.add(instantiateTextModel(instantiationService, 'const value = 1;', undefined, undefined, originalUri)); 		const modified = disposables.add(instantiateTextModel(instantiationService, 'const value = 2;', undefined, undefined, modifiedUri)); 		const documentItem = RefCounted.createOfNonDisposable<IDocumentDiffItem>({-			original,-			modified,+			original: new DiffItemSource(originalUri, original),+			modified: new DiffItemSource(modifiedUri, modified), 			options: { accessibilitySupport: 'off' }, 		}, { dispose() { } }); 		const model: IMultiDiffEditorModel = {@@ -181,7 +273,7 @@ suite('MultiDiffEditorWidget', () => { 		const originalContent = Array.from({ length: 64 }, (_, index) => `line ${index}`).join('\n'); 		const original = disposables.add(instantiateTextModel(instantiationService, originalContent, undefined, undefined, originalUri)); 		const documentItem = RefCounted.createOfNonDisposable<IDocumentDiffItem>({-			original,+			original: new DiffItemSource(originalUri, original), 			modified: undefined, 			options: { accessibilitySupport: 'off' }, 		}, { dispose() { } });@@ -259,8 +351,8 @@ suite('MultiDiffEditorWidget', () => { 			const original = disposables.add(instantiateTextModel(instantiationService, '', undefined, undefined, originalUri)); 			const modified = disposables.add(instantiateTextModel(instantiationService, 'const value = 1;', undefined, undefined, modifiedUri)); 			documentItems.push(RefCounted.createOfNonDisposable<IDocumentDiffItem>({-				original,-				modified,+				original: new DiffItemSource(originalUri, original),+				modified: new DiffItemSource(modifiedUri, modified), 				options: { accessibilitySupport: 'off' }, 			}, { dispose() { } })); 			originalUris.push(originalUri);

@@ -83,6 +83,7 @@ class SessionChangesUIElementFactory implements IWorkbenchUIElementFactory { 		@ICommandService private readonly commandService: ICommandService, 		@IChangesViewService private readonly changesViewService: IChangesViewService, 		@IInstantiationService private readonly instantiationService: IInstantiationService,+		@IEditorService private readonly editorService: IEditorService, 	) { }  	createResourceLabel(element: HTMLElement, kind: MultiDiffEditorItemLabelKind): IResourceLabel {@@ -111,6 +112,14 @@ class SessionChangesUIElementFactory implements IWorkbenchUIElementFactory { 		} 		return undefined; 	}++	openDiffEditor(original: URI, modified: URI): void {+		void this.editorService.openEditor({+			original: { resource: original },+			modified: { resource: modified },+			options: { pinned: true },+		});+	} }  class SessionChangesResourceLabel extends Disposable implements IResourceLabel {@@ -436,20 +445,16 @@ export class SessionChangesEditor extends AbstractEditorWithViewState<IMultiDiff 			return; 		} -		const control = widget.getActiveControl();-		if (control) {-			control.focus();+		if (widget.focus()) { 			return; 		}  		// The active file's diff editor may not be rendered yet (e.g. the editor 		// part was just revealed from a hidden state), so getActiveControl() is 		// undefined. Focus it as soon as it becomes available. 		this._pendingFocus.value = widget.onDidChangeActiveControl(() => {-			const activeControl = widget.getActiveControl();-			if (activeControl) {+			if (widget.focus()) { 				this._pendingFocus.clear();-				activeControl.focus(); 			} 		}); 	}

@@ -17,7 +17,7 @@ import { mock } from '../../../../../base/test/common/mock.js'; import { MultiDiffEditorWidget } from '../../../../../editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.js'; import { IDiffProviderFactoryService } from '../../../../../editor/browser/widget/diffEditor/diffProviderFactoryService.js'; import { RefCounted } from '../../../../../editor/browser/widget/diffEditor/utils.js';-import { IDocumentDiffItem } from '../../../../../editor/browser/widget/multiDiffEditor/model.js';+import { DiffItemSource, IDocumentDiffItem } from '../../../../../editor/browser/widget/multiDiffEditor/model.js'; import { IResourceLabel, IWorkbenchUIElementFactory } from '../../../../../editor/browser/widget/multiDiffEditor/workbenchUIElementFactory.js'; import { TestDiffProviderFactoryService } from '../../../../../editor/test/browser/diff/testDiffProviderFactoryService.js'; import { IMenu, IMenuActionOptions, IMenuService, MenuId, MenuItemAction } from '../../../../../platform/actions/common/actions.js';@@ -260,8 +260,14 @@ async function renderAgentsDiffEditor({ container, disposableStore, disposableSt 	const secondOriginal = textModels.add(createTextModel(instantiationService, 'export function count() {\n\treturn 1;\n}', URI.file('/workspace/src/second.original.ts'), 'typescript')); 	const secondModified = textModels.add(createTextModel(instantiationService, 'export function count() {\n\treturn 2;\n}', URI.file('/workspace/src/second.ts'), 'typescript')); -	const first = RefCounted.createOfNonDisposable<IDocumentDiffItem>({ original: firstOriginal, modified: firstModified }, { dispose() { } });-	const second = RefCounted.createOfNonDisposable<IDocumentDiffItem>({ original: secondOriginal, modified: secondModified }, { dispose() { } });+	const first = RefCounted.createOfNonDisposable<IDocumentDiffItem>({+		original: new DiffItemSource(firstOriginal.uri, firstOriginal),+		modified: new DiffItemSource(firstModified.uri, firstModified),+	}, { dispose() { } });+	const second = RefCounted.createOfNonDisposable<IDocumentDiffItem>({+		original: new DiffItemSource(secondOriginal.uri, secondOriginal),+		modified: new DiffItemSource(secondModified.uri, secondModified),+	}, { dispose() { } }); 	const widget = disposableStackStore.add(instantiationService.createInstance( 		MultiDiffEditorWidget, 		editorInstance,

@@ -150,7 +150,7 @@ export class MultiDiffEditor extends AbstractEditorWithViewState<IMultiDiffEdito 	override focus(): void { 		super.focus(); -		this._multiDiffEditorWidget?.getActiveControl()?.focus();+		this._multiDiffEditorWidget?.focus(); 	}  	override hasFocus(): boolean {@@ -248,6 +248,7 @@ class WorkbenchUIElementFactory implements IWorkbenchUIElementFactory { 	constructor( 		@IInstantiationService private readonly _instantiationService: IInstantiationService, 		@IContextKeyService contextKeyService: IContextKeyService,+		@IEditorService private readonly editorService: IEditorService, 	) { 		this.headerClickToCollapse = IsSessionsWindowContext.getValue(contextKeyService) === true; 	}@@ -267,4 +268,12 @@ class WorkbenchUIElementFactory implements IWorkbenchUIElementFactory { 			} 		}; 	}++	openDiffEditor(original: URI, modified: URI): void {+		void this.editorService.openEditor({+			original: { resource: original },+			modified: { resource: modified },+			options: { pinned: true },+		});+	} }

@@ -16,7 +16,7 @@ import { ThemeIcon } from '../../../../base/common/themables.js'; import { isDefined, isObject } from '../../../../base/common/types.js'; import { URI } from '../../../../base/common/uri.js'; import { RefCounted } from '../../../../editor/browser/widget/diffEditor/utils.js';-import { IDocumentDiffItem, IMultiDiffEditorModel } from '../../../../editor/browser/widget/multiDiffEditor/model.js';+import { DiffItemSource, IDocumentDiffItem, IMultiDiffEditorModel } from '../../../../editor/browser/widget/multiDiffEditor/model.js'; import { MultiDiffEditorViewModel } from '../../../../editor/browser/widget/multiDiffEditor/multiDiffEditorViewModel.js'; import { IDiffEditorOptions } from '../../../../editor/common/config/editorOptions.js'; import { IResolvedTextEditorModel, ITextModelService } from '../../../../editor/common/services/resolverService.js';@@ -28,10 +28,15 @@ import { IEditorConfiguration } from '../../../browser/parts/editor/textEditor.j import { DEFAULT_EDITOR_ASSOCIATION, EditorInputCapabilities, EditorInputWithOptions, GroupIdentifier, IEditorSerializer, IResourceMultiDiffEditorInput, IRevertOptions, ISaveOptions, IUntypedEditorInput } from '../../../common/editor.js'; import { EditorInput, IEditorCloseHandler } from '../../../common/editor/editorInput.js'; import { IEditorResolverService, RegisteredEditorPriority } from '../../../services/editor/common/editorResolverService.js';-import { ILanguageSupport, ITextFileEditorModel, ITextFileService } from '../../../services/textfile/common/textfiles.js';+import { ILanguageSupport, ITextFileEditorModel, ITextFileService, TextFileOperationError, TextFileOperationResult } from '../../../services/textfile/common/textfiles.js'; import { MultiDiffEditorIcon } from './icons.contribution.js'; import { IMultiDiffSourceResolverService, IResolvedMultiDiffSource, MultiDiffEditorItem } from './multiDiffSourceResolverService.js'; +function isBinaryTextFileOperationError(error: unknown): error is TextFileOperationError {+	return TextFileOperationError.isTextFileOperationError(error)+		&& error.textFileOperationResult === TextFileOperationResult.FILE_IS_BINARY;+}+ export class MultiDiffEditorInput extends EditorInput implements ILanguageSupport { 	public static fromResourceMultiDiffEditorInput(input: IResourceMultiDiffEditorInput, instantiationService: IInstantiationService): MultiDiffEditorInput { 		if (!input.multiDiffSource && !input.resources) {@@ -184,7 +189,7 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor 		const activeDiffItem = this._viewModel.requireValue().activeDiffItem.get(); 		const value = activeDiffItem?.documentDiffItem; 		if (!value) { return; }-		const target = value.modified ?? value.original;+		const target = (value.modified ?? value.original)?.textModel; 		if (!target) { return; } 		target.setLanguage(languageId, source); 	}@@ -226,25 +231,27 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor 				return undefined; 			} -			let errorResult: PromiseRejectedResult | undefined;-			if (originalResult.status === 'rejected') {-				errorResult = originalResult;-			} else if (modifiedResult.status === 'rejected') {-				errorResult = modifiedResult;-			}+			const errorResults = [originalResult, modifiedResult].filter((result): result is PromiseRejectedResult => result.status === 'rejected');+			const errorResult = errorResults.find(result => !isBinaryTextFileOperationError(result.reason)); 			if (errorResult) { 				multiDiffItemStore.dispose();-				// e.g. "File seems to be binary and cannot be opened as text" 				console.error(errorResult.reason); 				onUnexpectedError(errorResult.reason); 				return undefined; 			} +			const isBinary = errorResults.length > 0;+			if (isBinary) {+				multiDiffItemStore.clear();+				original = undefined;+				modified = undefined;+			}+ 			const uri = (r.modifiedUri ?? r.originalUri)!; 			const result: IDocumentDiffItemWithMultiDiffEditorItem = { 				multiDiffEditorItem: r,-				original: original?.object.textEditorModel,-				modified: modified?.object.textEditorModel,+				original: r.originalUri ? new DiffItemSource(r.originalUri, original?.object.textEditorModel) : undefined,+				modified: r.modifiedUri ? new DiffItemSource(r.modifiedUri, modified?.object.textEditorModel) : undefined, 				contextKeys: r.contextKeys, 				get options() { 					return {@@ -321,6 +328,9 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor 		const items = this._viewModel.currentValue?.items.get(); 		if (items) { 			await Promise.all(items.map(async item => {+				if (item.isBinary) {+					return;+				} 				const model = item.diffEditorViewModel.model; 				const handleOriginal = model.original.uri.scheme !== Schemas.untitled && this._textFileService.isDirty(model.original.uri); // match diff editor behaviour 

@@ -12,10 +12,14 @@ import { observableValue, ValueWithChangeEventFromObservable } from '../../../.. import { URI } from '../../../../../base/common/uri.js'; import { mock } from '../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';+import { IDiffProviderFactoryService } from '../../../../../editor/browser/widget/diffEditor/diffProviderFactoryService.js'; import { IResolvedTextEditorModel, ITextModelService } from '../../../../../editor/common/services/resolverService.js'; import { ITextResourceConfigurationService } from '../../../../../editor/common/services/textResourceConfiguration.js';+import { TestDiffProviderFactoryService } from '../../../../../editor/test/browser/diff/testDiffProviderFactoryService.js';+import { createCodeEditorServices } from '../../../../../editor/test/browser/testCodeEditor.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';-import { ITextFileEditorModelManager, ITextFileService } from '../../../../services/textfile/common/textfiles.js';+import { ServiceCollection } from '../../../../../platform/instantiation/common/serviceCollection.js';+import { ITextFileEditorModelManager, ITextFileService, TextFileOperationError, TextFileOperationResult } from '../../../../services/textfile/common/textfiles.js'; import { MultiDiffEditorInput } from '../../browser/multiDiffEditorInput.js'; import { IMultiDiffSourceResolverService, MultiDiffEditorItem } from '../../browser/multiDiffSourceResolverService.js'; @@ -99,4 +103,68 @@ suite('MultiDiffEditorInput', () => { 		await assert.rejects(viewModelPromise, CancellationError); 		assert.strictEqual(referenceDisposed, true); 	});++	test('keeps binary resources in the multi diff model', async () => {+		const originalUri = URI.parse('file:///original.png');+		const modifiedUri = URI.parse('file:///modified.png');+		const textModelService = new class extends mock<ITextModelService>() {+			override createModelReference(): Promise<IReference<IResolvedTextEditorModel>> {+				return Promise.reject(new TextFileOperationError('binary', TextFileOperationResult.FILE_IS_BINARY));+			}+		}();+		const textResourceConfigurationService = new class extends mock<ITextResourceConfigurationService>() {+			override readonly onDidChangeConfiguration = Event.None;+			override getValue<T>(): T { return {} as T; }+		}();+		let saveCallCount = 0;+		const textFileService = new class extends mock<ITextFileService>() {+			override readonly files = new class extends mock<ITextFileEditorModelManager>() {+				override readonly onDidChangeDirty = Event.None;+			}();+			override save(): Promise<undefined> {+				saveCallCount++;+				return Promise.resolve(undefined);+			}+		}();+		const services = new ServiceCollection();+		services.set(IDiffProviderFactoryService, new TestDiffProviderFactoryService());+		const instantiationService = createCodeEditorServices(disposables, services);+		const input = disposables.add(new MultiDiffEditorInput(+			URI.parse('multi-diff-editor:test'),+			'Test',+			[new MultiDiffEditorItem(originalUri, modifiedUri, undefined)],+			false,+			textModelService,+			textResourceConfigurationService,+			instantiationService,+			new class extends mock<IMultiDiffSourceResolverService>() { }(),+			textFileService,+		));++		const viewModel = await input.getViewModel();+		const item = viewModel.items.get()[0];+		await input.save(1);++		assert.deepStrictEqual({+			itemCount: viewModel.items.get().length,+			originalUri: item.originalUri?.toString(),+			modifiedUri: item.modifiedUri?.toString(),+			isBinary: item.isBinary,+			originalSourceUri: item.documentDiffItem.original?.uri.toString(),+			modifiedSourceUri: item.documentDiffItem.modified?.uri.toString(),+			hasOriginalTextModel: item.documentDiffItem.original?.textModel !== undefined,+			hasModifiedTextModel: item.documentDiffItem.modified?.textModel !== undefined,+			saveCallCount,+		}, {+			itemCount: 1,+			originalUri: originalUri.toString(),+			modifiedUri: modifiedUri.toString(),+			isBinary: true,+			originalSourceUri: originalUri.toString(),+			modifiedSourceUri: modifiedUri.toString(),+			hasOriginalTextModel: false,+			hasModifiedTextModel: false,+			saveCallCount: 0,+		});+	}); });

@@ -9,7 +9,7 @@ import { DisposableStore, toDisposable } from '../../../../../base/common/lifecy import { URI } from '../../../../../base/common/uri.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { createTimeout, timeout } from '../../../../../base/common/async.js';-import { IDocumentDiffItem, IMultiDiffEditorModel } from '../../../../../editor/browser/widget/multiDiffEditor/model.js';+import { DiffItemSource, IDocumentDiffItem, IMultiDiffEditorModel } from '../../../../../editor/browser/widget/multiDiffEditor/model.js'; import { RefCounted } from '../../../../../editor/browser/widget/diffEditor/utils.js'; import { IDiffProviderFactoryService } from '../../../../../editor/browser/widget/diffEditor/diffProviderFactoryService.js'; import { TestDiffProviderFactoryService } from '../../../../../editor/test/browser/diff/testDiffProviderFactoryService.js';@@ -63,7 +63,10 @@ function renderMultiDiffEditorHideOriginalLineNumbers({ container, disposableSto 	const textModels = disposableStackStore.add(new DisposableStore()); 	const original = textModels.add(createTextModel(instantiationService, ORIGINAL_HIDDEN, URI.parse('inmemory://original/settings.ts'), 'typescript')); 	const modified = textModels.add(createTextModel(instantiationService, MODIFIED_HIDDEN, URI.parse('inmemory://modified/settings.ts'), 'typescript'));-	const doc = RefCounted.createOfNonDisposable<IDocumentDiffItem>({ original, modified }, { dispose() { } });+	const doc = RefCounted.createOfNonDisposable<IDocumentDiffItem>({+		original: new DiffItemSource(original.uri, original),+		modified: new DiffItemSource(modified.uri, modified),+	}, { dispose() { } });  	const widget = disposableStackStore.add(createMultiDiffEditorFixtureWidget(instantiationService, container, { 		hideOriginalLineNumbers: true,@@ -162,7 +165,10 @@ function renderMultiDiffEditorDocumentSwap() { 		const makeDoc = (origText: string, modText: string, name: string) => { 			const original = textModels.add(createTextModel(instantiationService, origText, URI.parse(`inmemory://original/${name}`), 'typescript')); 			const modified = textModels.add(createTextModel(instantiationService, modText, URI.parse(`inmemory://modified/${name}`), 'typescript'));-			return RefCounted.createOfNonDisposable<IDocumentDiffItem>({ original, modified }, { dispose() { } });+			return RefCounted.createOfNonDisposable<IDocumentDiffItem>({+				original: new DiffItemSource(original.uri, original),+				modified: new DiffItemSource(modified.uri, modified),+			}, { dispose() { } }); 		};  		// Each document has exactly one line change.

@@ -10,7 +10,7 @@ import { mock } from '../../../../../base/test/common/mock.js'; import { RefCounted } from '../../../../../editor/browser/widget/diffEditor/utils.js'; import { IDiffProviderFactoryService } from '../../../../../editor/browser/widget/diffEditor/diffProviderFactoryService.js'; import { MultiDiffEditorWidget } from '../../../../../editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.js';-import { IDocumentDiffItem } from '../../../../../editor/browser/widget/multiDiffEditor/model.js';+import { DiffItemSource, IDocumentDiffItem } from '../../../../../editor/browser/widget/multiDiffEditor/model.js'; import { IResourceLabel as IMultiDiffResourceLabel, IWorkbenchUIElementFactory } from '../../../../../editor/browser/widget/multiDiffEditor/workbenchUIElementFactory.js'; import { IDiffEditorOptions } from '../../../../../editor/common/config/editorOptions.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';@@ -140,8 +140,17 @@ export function createMultiDiffEditorFixtureDocuments(instantiationService: Test 	const original3 = textModels.add(createTextModel(instantiationService, originalCode3, URI.parse('inmemory://original/server.ts'), 'typescript')); 	const modified3 = textModels.add(createTextModel(instantiationService, modifiedCode3, URI.parse('inmemory://modified/server.ts'), 'typescript')); 	return {-		doc1: RefCounted.createOfNonDisposable<IDocumentDiffItem>({ original: original1, modified: modified1 }, { dispose() { } }),-		doc2: RefCounted.createOfNonDisposable<IDocumentDiffItem>({ original: original2, modified: modified2 }, { dispose() { } }),-		doc3: RefCounted.createOfNonDisposable<IDocumentDiffItem>({ original: original3, modified: modified3 }, { dispose() { } }),+		doc1: RefCounted.createOfNonDisposable<IDocumentDiffItem>({+			original: new DiffItemSource(original1.uri, original1),+			modified: new DiffItemSource(modified1.uri, modified1),+		}, { dispose() { } }),+		doc2: RefCounted.createOfNonDisposable<IDocumentDiffItem>({+			original: new DiffItemSource(original2.uri, original2),+			modified: new DiffItemSource(modified2.uri, modified2),+		}, { dispose() { } }),+		doc3: RefCounted.createOfNonDisposable<IDocumentDiffItem>({+			original: new DiffItemSource(original3.uri, original3),+			modified: new DiffItemSource(modified3.uri, modified3),+		}, { dispose() { } }), 	}; }