From f9f14453e683eb0c27be7ea06c9712cbf5ab5487 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Wed, 29 Jan 2025 18:28:03 +0100 Subject: [PATCH] Remove insertion view and enhance edit growth logic (#239123) Do not show insertion view anymore. Grow the edit if possible, otherwise --- .../browser/view/inlineEdits/view.ts | 54 +++++++++++++------ .../view/inlineEdits/wordReplacementView.ts | 3 ++ 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts index 4bc8b06bc734b..636137c0fa64e 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts @@ -26,7 +26,7 @@ import { InlineEditsSideBySideDiff } from './sideBySideDiff.js'; import { applyEditToModifiedRangeMappings, createReindentEdit } from './utils.js'; import './view.css'; import { InlineEditWithChanges } from './viewAndDiffProducer.js'; -import { LineReplacementView, WordInsertView, WordReplacementView } from './wordReplacementView.js'; +import { LineReplacementView, WordReplacementView } from './wordReplacementView.js'; export class InlineEditsView extends Disposable { private readonly _editorObs = observableCodeEditor(this._editor); @@ -164,11 +164,7 @@ export class InlineEditsView extends Disposable { protected readonly _inlineDiffView = this._register(new OriginalEditorInlineDiffView(this._editor, this._inlineDiffViewState, this._previewTextModel)); protected readonly _wordReplacementViews = mapObservableArrayCached(this, this._uiState.map(s => s?.state?.kind === 'wordReplacements' ? s.state.replacements : []), (e, store) => { - if (e.range.isEmpty()) { - return store.add(this._instantiationService.createInstance(WordInsertView, this._editorObs, e)); - } else { - return store.add(this._instantiationService.createInstance(WordReplacementView, this._editorObs, e, [e])); - } + return store.add(this._instantiationService.createInstance(WordReplacementView, this._editorObs, e, [e])); }).recomputeInitiallyAndOnChange(this._store); protected readonly _lineReplacementView = mapObservableArrayCached(this, this._uiState.map(s => s?.state?.kind === 'lineReplacement' ? [s.state] : []), (e, store) => { // TODO: no need for map here, how can this be done with observables @@ -275,10 +271,18 @@ 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); + const allInnerChangesNotTooLong = inner.every(m => TextLength.ofRange(m.originalRange).columnCount < WordReplacementView.MAX_LENGTH && TextLength.ofRange(m.modifiedRange).columnCount < WordReplacementView.MAX_LENGTH); if (allInnerChangesNotTooLong && isSingleInnerEdit && numOriginalLines === 1 && numModifiedLines === 1) { - return 'wordReplacements'; - } else if (numOriginalLines > 0 && numModifiedLines > 0 && !InlineEditsSideBySideDiff.fitsInsideViewport(this._editor, edit, reader)) { + // Make sure there is no insertion, even if we grow them + if ( + !inner.some(m => m.originalRange.isEmpty()) || + !growEditsUntilWhitespace(inner.map(m => new SingleTextEdit(m.originalRange, '')), edit.originalText).some(e => e.range.isEmpty() && TextLength.ofRange(e.range).columnCount < WordReplacementView.MAX_LENGTH) + ) { + return 'wordReplacements'; + } + } + + if (numOriginalLines > 0 && numModifiedLines > 0 && !InlineEditsSideBySideDiff.fitsInsideViewport(this._editor, edit, reader)) { return 'lineReplacement'; } @@ -336,9 +340,15 @@ export class InlineEditsView extends Disposable { } if (view === 'wordReplacements') { + let grownEdits = growEditsToEntireWord(replacements, edit.originalText); + + if (grownEdits.some(e => e.range.isEmpty())) { + grownEdits = growEditsUntilWhitespace(replacements, edit.originalText); + } + return { kind: 'wordReplacements' as const, - replacements: growEditsToEntireWord(replacements, edit.originalText), + replacements: grownEdits, }; } @@ -417,6 +427,14 @@ function isSingleLineDeletion(diff: DetailedLineRangeMapping[]): boolean { } function growEditsToEntireWord(replacements: SingleTextEdit[], originalText: AbstractText): SingleTextEdit[] { + return _growEdits(replacements, originalText, (char) => /^[a-zA-Z]$/.test(char)); +} + +function growEditsUntilWhitespace(replacements: SingleTextEdit[], originalText: AbstractText): SingleTextEdit[] { + return _growEdits(replacements, originalText, (char) => !(/^\s$/.test(char))); +} + +function _growEdits(replacements: SingleTextEdit[], originalText: AbstractText, fn: (c: string) => boolean): SingleTextEdit[] { const result: SingleTextEdit[] = []; replacements.sort((a, b) => Range.compareRangesUsingStarts(a.range, b.range)); @@ -429,17 +447,17 @@ function growEditsToEntireWord(replacements: SingleTextEdit[], originalText: Abs const startLineContent = originalText.getLineAt(edit.range.startLineNumber); const endLineContent = originalText.getLineAt(edit.range.endLineNumber); - if (isWordChar(startLineContent[startIndex])) { + if (isIncluded(startLineContent[startIndex])) { // grow to the left - while (isWordChar(startLineContent[startIndex - 1])) { + while (isIncluded(startLineContent[startIndex - 1])) { prefix = startLineContent[startIndex - 1] + prefix; startIndex--; } } - if (isWordChar(endLineContent[endIndex])) { + if (isIncluded(endLineContent[endIndex]) || endIndex < startIndex) { // grow to the right - while (isWordChar(endLineContent[endIndex + 1])) { + while (isIncluded(endLineContent[endIndex + 1])) { suffix += endLineContent[endIndex + 1]; endIndex++; } @@ -454,9 +472,11 @@ function growEditsToEntireWord(replacements: SingleTextEdit[], originalText: Abs result.push(newEdit); } - function isWordChar(char: string | undefined) { - if (!char) { return false; } - return /^[a-zA-Z]$/.test(char); + function isIncluded(c: string | undefined) { + if (c === undefined) { + return false; + } + return fn(c); } return result; diff --git a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/wordReplacementView.ts b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/wordReplacementView.ts index fe5996d1db9cc..2eccc056602dc 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/wordReplacementView.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/wordReplacementView.ts @@ -40,6 +40,9 @@ export const transparentHoverBackground = registerColor( ); export class WordReplacementView extends Disposable implements IInlineEditsView { + + public static MAX_LENGTH = 100; + private readonly _start = this._editor.observePosition(constObservable(this._edit.range.getStartPosition()), this._store); private readonly _end = this._editor.observePosition(constObservable(this._edit.range.getEndPosition()), this._store);