Skip to content

Commit 32c963b

Browse files
ulugbeknaCopilot
ulugbekna
and
Copilot
committed
nes: refactor: simplify toInlineSuggestion
Co-authored-by: Copilot <copilot@github.com>
1 parent ed59fcd commit 32c963b

File tree

2 files changed

+100
-45
lines changed

2 files changed

+100
-45
lines changed

‎extensions/copilot/src/extension/inlineEdits/test/vscode-node/isInlineSuggestion.spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,4 +644,25 @@ const fieldLabels: Record<keyof FormData, string> = {
644644
assert.strictEqual(result!.newText, '\r\n');
645645
});
646646
});
647+
648+
suite('multi-line range, no common prefix', () => {
649+
650+
// Regression: when commonLen === 0 and the replaced text starts with '\n',
651+
// `lastIndexOf('\n', -1)` would (incorrectly) clamp to 0 and report a
652+
// match, causing the leading newline to be stripped — which can collapse
653+
// the multi-line range into a same-line "suggestion" that the function
654+
// then accepts. With the original substring-based check, no strip occurs
655+
// and the result is `undefined`.
656+
test('does not strip leading newline when nothing is in common', () => {
657+
const document = createMockDocument(['abc', 'x', 'rest']);
658+
// replacedText = '\nx', newText[0]='Y' differs from '\n', commonLen=0.
659+
const replaceRange = new Range(0, 3, 1, 1);
660+
const cursorPosition = new Position(1, 1);
661+
const replaceText = 'Yx';
662+
663+
// The range cannot legitimately be collapsed to a single line, so
664+
// the function must not synthesize a ghost-text suggestion.
665+
assert.isUndefined(toInlineSuggestion(cursorPosition, document, replaceRange, replaceText));
666+
});
667+
});
647668
});

‎extensions/copilot/src/extension/inlineEdits/vscode-node/isInlineSuggestion.ts‎

Lines changed: 79 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Position, Range, TextDocument } from 'vscode';
7-
import { OffsetRange } from '../../../util/vs/editor/common/core/ranges/offsetRange';
87

98
export interface InlineSuggestionEdit {
109
readonly range: Range;
@@ -17,73 +16,108 @@ export interface InlineSuggestionEdit {
1716
* which is required for VS Code to render ghost text.
1817
*/
1918
export function toInlineSuggestion(cursorPos: Position, doc: TextDocument, range: Range, newText: string, advanced: boolean = true): InlineSuggestionEdit | undefined {
20-
// If multi line insertion starts on the next line
21-
// All new lines have to be newly created lines
22-
if (range.isEmpty && cursorPos.line + 1 === range.start.line && range.start.character === 0
23-
&& doc.lineAt(cursorPos.line).text.length === cursorPos.character // cursor is at the end of the line
24-
&& (newText.endsWith('\n') || (newText.includes('\n') && doc.lineAt(range.end.line).text.length === range.end.character)) // no remaining content after insertion
25-
) {
26-
// Use an empty range at the cursor so the suggestion is a pure insertion
27-
const adjustedRange = new Range(cursorPos, cursorPos);
28-
const textBetweenCursorAndRange = doc.getText(new Range(cursorPos, range.start));
29-
// The original range is on the next line, so the line terminator that
30-
// already separates the cursor's line from range.start is preserved.
31-
// Drop a single trailing line ending from newText (if present) to avoid
32-
// inserting an extra blank line after the suggestion. Handle CRLF as
33-
// well as LF so we don't leave a dangling '\r'.
34-
const adjustedNewText = newText.replace(/\r?\n$/, '');
35-
return { range: adjustedRange, newText: textBetweenCursorAndRange + adjustedNewText };
19+
// Special case: a multi-line insertion that starts on the line *after* the cursor
20+
// can be re-expressed as a pure insertion at the cursor.
21+
const nextLineInsertion = tryAdjustNextLineInsertion(cursorPos, doc, range, newText);
22+
if (nextLineInsertion) {
23+
return nextLineInsertion;
3624
}
3725

38-
if (advanced) {
39-
// If the range spans multiple lines, try to reduce it by stripping a common
40-
// prefix (up to a newline boundary) from the replaced text and newText.
41-
if (range.start.line !== range.end.line) {
42-
const fullReplacedText = doc.getText(range);
43-
let commonLen = 0;
44-
const maxLen = Math.min(fullReplacedText.length, newText.length);
45-
while (commonLen < maxLen && fullReplacedText[commonLen] === newText[commonLen]) {
46-
commonLen++;
47-
}
48-
const lastNewline = fullReplacedText.substring(0, commonLen).lastIndexOf('\n');
49-
if (lastNewline >= 0) {
50-
const strippedLen = lastNewline + 1;
51-
newText = newText.substring(strippedLen);
52-
const newStart = doc.positionAt(doc.offsetAt(range.start) + strippedLen);
53-
range = new Range(newStart, range.end);
54-
}
55-
}
26+
// If the range spans multiple lines, try to collapse it to a single line by
27+
// trimming a shared prefix up to a newline boundary.
28+
if (advanced && range.start.line !== range.end.line) {
29+
({ range, newText } = stripCommonLinePrefix(doc, range, newText));
5630
}
5731

32+
// Ghost text requires the edit to be on the cursor's line.
5833
if (range.start.line !== range.end.line || range.start.line !== cursorPos.line) {
5934
return undefined;
6035
}
6136

62-
const cursorOffset = doc.offsetAt(cursorPos);
63-
const offsetRange = new OffsetRange(doc.offsetAt(range.start), doc.offsetAt(range.end));
64-
65-
const replacedText = offsetRange.substring(doc.getText());
37+
return validateSameLineGhostText(cursorPos, doc, range, newText);
38+
}
6639

67-
const cursorOffsetInReplacedText = cursorOffset - offsetRange.start;
68-
if (cursorOffsetInReplacedText < 0) {
40+
/**
41+
* If the cursor is at the end of a line and the edit is an empty-range insertion
42+
* at column 0 of the next line (with no leftover content after the insertion),
43+
* rewrite it as a pure insertion at the cursor position.
44+
*/
45+
function tryAdjustNextLineInsertion(cursorPos: Position, doc: TextDocument, range: Range, newText: string): InlineSuggestionEdit | undefined {
46+
if (!range.isEmpty) {
47+
return undefined;
48+
}
49+
if (cursorPos.line + 1 !== range.start.line || range.start.character !== 0) {
6950
return undefined;
7051
}
52+
if (doc.lineAt(cursorPos.line).text.length !== cursorPos.character) {
53+
return undefined; // cursor is not at the end of the line
54+
}
7155

72-
const textBeforeCursorIsEqual = replacedText.substring(0, cursorOffsetInReplacedText) === newText.substring(0, cursorOffsetInReplacedText);
73-
if (!textBeforeCursorIsEqual) {
56+
const targetLineFullyConsumed = doc.lineAt(range.end.line).text.length === range.end.character;
57+
const noLeftoverAfterInsertion = newText.endsWith('\n') || (newText.includes('\n') && targetLineFullyConsumed);
58+
if (!noLeftoverAfterInsertion) {
7459
return undefined;
7560
}
7661

62+
// Use an empty range at the cursor so the suggestion is a pure insertion.
63+
// The original line terminator between the cursor and `range.start` is preserved
64+
// in the document, so:
65+
// - prepend that terminator to `newText` (it lives in the doc, not in the edit), and
66+
// - drop a single trailing line ending from `newText` to avoid an extra blank line.
67+
// CRLF-safe so we don't leak a dangling '\r' into the suggestion.
68+
const lineBreak = doc.getText(new Range(cursorPos, range.start));
69+
const trimmedNewText = newText.replace(/\r?\n$/, '');
70+
return { range: new Range(cursorPos, cursorPos), newText: lineBreak + trimmedNewText };
71+
}
72+
73+
/**
74+
* Strip the longest shared prefix that ends on a newline boundary from both sides
75+
* of a multi-line edit. This often shrinks the range so it fits on a single line,
76+
* which is required for ghost text rendering.
77+
*/
78+
function stripCommonLinePrefix(doc: TextDocument, range: Range, newText: string): { range: Range; newText: string } {
79+
const replacedText = doc.getText(range);
80+
const maxLen = Math.min(replacedText.length, newText.length);
81+
let commonLen = 0;
82+
while (commonLen < maxLen && replacedText[commonLen] === newText[commonLen]) {
83+
commonLen++;
84+
}
85+
if (commonLen === 0) {
86+
return { range, newText };
87+
}
88+
const lastNewline = replacedText.lastIndexOf('\n', commonLen - 1);
89+
if (lastNewline < 0) {
90+
return { range, newText };
91+
}
92+
const strippedLen = lastNewline + 1;
93+
const newStart = doc.positionAt(doc.offsetAt(range.start) + strippedLen);
94+
return { range: new Range(newStart, range.end), newText: newText.substring(strippedLen) };
95+
}
96+
97+
/**
98+
* Validate that a single-line edit can be rendered as ghost text at the cursor:
99+
* - the cursor is at or after `range.start`
100+
* - everything before the cursor in the replaced text matches `newText`
101+
* - the replaced text is a subword of `newText` (i.e. only insertions are needed)
102+
*/
103+
function validateSameLineGhostText(cursorPos: Position, doc: TextDocument, range: Range, newText: string): InlineSuggestionEdit | undefined {
104+
const replacedText = doc.getText(range);
105+
const cursorOffsetInReplacedText = cursorPos.character - range.start.character;
106+
if (cursorOffsetInReplacedText < 0) {
107+
return undefined;
108+
}
109+
if (replacedText.substring(0, cursorOffsetInReplacedText) !== newText.substring(0, cursorOffsetInReplacedText)) {
110+
return undefined;
111+
}
77112
if (!isSubword(replacedText, newText)) {
78113
return undefined;
79114
}
80-
81115
return { range, newText };
82116
}
117+
83118
/**
84119
* a is subword of b if a can be obtained by removing characters from b
85120
*/
86-
87121
export function isSubword(a: string, b: string): boolean {
88122
for (let aIdx = 0, bIdx = 0; aIdx < a.length; bIdx++) {
89123
if (bIdx >= b.length) {

0 commit comments

Comments
 (0)