Skip to content

Commit

Permalink
Choose line replacement or side-by-side view based on editor width (#…
Browse files Browse the repository at this point in the history
…238659)

* pick linereplcement or side by side depending on editor width

* do not enable overflow
  • Loading branch information
benibenj authored Jan 24, 2025
1 parent a740888 commit afdf18f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 7 deletions.
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

0 comments on commit afdf18f

Please sign in to comment.