Skip to content

Commit

Permalink
Properly trim prefixes in inline edits (#238672)
Browse files Browse the repository at this point in the history 
proper prefix trim
  • Loading branch information
@benibenj
benibenj authored Jan 24, 2025
1 parent 1384466 commit 7717b47
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 73 deletions.
2 changes: 1 addition & 1 deletion src/vs/editor/common/config/editorOptions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4250,7 +4250,7 @@ class InlineEditorSuggest extends BaseEditorOption<EditorOption.inlineSuggest, I
useMixedLinesDiff: 'forStableInsertions',
useInterleavedLinesDiff: 'never',
useGutterIndicator: true,
useCodeOverlay: 'whenPossible',
useCodeOverlay: 'moveCodeWhenPossible',
},
},
};
Expand Down
22 changes: 14 additions & 8 deletions src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/deletionView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { IObservable, constObservable, derived, derivedObservableWithCache } fro
import { ICodeEditor } from '../../../../../browser/editorBrowser.js';
import { observableCodeEditor } from '../../../../../browser/observableCodeEditor.js';
import { Point } from '../../../../../browser/point.js';
import { EditorOption } from '../../../../../common/config/editorOptions.js';
import { LineRange } from '../../../../../common/core/lineRange.js';
import { Position } from '../../../../../common/core/position.js';
import { Range } from '../../../../../common/core/range.js';
import { IInlineEditsView } from './sideBySideDiff.js';
import { createRectangle, mapOutFalsy, maxContentWidthInRange, n } from './utils.js';
import { createRectangle, getPrefixTrim, mapOutFalsy, maxContentWidthInRange, n } from './utils.js';
import { InlineEditWithChanges } from './viewAndDiffProducer.js';

export class InlineEditsDeletionView extends Disposable implements IInlineEditsView {
Expand All @@ -21,9 +21,8 @@ export class InlineEditsDeletionView extends Disposable implements IInlineEditsV
private readonly _editor: ICodeEditor,
private readonly _edit: IObservable<InlineEditWithChanges | undefined>,
private readonly _uiState: IObservable<{
edit: InlineEditWithChanges;
originalDisplayRange: LineRange;
widgetStartColumn: number;
originalRange: LineRange;
deletions: Range[];
} | undefined>,
) {
super();
Expand Down Expand Up @@ -55,7 +54,7 @@ export class InlineEditsDeletionView extends Disposable implements IInlineEditsV
private readonly _originalVerticalStartPosition = this._editorObs.observePosition(this._originalStartPosition, this._store).map(p => p?.y);
private readonly _originalVerticalEndPosition = this._editorObs.observePosition(this._originalEndPosition, this._store).map(p => p?.y);

private readonly _originalDisplayRange = this._uiState.map(s => s?.originalDisplayRange);
private readonly _originalDisplayRange = this._uiState.map(s => s?.originalRange);
private readonly _editorMaxContentWidthInRange = derived(this, reader => {
const originalDisplayRange = this._originalDisplayRange.read(reader);
if (!originalDisplayRange) {
Expand All @@ -71,6 +70,14 @@ export class InlineEditsDeletionView extends Disposable implements IInlineEditsV
});
}).map((v, r) => v.read(r));

private readonly _maxPrefixTrim = derived(reader => {
const state = this._uiState.read(reader);
if (!state) {
return { prefixTrim: 0, prefixLeftOffset: 0 };
}
return getPrefixTrim(state.deletions, state.originalRange, [], this._editor);
});

private readonly _editorLayoutInfo = derived(this, (reader) => {
const inlineEdit = this._edit.read(reader);
if (!inlineEdit) {
Expand All @@ -81,7 +88,6 @@ export class InlineEditsDeletionView extends Disposable implements IInlineEditsV
return null;
}

const w = this._editorObs.getOption(EditorOption.fontInfo).read(reader).typicalHalfwidthCharacterWidth;
const editorLayout = this._editorObs.layoutInfo.read(reader);
const horizontalScrollOffset = this._editorObs.scrollLeft.read(reader);

Expand All @@ -91,7 +97,7 @@ export class InlineEditsDeletionView extends Disposable implements IInlineEditsV
const selectionTop = this._originalVerticalStartPosition.read(reader) ?? this._editor.getTopForLineNumber(range.startLineNumber) - this._editorObs.scrollTop.read(reader);
const selectionBottom = this._originalVerticalEndPosition.read(reader) ?? this._editor.getTopForLineNumber(range.endLineNumberExclusive) - this._editorObs.scrollTop.read(reader);

const codeLeft = editorLayout.contentLeft + state.widgetStartColumn * w;
const codeLeft = editorLayout.contentLeft + this._maxPrefixTrim.read(reader).prefixLeftOffset;

if (left <= codeLeft) {
return null;
Expand Down
18 changes: 18 additions & 0 deletions src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { OS } from '../../../../../../base/common/platform.js';
import { getIndentationLength, splitLines } from '../../../../../../base/common/strings.js';
import { URI } from '../../../../../../base/common/uri.js';
import { MenuEntryActionViewItem } from '../../../../../../platform/actions/browser/menuEntryActionViewItem.js';
import { ICodeEditor } from '../../../../../browser/editorBrowser.js';
import { ObservableCodeEditor } from '../../../../../browser/observableCodeEditor.js';
import { Point } from '../../../../../browser/point.js';
import { Rect } from '../../../../../browser/rect.js';
Expand All @@ -24,6 +25,7 @@ import { Position } from '../../../../../common/core/position.js';
import { Range } from '../../../../../common/core/range.js';
import { SingleTextEdit, TextEdit } from '../../../../../common/core/textEdit.js';
import { RangeMapping } from '../../../../../common/diff/rangeMapping.js';
import { indentOfLine } from '../../../../../common/model/textModel.js';

export function maxContentWidthInRange(editor: ObservableCodeEditor, range: LineRange, reader: IReader | undefined): number {
editor.layoutInfo.read(reader);
Expand Down Expand Up @@ -66,6 +68,22 @@ export function getOffsetForPos(editor: ObservableCodeEditor, pos: Position, rea
return lineContentWidth;
}

export function getPrefixTrim(diffRanges: Range[], originalLinesRange: LineRange, modifiedLines: string[], editor: ICodeEditor): { prefixTrim: number; prefixLeftOffset: number } {
const textModel = editor.getModel();
if (!textModel) {
return { prefixTrim: 0, prefixLeftOffset: 0 };
}

const replacementStart = diffRanges.map(r => r.isSingleLine() ? r.startColumn - 1 : 0);
const originalIndents = originalLinesRange.mapToLineArray(line => indentOfLine(textModel.getLineContent(line)));
const modifiedIndents = modifiedLines.map(line => indentOfLine(line));
const prefixTrim = Math.min(...replacementStart, ...originalIndents, ...modifiedIndents);

const prefixLeftOffset = editor.getOffsetForColumn(originalLinesRange.startLineNumber, prefixTrim + 1);

return { prefixTrim, prefixLeftOffset };
}

export class StatusBarViewItem extends MenuEntryActionViewItem {
protected readonly _updateLabelListener = this._register(this._contextKeyService.onDidChangeContext(() => {
this.updateLabel();
Expand Down
40 changes: 8 additions & 32 deletions src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,8 @@ export class InlineEditsView extends Disposable {
this._editor,
this._edit,
this._uiState.map(s => s && s.state?.kind === 'deletion' ? ({
edit: s.edit,
originalDisplayRange: s.originalDisplayRange,
widgetStartColumn: s.state.widgetStartColumn,
originalRange: s.state.originalRange,
deletions: s.state.deletions,
}) : undefined),
));

Expand Down Expand Up @@ -244,7 +243,7 @@ export class InlineEditsView extends Disposable {
const numOriginalLines = edit.originalLineRange.length;
const numModifiedLines = edit.modifiedLineRange.length;
const allInnerChangesNotTooLong = inner.every(m => TextLength.ofRange(m.originalRange).columnCount < 100 && TextLength.ofRange(m.modifiedRange).columnCount < 100);
if (allInnerChangesNotTooLong && isSingleInnerEdit && numOriginalLines === 1 && numModifiedLines === 1 && useCodeOverlay === 'whenPossible') {
if (allInnerChangesNotTooLong && isSingleInnerEdit && numOriginalLines === 1 && numModifiedLines === 1) {
return 'wordReplacements';
} else if (numOriginalLines > 0 && numModifiedLines > 0 && !InlineEditsSideBySideDiff.fitsInsideViewport(this._editor, edit, reader)) {
return 'lineReplacement';
Expand Down Expand Up @@ -282,9 +281,11 @@ export class InlineEditsView extends Disposable {
const inner = diff.flatMap(d => d.innerChanges ?? []);

if (view === 'deletion') {
const trimLength = getPrefixTrimLength(edit, inner, newText);
const widgetStartColumn = Math.min(trimLength, ...inner.map(m => m.originalRange.startLineNumber !== m.originalRange.endLineNumber ? 0 : m.originalRange.startColumn - 1));
return { kind: 'deletion' as const, widgetStartColumn };
return {
kind: 'deletion' as const,
originalRange: edit.originalLineRange,
deletions: inner.map(m => m.originalRange),
};
}

const replacements = inner.map(m => new SingleTextEdit(m.originalRange, newText.getValueOfRange(m.modifiedRange)));
Expand Down Expand Up @@ -354,28 +355,3 @@ function isSingleLineDeletion(diff: DetailedLineRangeMapping[]): boolean {
return true;
}
}

function getPrefixTrimLength(edit: InlineEditWithChanges, innerChanges: RangeMapping[], newText: StringText) {
if (innerChanges.some(m => m.originalRange.startLineNumber !== m.originalRange.endLineNumber)) {
return 0;
}

let minTrimLength = Number.MAX_SAFE_INTEGER;
for (let i = 0; i < edit.originalLineRange.length; i++) {
const lineNumber = edit.originalLineRange.startLineNumber + i;
const originalLine = edit.originalText.getLineAt(lineNumber);
const editedLine = newText.getLineAt(lineNumber);
const trimLength = getLinePrefixTrimLength(originalLine, editedLine);
minTrimLength = Math.min(minTrimLength, trimLength);
}

return Math.min(minTrimLength, ...innerChanges.map(m => m.originalRange.startColumn - 1));

function getLinePrefixTrimLength(originalLine: string, editedLine: string) {
let startTrim = 0;
while (originalLine[startTrim] === editedLine[startTrim] && (originalLine[startTrim] === ' ' || originalLine[startTrim] === '\t')) {
startTrim++;
}
return startTrim;
}
}
40 changes: 8 additions & 32 deletions src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/wordReplacementView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { SingleTextEdit } from '../../../../../common/core/textEdit.js';
import { ILanguageService } from '../../../../../common/languages/language.js';
import { LineTokens } from '../../../../../common/tokens/lineTokens.js';
import { TokenArray } from '../../../../../common/tokens/tokenArray.js';
import { mapOutFalsy, n, rectToProps } from './utils.js';
import { getPrefixTrim, mapOutFalsy, n, rectToProps } from './utils.js';
import { localize } from '../../../../../../nls.js';
import { IInlineEditsView } from './sideBySideDiff.js';
import { Range } from '../../../../../common/core/range.js';
Expand Down Expand Up @@ -274,35 +274,13 @@ export class LineReplacementView extends Disposable implements IInlineEditsView
stickiness: TrackedRangeStickiness.NeverGrowsWhenTypingAtEdges
};

private readonly maxPrefixTrim = derived(this, reader => {
const maxPrefixTrim = Math.max(...this._replacements.flatMap(r => [r.originalRange, r.modifiedRange]).map(r => r.isSingleLine() ? r.startColumn - 1 : 0));
if (maxPrefixTrim === 0) {
return 0;
}

const textModel = this._editor.editor.getModel()!;

const getLineTrimColLength = (line: string) => {
let i = 0;
while (i < line.length && line[i] === ' ') { i++; }
return i;
};

// TODO: make sure this works for tabs
return Math.min(
maxPrefixTrim,
...this._originalRange.mapToLineArray(line => getLineTrimColLength(textModel.getLineContent(line))),
...this._modifiedLines.map(line => getLineTrimColLength(line))
);
});
private readonly _maxPrefixTrim = getPrefixTrim(this._replacements.flatMap(r => [r.originalRange, r.modifiedRange]), this._originalRange, this._modifiedLines, this._editor.editor);

private readonly _modifiedLineElements = derived(reader => {

const maxPrefixTrim = this.maxPrefixTrim.read(reader);

const lines = [];
let requiredWidth = 0;

const maxPrefixTrim = this._maxPrefixTrim.prefixTrim;
const modifiedBubbles = rangesToBubbleRanges(this._replacements.map(r => r.modifiedRange)).map(r => new Range(r.startLineNumber, r.startColumn - maxPrefixTrim, r.endLineNumber, r.endColumn - maxPrefixTrim));

const textModel = this._editor.model.get()!;
Expand Down Expand Up @@ -351,12 +329,12 @@ export class LineReplacementView extends Disposable implements IInlineEditsView
const PADDING = 4;

const editorModel = this._editor.editor.getModel()!;
const maxPrefixTrim = this.maxPrefixTrim.read(reader);
const { prefixTrim, prefixLeftOffset } = this._maxPrefixTrim;

// TODO, correctly count tabs
// TODO: correctly count tabs
const originalLineContents: string[] = [];
this._originalRange.forEach(line => originalLineContents.push(editorModel.getLineContent(line)));
const maxOriginalLineLength = Math.max(...originalLineContents.map(l => l.length)) - maxPrefixTrim;
const maxOriginalLineLength = Math.max(...originalLineContents.map(l => l.length)) - prefixTrim;
const maxLineWidth = Math.max(maxOriginalLineLength * w, requiredWidth);

const startLineNumber = this._originalRange.startLineNumber;
Expand All @@ -369,11 +347,9 @@ export class LineReplacementView extends Disposable implements IInlineEditsView
return undefined;
}

const prefixTrimOffset = maxPrefixTrim * w;

// Box Widget positioning
const originalLinesOverlay = Rect.fromLeftTopWidthHeight(
editorLeftOffset + prefixTrimOffset,
editorLeftOffset + prefixLeftOffset,
topOfOriginalLines,
maxLineWidth,
bottomOfOriginalLines - topOfOriginalLines + PADDING
Expand Down Expand Up @@ -407,7 +383,7 @@ export class LineReplacementView extends Disposable implements IInlineEditsView
lowerBackground,
lowerText,
padding: PADDING,
minContentWidthRequired: prefixTrimOffset + maxLineWidth + PADDING * 2,
minContentWidthRequired: maxLineWidth + PADDING * 2,
};
});

Expand Down

0 comments on commit 7717b47

Please sign in to comment.