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.

Sign up for GitHub

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

improve diff for /edits #237801

Merged
merged 2 commits into from
Jan 13, 2025
+96 −46
Merged

improve diff for /edits #237801

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe37bc0
improve diff for /edits
jrieken Jan 13, 2025
70989f9
fix compiler
jrieken Jan 13, 2025
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
17 changes: 15 additions & 2 deletions src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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 },
Expand All @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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'
);

Expand Down
100 changes: 60 additions & 40 deletions src/vs/workbench/contrib/chat/browser/chatEditorController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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({
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand All @@ -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) {
Expand All @@ -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);
Expand Down
20 changes: 17 additions & 3 deletions src/vs/workbench/contrib/chat/browser/chatEditorOverlay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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');
Expand All @@ -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: {
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/chat/browser/media/chatEditorOverlay.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
align-items: center;
z-index: 10;
box-shadow: 0 2px 8px var(--vscode-widget-shadow);
overflow: hidden;
}

@keyframes pulse {
Expand Down
Loading