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.

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

Choose line replacement or side-by-side view based on editor width #238659

Merged
merged 2 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/vs/editor/browser/observableCodeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ export class ObservableCodeEditor extends Disposable {
public readonly layoutInfo = observableFromEvent(this.editor.onDidLayoutChange, () => this.editor.getLayoutInfo());
public readonly layoutInfoContentLeft = this.layoutInfo.map(l => l.contentLeft);
public readonly layoutInfoDecorationsLeft = this.layoutInfo.map(l => l.decorationsLeft);
public readonly layoutInfoWidth = this.layoutInfo.map(l => l.width);

public readonly contentWidth = observableFromEvent(this.editor.onDidContentSizeChange, () => this.editor.getContentWidth());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { IAction } from '../../../../../../base/common/actions.js';
import { Color } from '../../../../../../base/common/color.js';
import { structuralEquals } from '../../../../../../base/common/equals.js';
import { Disposable } from '../../../../../../base/common/lifecycle.js';
import { IObservable, autorun, constObservable, derived, derivedObservableWithCache, derivedOpts, observableFromEvent } from '../../../../../../base/common/observable.js';
import { IObservable, IReader, autorun, constObservable, derived, derivedObservableWithCache, derivedOpts, observableFromEvent } from '../../../../../../base/common/observable.js';
import { MenuId, MenuItemAction } from '../../../../../../platform/actions/common/actions.js';
import { ICommandService } from '../../../../../../platform/commands/common/commands.js';
import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
Expand Down Expand Up @@ -111,7 +111,27 @@ export interface IInlineEditsView {
isHovered: IObservable<boolean>;
}

const PADDING = 4;
const ENABLE_OVERFLOW = false;

export class InlineEditsSideBySideDiff extends Disposable implements IInlineEditsView {

// This is an approximation and should be improved by using the real parameters used bellow
static fitsInsideViewport(editor: ICodeEditor, 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 w = editor.getOption(EditorOption.fontInfo).typicalHalfwidthCharacterWidth;

const maxOriginalContent = maxContentWidthInRange(editorObs, edit.originalLineRange, undefined/* do not reconsider on each layout info change */);
const maxModifiedContent = edit.lineEdit.newLines.reduce((max, line) => Math.max(max, line.length * w), 0);
const endOfEditorPadding = 20; // padding after last line of editor
const editorsPadding = edit.modifiedLineRange.length <= edit.originalLineRange.length ? PADDING * 3 + endOfEditorPadding : 60 + endOfEditorPadding * 2;

return maxOriginalContent + maxModifiedContent + editorsPadding < editorWidth - editorContentLeft - editorVerticalScrollBar;
}

private readonly _editorObs = observableCodeEditor(this._editor);

constructor(
Expand Down Expand Up @@ -471,7 +491,6 @@ export class InlineEditsSideBySideDiff extends Disposable implements IInlineEdit
: new OffsetRange(60, 61);

const clipped = dist === 0;
const PADDING = 4;

const codeEditDist = editIsSameHeight ? PADDING : codeEditDistRange.clip(dist); // TODO: Is there a better way to specify the distance?

Expand Down Expand Up @@ -527,6 +546,9 @@ export class InlineEditsSideBySideDiff extends Disposable implements IInlineEdit
private readonly _stickyScrollHeight = this._stickyScrollController ? observableFromEvent(this._stickyScrollController.onDidChangeStickyScrollHeight, () => this._stickyScrollController!.stickyScrollWidgetHeight) : constObservable(0);

private readonly _shouldOverflow = derived(reader => {
if (!ENABLE_OVERFLOW) {
return false;
}
const range = this._edit.read(reader)?.originalLineRange;
if (!range) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { Range } from '../../../../../common/core/range.js';
import { SingleTextEdit, TextEdit } from '../../../../../common/core/textEdit.js';
import { RangeMapping } from '../../../../../common/diff/rangeMapping.js';

export function maxContentWidthInRange(editor: ObservableCodeEditor, range: LineRange, reader: IReader): number {
export function maxContentWidthInRange(editor: ObservableCodeEditor, range: LineRange, reader: IReader | undefined): number {
editor.layoutInfo.read(reader);
editor.value.read(reader);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ export class InlineEditsView extends Disposable {
private readonly _useInterleavedLinesDiff = observableCodeEditor(this._editor).getOption(EditorOption.inlineSuggest).map(s => s.edits.experimental.useInterleavedLinesDiff);
private readonly _useCodeOverlay = observableCodeEditor(this._editor).getOption(EditorOption.inlineSuggest).map(s => s.edits.experimental.useCodeOverlay);

private _previousView: { id: string; view: ReturnType<typeof InlineEditsView.prototype.determineView>; userJumpedToIt: boolean } | undefined;
private _previousView: {
id: string;
view: ReturnType<typeof InlineEditsView.prototype.determineView>;
userJumpedToIt: boolean;
editorWidth: number;
} | undefined;

constructor(
private readonly _editor: ICodeEditor,
Expand Down Expand Up @@ -202,11 +207,18 @@ export class InlineEditsView extends Disposable {
(this._useMixedLinesDiff.read(reader) === 'afterJumpWhenPossible' && this._previousView?.view !== 'mixedLines') ||
(this._useInterleavedLinesDiff.read(reader) === 'afterJump' && this._previousView?.view !== 'interleavedLines')
);
const reconsiderViewEditorWidthChange = this._previousView?.editorWidth !== this._editor.getLayoutInfo().width &&
(
(this._previousView?.view === 'sideBySide' && this._useCodeOverlay.read(reader) !== 'never') ||
(this._previousView?.view === 'lineReplacement')
);

if (canUseCache && !reconsiderViewAfterJump) {
if (canUseCache && !reconsiderViewAfterJump && !reconsiderViewEditorWidthChange) {
return this._previousView!.view;
}

// Determine the view based on the edit / diff

if (edit.isCollapsed) {
return 'collapsed';
}
Expand Down Expand Up @@ -234,7 +246,7 @@ export class InlineEditsView extends Disposable {
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') {
return 'wordReplacements';
} else if (numOriginalLines > 0 && numModifiedLines > 0) {
} else if (numOriginalLines > 0 && numModifiedLines > 0 && !InlineEditsSideBySideDiff.fitsInsideViewport(this._editor, edit, reader)) {
return 'lineReplacement';
}
}
Expand All @@ -256,7 +268,8 @@ export class InlineEditsView extends Disposable {
private determineRenderState(edit: InlineEditWithChanges, reader: IReader, diff: DetailedLineRangeMapping[], newText: StringText) {

const view = this.determineView(edit, reader, diff, newText);
this._previousView = { id: edit.inlineCompletion.id, view, userJumpedToIt: edit.userJumpedToIt };

this._previousView = { id: edit.inlineCompletion.id, view, userJumpedToIt: edit.userJumpedToIt, editorWidth: this._editor.getLayoutInfo().width };

switch (view) {
case 'collapsed': return { kind: 'collapsed' as const };
Expand Down
Loading