Skip to content

Commit df2a441

Browse files
ulugbeknaCopilot
andcommitted
nes: fix: do not insert spurious trailing newline after suggestion (#311441)
* nes: show how insertion at cursor leaves an unnecessary newline after the suggestion Co-authored-by: Copilot <copilot@github.com> * nes: fix: do not insert spurious trailing newline after suggestion Co-authored-by: Copilot <copilot@github.com> * account for CRLF Co-authored-by: Copilot <copilot@github.com> --------- Co-authored-by: Copilot <copilot@github.com>
1 parent fb8b7a3 commit df2a441

File tree

2 files changed

+109
-6
lines changed

2 files changed

+109
-6
lines changed

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

Lines changed: 102 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,9 @@ suite('toInlineSuggestion', () => {
101101
assert.isDefined(result);
102102
// Range is an empty range at the cursor for a pure insertion
103103
assert.deepStrictEqual(result!.range, new Range(1, 15, 1, 15));
104-
// Text is prepended with the newline between cursor and original range
105-
assert.strictEqual(result!.newText, '\n' + replaceText);
104+
// Text is prepended with the newline between cursor and original range,
105+
// and the trailing newline is dropped so we don't introduce a blank line.
106+
assert.strictEqual(result!.newText, '\n' + replaceText.replace(/\r?\n$/, ''));
106107
});
107108

108109
test('should not use ghost text when inserting on next line when none empty', () => {
@@ -149,7 +150,8 @@ suite('toInlineSuggestion', () => {
149150
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
150151
assert.isDefined(result);
151152
assert.deepStrictEqual(result!.range, new Range(0, 13, 0, 13));
152-
assert.strictEqual(result!.newText, '\n' + replaceText);
153+
// Trailing '\n' is dropped to avoid a spurious blank line.
154+
assert.strictEqual(result!.newText, '\n' + replaceText.replace(/\r?\n$/, ''));
153155
});
154156

155157
test('multi-line insertion without trailing newline rejected when target line has content', () => {
@@ -318,7 +320,8 @@ function createDocumentSymbol(
318320
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
319321
assert.isDefined(result);
320322
assert.deepStrictEqual(result!.range, new Range(0, 6, 0, 6));
321-
assert.strictEqual(result!.newText, '\n\n');
323+
// Trailing '\n' is dropped — only the prepended newline remains.
324+
assert.strictEqual(result!.newText, '\n');
322325
});
323326

324327
test('next-line: cursor at end of an empty line', () => {
@@ -330,7 +333,8 @@ function createDocumentSymbol(
330333
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
331334
assert.isDefined(result);
332335
assert.deepStrictEqual(result!.range, new Range(0, 0, 0, 0));
333-
assert.strictEqual(result!.newText, '\nnew line\n');
336+
// Trailing '\n' is dropped to avoid a spurious blank line.
337+
assert.strictEqual(result!.newText, '\nnew line');
334338
});
335339

336340
test('next-line: range on line before cursor is rejected', () => {
@@ -547,4 +551,97 @@ function createDocumentSymbol(
547551
assert.deepStrictEqual(result!.range, new Range(1, 0, 1, 0));
548552
assert.strictEqual(result!.newText, '');
549553
});
554+
555+
test('insertion on next line in fieldLabels object', () => {
556+
const doc = `import React, { useState } from "react";
557+
558+
interface FormData {
559+
firstName: string;
560+
lastName: string;
561+
password: string;
562+
email: string;
563+
age: string;
564+
city: string;
565+
}
566+
567+
const initialFormData: FormData = {
568+
firstName: "",
569+
lastName: "",
570+
password: "",
571+
email: "",
572+
age: "",
573+
city: "",
574+
};
575+
576+
const fieldLabels: Record<keyof FormData, string> = {
577+
firstName: "First Name",
578+
lastName: "Last Name",
579+
email: "Email Address",
580+
age: "Age",
581+
city: "City",
582+
};
583+
`;
584+
const document = createTextDocumentData(Uri.from({ scheme: 'test', path: '/test/file.tsx' }), doc, 'typescriptreact').document;
585+
const cursorPosition = new Position(22, 26); // end of ` lastName: "Last Name",`
586+
const replaceRange = new Range(23, 0, 23, 0);
587+
const replaceText = ' password: "Password",\n';
588+
589+
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText, true);
590+
assert.isDefined(result);
591+
assert.deepStrictEqual(result!.range, new Range(22, 26, 22, 26));
592+
// Trailing '\n' is dropped because the original line terminator after
593+
// the cursor is preserved.
594+
assert.strictEqual(result!.newText, '\n password: "Password",');
595+
});
596+
597+
suite('CRLF', () => {
598+
599+
function createCRLFDocument(lines: string[], languageId: string = 'typescript') {
600+
return createTextDocumentData(
601+
Uri.from({ scheme: 'test', path: '/test/file.ts' }),
602+
lines.join('\r\n'),
603+
languageId,
604+
'\r\n',
605+
).document;
606+
}
607+
608+
test('next-line insertion: trailing CRLF is dropped (no dangling \\r)', () => {
609+
const document = createCRLFDocument(['function foo(', '', 'other']);
610+
const cursorPosition = new Position(0, 13); // end of "function foo("
611+
const replaceRange = new Range(1, 0, 1, 0); // empty line
612+
const replaceText = ' a: string,\r\n b: number\r\n';
613+
614+
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
615+
assert.isDefined(result);
616+
assert.deepStrictEqual(result!.range, new Range(0, 13, 0, 13));
617+
// The trailing CRLF must be stripped entirely; no dangling '\r'
618+
// should leak into the suggestion text.
619+
assert.strictEqual(result!.newText, '\r\n a: string,\r\n b: number');
620+
});
621+
622+
test('next-line insertion: trailing CRLF on non-empty target line', () => {
623+
const document = createCRLFDocument(['function foo(', ')', 'other']);
624+
const cursorPosition = new Position(0, 13);
625+
const replaceRange = new Range(1, 0, 1, 0);
626+
const replaceText = ' a: string,\r\n b: number\r\n';
627+
628+
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
629+
assert.isDefined(result);
630+
assert.deepStrictEqual(result!.range, new Range(0, 13, 0, 13));
631+
assert.strictEqual(result!.newText, '\r\n a: string,\r\n b: number');
632+
});
633+
634+
test('next-line insertion: CRLF-only newText is fully stripped', () => {
635+
const document = createCRLFDocument(['line 0', '', 'line 2']);
636+
const cursorPosition = new Position(0, 6);
637+
const replaceRange = new Range(1, 0, 1, 0);
638+
const replaceText = '\r\n';
639+
640+
const result = toInlineSuggestion(cursorPosition, document, replaceRange, replaceText);
641+
assert.isDefined(result);
642+
assert.deepStrictEqual(result!.range, new Range(0, 6, 0, 6));
643+
// Only the prepended CRLF between cursor and original range remains.
644+
assert.strictEqual(result!.newText, '\r\n');
645+
});
646+
});
550647
});

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,13 @@ export function toInlineSuggestion(cursorPos: Position, doc: TextDocument, range
2626
// Use an empty range at the cursor so the suggestion is a pure insertion
2727
const adjustedRange = new Range(cursorPos, cursorPos);
2828
const textBetweenCursorAndRange = doc.getText(new Range(cursorPos, range.start));
29-
return { range: adjustedRange, newText: textBetweenCursorAndRange + newText };
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 };
3036
}
3137

3238
if (advanced) {

0 commit comments

Comments
 (0)