improve diff for /edits (#237801)

* improve diff for /edits

* no double-diff for chat modified entries
* align whitespace property
* overflow logic for overlay widget

* fix compiler

Author: jrieken

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts

@@ -7,7 +7,7 @@ import { RunOnceScheduler } from '../../../../../base/common/async.js';
 import { Emitter } from '../../../../../base/common/event.js';
 import { Disposable, IReference, toDisposable } from '../../../../../base/common/lifecycle.js';
 import { Schemas } from '../../../../../base/common/network.js';
-import { IObservable, ITransaction, observableValue, transaction } from '../../../../../base/common/observable.js';
+import { autorun, IObservable, ITransaction, observableValue, transaction } from '../../../../../base/common/observable.js';
 import { themeColorFromId } from '../../../../../base/common/themables.js';
 import { URI } from '../../../../../base/common/uri.js';
 import { EditOperation, ISingleEditOperation } from '../../../../../editor/common/core/editOperation.js';
@@ -26,7 +26,9 @@ import { IModelService } from '../../../../../editor/common/services/model.js';
 import { IResolvedTextEditorModel, ITextModelService } from '../../../../../editor/common/services/resolverService.js';
 import { IModelContentChangedEvent } from '../../../../../editor/common/textModelEvents.js';
 import { localize } from '../../../../../nls.js';
+import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
 import { IFileService } from '../../../../../platform/files/common/files.js';
+import { observableConfigValue } from '../../../../../platform/observable/common/platformObservableUtils.js';
 import { editorSelectionBackground } from '../../../../../platform/theme/common/colorRegistry.js';
 import { IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
 import { SaveReason } from '../../../../common/editor.js';
@@ -130,6 +132,8 @@ export class ChatEditingModifiedFileEntry extends Disposable implements IModifie
 		return this._telemetryInfo.requestId;
 	}
 
+	private readonly _diffTrimWhitespace: IObservable<boolean>;
+
 	constructor(
 		resourceRef: IReference<IResolvedTextEditorModel>,
 		private readonly _multiDiffEntryDelegate: { collapse: (transaction: ITransaction | undefined) => void },
@@ -139,6 +143,7 @@ export class ChatEditingModifiedFileEntry extends Disposable implements IModifie
 		@IModelService modelService: IModelService,
 		@ITextModelService textModelService: ITextModelService,
 		@ILanguageService languageService: ILanguageService,
+		@IConfigurationService configService: IConfigurationService,
 		@IChatService private readonly _chatService: IChatService,
 		@IEditorWorkerService private readonly _editorWorkerService: IEditorWorkerService,
 		@IUndoRedoService private readonly _undoRedoService: IUndoRedoService,
@@ -186,6 +191,12 @@ export class ChatEditingModifiedFileEntry extends Disposable implements IModifie
 		this._register(toDisposable(() => {
 			this._clearCurrentEditLineDecoration();
 		}));
+
+		this._diffTrimWhitespace = observableConfigValue('diffEditor.ignoreTrimWhitespace', true, configService);
+		this._register(autorun(r => {
+			this._diffTrimWhitespace.read(r);
+			this._updateDiffInfoSeq();
+		}));
 	}
 
 	private _clearCurrentEditLineDecoration() {
@@ -413,10 +424,12 @@ export class ChatEditingModifiedFileEntry extends Disposable implements IModifie
 		const docVersionNow = this.doc.getVersionId();
 		const snapshotVersionNow = this.docSnapshot.getVersionId();
 
+		const ignoreTrimWhitespace = this._diffTrimWhitespace.get();
+
 		const diff = await this._editorWorkerService.computeDiff(
 			this.docSnapshot.uri,
 			this.doc.uri,
-			{ computeMoves: true, ignoreTrimWhitespace: false, maxComputationTimeMs: 3000 },
+			{ ignoreTrimWhitespace, computeMoves: false, maxComputationTimeMs: 3000 },
 			'advanced'
 		);
 

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedNotebookEntry.ts

@@ -9,6 +9,7 @@ import { ILanguageService } from '../../../../../editor/common/languages/languag
 import { IEditorWorkerService } from '../../../../../editor/common/services/editorWorker.js';
 import { IModelService } from '../../../../../editor/common/services/model.js';
 import { IResolvedTextEditorModel, ITextModelService } from '../../../../../editor/common/services/resolverService.js';
+import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
 import { IFileService } from '../../../../../platform/files/common/files.js';
 import { IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
 import { SaveReason } from '../../../../common/editor.js';
@@ -33,8 +34,9 @@ export class ChatEditingModifiedNotebookEntry extends ChatEditingModifiedFileEnt
 		@IEditorWorkerService _editorWorkerService: IEditorWorkerService,
 		@IUndoRedoService _undoRedoService: IUndoRedoService,
 		@IFileService _fileService: IFileService,
+		@IConfigurationService configService: IConfigurationService
 	) {
-		super(resourceRef, _multiDiffEntryDelegate, _telemetryInfo, kind, initialContent, modelService, textModelService, languageService, _chatService, _editorWorkerService, _undoRedoService, _fileService);
+		super(resourceRef, _multiDiffEntryDelegate, _telemetryInfo, kind, initialContent, modelService, textModelService, languageService, configService, _chatService, _editorWorkerService, _undoRedoService, _fileService);
 		this.resolveTextFileEditorModel = resourceRef.object as IResolvedTextFileEditorModel;
 	}
 

src/vs/workbench/contrib/chat/browser/chatEditorController.ts

@@ -6,7 +6,7 @@
 import './media/chatEditorController.css';
 import { addStandardDisposableListener, getTotalWidth } from '../../../../base/browser/dom.js';
 import { Disposable, DisposableStore, dispose, toDisposable } from '../../../../base/common/lifecycle.js';
-import { autorun, autorunWithStore, derived, IObservable, observableFromEvent, observableValue, transaction } from '../../../../base/common/observable.js';
+import { autorun, autorunWithStore, derived, IObservable, observableFromEvent, observableValue } from '../../../../base/common/observable.js';
 import { themeColorFromId } from '../../../../base/common/themables.js';
 import { ICodeEditor, IOverlayWidget, IOverlayWidgetPosition, IOverlayWidgetPositionCoordinates, IViewZone, MouseTargetType } from '../../../../editor/browser/editorBrowser.js';
 import { LineSource, renderLines, RenderOptions } from '../../../../editor/browser/widget/diffEditor/components/diffEditorViewZones/renderLines.js';
@@ -80,9 +80,10 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 		this._ctxHasEditorModification = ctxHasEditorModification.bindTo(contextKeyService);
 		this._ctxRequestInProgress = ctxHasRequestInProgress.bindTo(contextKeyService);
 
-		const fontInfoObs = observableCodeEditor(this._editor).getOption(EditorOption.fontInfo);
-		const lineHeightObs = observableCodeEditor(this._editor).getOption(EditorOption.lineHeight);
-		const modelObs = observableCodeEditor(this._editor).model;
+		const editorObs = observableCodeEditor(this._editor);
+		const fontInfoObs = editorObs.getOption(EditorOption.fontInfo);
+		const lineHeightObs = editorObs.getOption(EditorOption.lineHeight);
+		const modelObs = editorObs.model;
 
 
 		this._store.add(autorun(r => {
@@ -112,13 +113,13 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 			const currentEditorEntry = entryForEditor.read(r);
 
 			if (!currentEditorEntry) {
-				this._clearRendering();
+				this._clear();
 				didReval = false;
 				return;
 			}
 
 			if (this._editor.getOption(EditorOption.inDiffEditor) && !_instantiationService.invokeFunction(isDiffEditorForEntry, currentEditorEntry.entry, this._editor)) {
-				this._clearRendering();
+				this._clear();
 				return;
 			}
 
@@ -145,7 +146,16 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 				lineHeightObs.read(r);
 
 				const diff = entry?.diffInfo.read(r);
-				this._updateWithDiff(entry, diff);
+
+				// Add line decorations (just markers, no UI) for diff navigation
+				this._updateDiffLineDecorations(diff);
+
+				// Add diff decoration to the UI (unless in diff editor)
+				if (!this._editor.getOption(EditorOption.inDiffEditor)) {
+					this._updateDiffRendering(entry, diff);
+				} else {
+					this._clearDiffRendering();
+				}
 
 				if (!didReval && !diff.identical) {
 					didReval = true;
@@ -199,11 +209,19 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 	}
 
 	override dispose(): void {
-		this._clearRendering();
+		this._clear();
 		super.dispose();
 	}
 
-	private _clearRendering() {
+	private _clear() {
+		this._clearDiffRendering();
+		this._diffLineDecorations.clear();
+		this._currentChangeIndex.set(undefined, undefined);
+		this._currentEntryIndex.set(undefined, undefined);
+		this._ctxHasEditorModification.reset();
+	}
+
+	private _clearDiffRendering() {
 		this._editor.changeViewZones((viewZoneChangeAccessor) => {
 			for (const id of this._viewZones) {
 				viewZoneChangeAccessor.removeZone(id);
@@ -212,18 +230,11 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 		this._viewZones = [];
 		this._diffHunksRenderStore.clear();
 		this._diffVisualDecorations.clear();
-		this._diffLineDecorations.clear();
-		this._ctxHasEditorModification.reset();
-		transaction(tx => {
-			this._currentEntryIndex.set(undefined, tx);
-			this._currentChangeIndex.set(undefined, tx);
-		});
 		this._scrollLock = false;
 	}
 
-	private _updateWithDiff(entry: IModifiedFileEntry, diff: IDocumentDiff): void {
+	private _updateDiffRendering(entry: IModifiedFileEntry, diff: IDocumentDiff): void {
 
-		this._ctxHasEditorModification.set(!diff.identical);
 		const originalModel = entry.originalModel;
 
 		const chatDiffAddDecoration = ModelDecorationOptions.createDynamic({
@@ -255,7 +266,6 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 			}
 			this._viewZones = [];
 			const modifiedVisualDecorations: IModelDeltaDecoration[] = [];
-			const modifiedLineDecorations: IModelDeltaDecoration[] = [];
 			const mightContainNonBasicASCII = originalModel.mightContainNonBasicASCII();
 			const mightContainRTL = originalModel.mightContainRTL();
 			const renderOptions = RenderOptions.fromEditor(this._editor);
@@ -344,16 +354,9 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 						stickiness: TrackedRangeStickiness.AlwaysGrowsWhenTypingAtEdges
 					}
 				});
-
-				// Add line decorations for diff navigation
-				modifiedLineDecorations.push({
-					range: diffEntry.modified.toInclusiveRange() ?? new Range(diffEntry.modified.startLineNumber, 1, diffEntry.modified.startLineNumber, Number.MAX_SAFE_INTEGER),
-					options: ChatEditorController._diffLineDecorationData
-				});
 			}
 
 			this._diffVisualDecorations.set(modifiedVisualDecorations);
-			this._diffLineDecorations.set(modifiedLineDecorations);
 		});
 
 		const diffHunkDecoCollection = this._editor.createDecorationsCollection(diffHunkDecorations);
@@ -428,6 +431,20 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 		}));
 	}
 
+	private _updateDiffLineDecorations(diff: IDocumentDiff): void {
+		this._ctxHasEditorModification.set(!diff.identical);
+
+		const modifiedLineDecorations: IModelDeltaDecoration[] = [];
+
+		for (const diffEntry of diff.changes) {
+			modifiedLineDecorations.push({
+				range: diffEntry.modified.toInclusiveRange() ?? new Range(diffEntry.modified.startLineNumber, 1, diffEntry.modified.startLineNumber, Number.MAX_SAFE_INTEGER),
+				options: ChatEditorController._diffLineDecorationData
+			});
+		}
+		this._diffLineDecorations.set(modifiedLineDecorations);
+	}
+
 	unlockScroll(): void {
 		this._scrollLock = false;
 	}
@@ -533,6 +550,12 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 		if (!this._editor.hasModel()) {
 			return;
 		}
+
+		const entry = this._chatEditingService.currentEditingSessionObs.get()?.getEntry(this._editor.getModel().uri);
+		if (!entry) {
+			return;
+		}
+
 		const lineRelativeTop = this._editor.getTopForLineNumber(this._editor.getPosition().lineNumber) - this._editor.getScrollTop();
 		let closestDistance = Number.MAX_VALUE;
 
@@ -549,33 +572,30 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 			}
 		}
 
-		if (!(widget instanceof DiffHunkWidget)) {
-			return;
-		}
-
-
-		const lineNumber = widget.getStartLineNumber();
-		const position = lineNumber ? new Position(lineNumber, 1) : undefined;
 		let selection = this._editor.getSelection();
-		if (position && !selection.containsPosition(position)) {
-			selection = Selection.fromPositions(position);
+		if (widget instanceof DiffHunkWidget) {
+			const lineNumber = widget.getStartLineNumber();
+			const position = lineNumber ? new Position(lineNumber, 1) : undefined;
+			if (position && !selection.containsPosition(position)) {
+				selection = Selection.fromPositions(position);
+			}
 		}
 
 		const isDiffEditor = this._editor.getOption(EditorOption.inDiffEditor);
 
 		if (isDiffEditor) {
 			// normal EDITOR
-			await this._editorService.openEditor({ resource: widget.entry.modifiedURI });
+			await this._editorService.openEditor({ resource: entry.modifiedURI });
 
 		} else {
 			// DIFF editor
 			const defaultAgentName = this._chatAgentService.getDefaultAgent(ChatAgentLocation.EditingSession)?.fullName;
 			const diffEditor = await this._editorService.openEditor({
-				original: { resource: widget.entry.originalURI, options: { selection: undefined } },
-				modified: { resource: widget.entry.modifiedURI, options: { selection } },
+				original: { resource: entry.originalURI, options: { selection: undefined } },
+				modified: { resource: entry.modifiedURI, options: { selection } },
 				label: defaultAgentName
-					? localize('diff.agent', '{0} (changes from {1})', basename(widget.entry.modifiedURI), defaultAgentName)
-					: localize('diff.generic', '{0} (changes from chat)', basename(widget.entry.modifiedURI))
+					? localize('diff.agent', '{0} (changes from {1})', basename(entry.modifiedURI), defaultAgentName)
+					: localize('diff.generic', '{0} (changes from chat)', basename(entry.modifiedURI))
 			});
 
 			if (diffEditor && diffEditor.input) {
@@ -586,7 +606,7 @@ export class ChatEditorController extends Disposable implements IEditorContribut
 
 				// close diff editor when entry is decided
 				const d = autorun(r => {
-					const state = widget.entry.state.read(r);
+					const state = entry.state.read(r);
 					if (state === WorkingSetEntryState.Accepted || state === WorkingSetEntryState.Rejected) {
 						d.dispose();
 						this._editorService.closeEditor(editorIdent);

src/vs/workbench/contrib/chat/browser/chatEditorOverlay.ts

@@ -28,10 +28,11 @@ import { ChatEditorController } from './chatEditorController.js';
 import { EditorOption } from '../../../../editor/common/config/editorOptions.js';
 import { isDiffEditorForEntry } from './chatEditing/chatEditing.js';
 import './media/chatEditorOverlay.css';
+import { findDiffEditorContainingCodeEditor } from '../../../../editor/browser/widget/diffEditor/commands.js';
 
 class ChatEditorOverlayWidget implements IOverlayWidget {
 
-	readonly allowEditorOverflow = false;
+	readonly allowEditorOverflow = true;
 
 	private readonly _domNode: HTMLElement;
 	private readonly _progressNode: HTMLElement;
@@ -47,7 +48,7 @@ class ChatEditorOverlayWidget implements IOverlayWidget {
 	constructor(
 		private readonly _editor: ICodeEditor,
 		@IEditorService editorService: IEditorService,
-		@IInstantiationService instaService: IInstantiationService,
+		@IInstantiationService private readonly _instaService: IInstantiationService,
 	) {
 		this._domNode = document.createElement('div');
 		this._domNode.classList.add('chat-editor-overlay-widget');
@@ -62,7 +63,7 @@ class ChatEditorOverlayWidget implements IOverlayWidget {
 		toolbarNode.classList.add('chat-editor-overlay-toolbar');
 		this._domNode.appendChild(toolbarNode);
 
-		this._toolbar = instaService.createInstance(MenuWorkbenchToolBar, toolbarNode, MenuId.ChatEditingEditorContent, {
+		this._toolbar = _instaService.createInstance(MenuWorkbenchToolBar, toolbarNode, MenuId.ChatEditingEditorContent, {
 			telemetrySource: 'chatEditor.overlayToolbar',
 			hiddenItemStrategy: HiddenItemStrategy.Ignore,
 			toolbarOptions: {
@@ -228,6 +229,19 @@ class ChatEditorOverlayWidget implements IOverlayWidget {
 			this._navigationBearings.set({ changeCount: changes, activeIdx, entriesCount: entries.length }, undefined);
 		}));
 
+
+		const editorWithObs = observableFromEvent(this._editor.onDidLayoutChange, () => {
+			const diffEditor = this._instaService.invokeFunction(findDiffEditorContainingCodeEditor, this._editor);
+			return diffEditor
+				? diffEditor.getOriginalEditor().getLayoutInfo().contentWidth + diffEditor.getModifiedEditor().getLayoutInfo().contentWidth
+				: this._editor.getLayoutInfo().contentWidth;
+		});
+
+		this._showStore.add(autorun(r => {
+			const width = editorWithObs.read(r);
+			this._domNode.style.maxWidth = `${width - 20}px`;
+		}));
+
 		if (!this._isAdded) {
 			this._editor.addOverlayWidget(this);
 			this._isAdded = true;

src/vs/workbench/contrib/chat/browser/media/chatEditorOverlay.css

@@ -13,6 +13,7 @@
 	align-items: center;
 	z-index: 10;
 	box-shadow: 0 2px 8px var(--vscode-widget-shadow);
+	overflow: hidden;
 }
 
 @keyframes pulse {