Skip to content

Cleanup display ranges and support alternative display ranges #246196

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

Merged
benibenj merged 1 commit into from
Apr 10, 2025
Merged

Cleanup display ranges and support alternative display ranges #246196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/inlineEditWithChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { SingleLineEdit } from '../../../../../common/core/lineEdit.js';
import { LineRange } from '../../../../../common/core/lineRange.js';
import { Position } from '../../../../../common/core/position.js';
import { AbstractText, TextEdit } from '../../../../../common/core/textEdit.js';
import { Command } from '../../../../../common/languages.js';
Expand All @@ -17,6 +18,14 @@ export class InlineEditWithChanges {
public get originalLineRange() { return this.lineEdit.lineRange; }
public get modifiedLineRange() { return this.lineEdit.toLineEdit().getNewLineRanges()[0]; }

public get displayRange() {
return this.originalText.lineRange.intersect(
this.originalLineRange.join(
LineRange.ofLength(this.originalLineRange.startLineNumber, this.lineEdit.newLines.length)
)
)!;
}

constructor(
public readonly originalText: AbstractText,
public readonly edit: TextEdit,
Expand Down
36 changes: 19 additions & 17 deletions src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/inlineEditsView.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ export class InlineEditsView extends Disposable {
edit: InlineEditWithChanges;
newText: string;
newTextLineCount: number;
originalDisplayRange: LineRange;
} | undefined>(this, reader => {
const model = this._model.read(reader);
if (!model || !this._constructorDone.read(reader)) {
Expand All @@ -121,13 +120,7 @@ export class InlineEditsView extends Disposable {
let newText = inlineEdit.edit.apply(inlineEdit.originalText);
let diff = lineRangeMappingFromRangeMappings(mappings, inlineEdit.originalText, new StringText(newText));

const originalDisplayRange = inlineEdit.originalText.lineRange.intersect(
inlineEdit.originalLineRange.join(
LineRange.ofLength(inlineEdit.originalLineRange.startLineNumber, inlineEdit.lineEdit.newLines.length)
)
)!;

let state = this.determineRenderState(model, reader, diff, new StringText(newText), originalDisplayRange);
let state = this.determineRenderState(model, reader, diff, new StringText(newText));
if (!state) {
model.abort(`unable to determine view: tried to render ${this._previousView?.view}`);
return undefined;
Expand Down Expand Up @@ -159,7 +152,6 @@ export class InlineEditsView extends Disposable {
edit: inlineEdit,
newText,
newTextLineCount: inlineEdit.modifiedLineRange.length,
originalDisplayRange: originalDisplayRange,
};
});

Expand All @@ -185,10 +177,21 @@ export class InlineEditsView extends Disposable {
}

const state = this._uiState.read(reader);
if (state?.state?.kind === 'insertionMultiLine') {
if (!state) { return undefined; }

if (state.state?.kind === 'custom') {
const range = state.state.displayLocation?.range;
if (!range) {
throw new BugIndicatingError('custom view should have a range');
}
return new LineRange(range.startLineNumber, range.endLineNumber);
}

if (state.state?.kind === 'insertionMultiLine') {
return this._insertion.originalLines.read(reader);
}
return state?.originalDisplayRange;

return state.edit.displayRange;
});

const modelWithGhostTextSupport = derived<InlineEditModel | undefined>(this, reader => {
Expand Down Expand Up @@ -240,7 +243,6 @@ export class InlineEditsView extends Disposable {
this._previewTextModel,
this._uiState.map(s => s && s.state?.kind === 'sideBySide' ? ({
newTextLineCount: s.newTextLineCount,
originalDisplayRange: s.originalDisplayRange,
}) : undefined),
this._tabAction,
));
Expand Down Expand Up @@ -311,7 +313,7 @@ export class InlineEditsView extends Disposable {
return model.inlineEdit.inlineCompletion.identity.id;
}

private determineView(model: IInlineEditModel, reader: IReader, diff: DetailedLineRangeMapping[], newText: StringText, originalDisplayRange: LineRange): string {
private determineView(model: IInlineEditModel, reader: IReader, diff: DetailedLineRangeMapping[], newText: StringText): string {
// Check if we can use the previous view if it is the same InlineCompletion as previously shown
const inlineEdit = model.inlineEdit;
const canUseCache = this._previousView?.id === this.getCacheId(model);
Expand Down Expand Up @@ -363,7 +365,7 @@ export class InlineEditsView extends Disposable {
}
}
if (numOriginalLines > 0 && numModifiedLines > 0) {
if (this._renderSideBySide.read(reader) !== 'never' && InlineEditsSideBySideView.fitsInsideViewport(this._editor, this._previewTextModel, inlineEdit, originalDisplayRange, reader)) {
if (this._renderSideBySide.read(reader) !== 'never' && InlineEditsSideBySideView.fitsInsideViewport(this._editor, this._previewTextModel, inlineEdit, reader)) {
return 'sideBySide';
}

Expand All @@ -373,15 +375,15 @@ export class InlineEditsView extends Disposable {
return 'sideBySide';
}

private determineRenderState(model: IInlineEditModel, reader: IReader, diff: DetailedLineRangeMapping[], newText: StringText, originalDisplayRange: LineRange) {
private determineRenderState(model: IInlineEditModel, reader: IReader, diff: DetailedLineRangeMapping[], newText: StringText) {
const inlineEdit = model.inlineEdit;

const view = this.determineView(model, reader, diff, newText, originalDisplayRange);
const view = this.determineView(model, reader, diff, newText);

this._previousView = { id: this.getCacheId(model), view, editorWidth: this._editor.getLayoutInfo().width, timestamp: Date.now() };

switch (view) {
case 'custom': return { kind: 'custom' as const };
case 'custom': return { kind: 'custom' as const, displayLocation: model.displayLocation };
case 'insertionInline': return { kind: 'insertionInline' as const };
case 'sideBySide': return { kind: 'sideBySide' as const };
case 'collapsed': return { kind: 'collapsed' as const };
Expand Down
8 changes: 3 additions & 5 deletions .../inlineCompletions/browser/view/inlineEdits/inlineEditsViews/inlineEditsSideBySideView.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { observableCodeEditor } from '../../../../../../browser/observableCodeEd
import { Rect } from '../../../../../../browser/rect.js';
import { EmbeddedCodeEditorWidget } from '../../../../../../browser/widget/codeEditor/embeddedCodeEditorWidget.js';
import { EditorOption } from '../../../../../../common/config/editorOptions.js';
import { LineRange } from '../../../../../../common/core/lineRange.js';
import { OffsetRange } from '../../../../../../common/core/offsetRange.js';
import { Position } from '../../../../../../common/core/position.js';
import { Range } from '../../../../../../common/core/range.js';
Expand All @@ -42,14 +41,14 @@ const MODIFIED_END_PADDING = 12;
export class InlineEditsSideBySideView extends Disposable implements IInlineEditsView {

// This is an approximation and should be improved by using the real parameters used bellow
static fitsInsideViewport(editor: ICodeEditor, textModel: ITextModel, edit: InlineEditWithChanges, originalDisplayRange: LineRange, reader: IReader): boolean {
static fitsInsideViewport(editor: ICodeEditor, textModel: ITextModel, edit: InlineEditWithChanges, reader: IReader): boolean {
const editorObs = observableCodeEditor(editor);
const editorWidth = editorObs.layoutInfoWidth.read(reader);
const editorContentLeft = editorObs.layoutInfoContentLeft.read(reader);
const editorVerticalScrollbar = editor.getLayoutInfo().verticalScrollbarWidth;
const minimapWidth = editorObs.layoutInfoMinimap.read(reader).minimapLeft !== 0 ? editorObs.layoutInfoMinimap.read(reader).minimapWidth : 0;

const maxOriginalContent = maxContentWidthInRange(editorObs, originalDisplayRange, undefined/* do not reconsider on each layout info change */);
const maxOriginalContent = maxContentWidthInRange(editorObs, edit.displayRange, undefined/* do not reconsider on each layout info change */);
const maxModifiedContent = edit.lineEdit.newLines.reduce((max, line) => Math.max(max, getContentRenderWidth(line, editor, textModel)), 0);
const originalPadding = ORIGINAL_END_PADDING; // padding after last line of original editor
const modifiedPadding = MODIFIED_END_PADDING + 2 * BORDER_WIDTH; // padding after last line of modified editor
Expand All @@ -68,7 +67,6 @@ export class InlineEditsSideBySideView extends Disposable implements IInlineEdit
private readonly _previewTextModel: ITextModel,
private readonly _uiState: IObservable<{
newTextLineCount: number;
originalDisplayRange: LineRange;
} | undefined>,
private readonly _tabAction: IObservable<InlineEditTabAction>,
@IInstantiationService private readonly _instantiationService: IInstantiationService,
Expand Down Expand Up @@ -260,7 +258,7 @@ export class InlineEditsSideBySideView extends Disposable implements IInlineEdit
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._edit.map(e => e?.displayRange);
private readonly _editorMaxContentWidthInRange = derived(this, reader => {
const originalDisplayRange = this._originalDisplayRange.read(reader);
if (!originalDisplayRange) {
Expand Down
Loading