Skip to content

Commit 53a3b1c

Browse files
Copilotntotten
andauthored
Fix: Skip TextEdit when document already formatted (#3864)
* Initial plan * Fix: Return empty edits when document is already formatted This change prevents returning unnecessary TextEdits when the formatted output is identical to the source document. This could potentially fix issues where empty lines appear to be added to files during formatting. The minimalEdit function now: 1. Checks if the document text is identical to the formatted result 2. Returns null if no changes are needed 3. The provideEdits function returns an empty array when no edit is needed This ensures VS Code doesn't apply empty TextEdits which could cause unexpected behavior with line endings or cursor positioning. Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com> * Add logging for when document is already formatted Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com> * Address code review feedback - Fix countTrailingNewlines to use continue instead of break for \r - Clarify condition in forceFormatDocument to check > 1 instead of !== 1 Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com> * Update warning message to reflect that 0 edits is valid Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com> * Update CHANGELOG.md with fix for unnecessary TextEdits Added changelog entry documenting the fix for issue #3232 where unnecessary TextEdits were applied when document was already formatted. Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com> * Fix test to format via extension twice for idempotency Updated the markdown test to call format() twice instead of calling format() once and prettier.format() once. This better reproduces the original bug where formatting via the extension multiple times would add spurious empty lines. Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com> Co-authored-by: Nathan Totten <nate@zuplo.com>
1 parent 09f61c5 commit 53a3b1c

4 files changed

Lines changed: 51 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ All notable changes to the "prettier-vscode" extension will be documented in thi
1414
- Fixed parser detection fallback when using plugins with Prettier v3
1515
- Added new Prettier v3 options: `objectWrap`, `experimentalOperatorPosition`
1616
- Added support for TypeScript config files (`.prettierrc.ts`, `.prettierrc.cts`, `.prettierrc.mts`, `prettier.config.ts`, etc.) introduced in Prettier 3.5.0 - Thanks to [@dr2009](https://github.com/dr2009)
17+
- Fixed issue where unnecessary TextEdits were applied when document was already formatted, which could cause spurious changes or cursor positioning issues (#3232)
1718

1819
## [11.0.0]
1920

src/PrettierEditService.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,12 @@ export default class PrettierEditService implements Disposable {
202202
);
203203

204204
const edits = await this.provideEdits(editor.document, { force: true });
205-
if (edits.length !== 1) {
205+
if (edits.length === 0) {
206+
this.loggingService.logInfo("Document is already formatted.");
207+
return;
208+
}
209+
if (edits.length > 1) {
210+
this.loggingService.logWarning(`Unexpected multiple edits (${edits.length}), expected 0 or 1`);
206211
return;
207212
}
208213

@@ -446,11 +451,22 @@ export default class PrettierEditService implements Disposable {
446451
const duration = new Date().getTime() - startTime;
447452
this.loggingService.logInfo(`Formatting completed in ${duration}ms.`);
448453
const edit = this.minimalEdit(document, result);
454+
if (!edit) {
455+
// Document is already formatted, no changes needed
456+
this.loggingService.logDebug("Document is already formatted, no changes needed.");
457+
return [];
458+
}
449459
return [edit];
450460
};
451461

452-
private minimalEdit(document: TextDocument, string1: string) {
462+
private minimalEdit(document: TextDocument, string1: string): TextEdit | null {
453463
const string0 = document.getText();
464+
465+
// Quick check: if strings are identical, no edit needed
466+
if (string0 === string1) {
467+
return null;
468+
}
469+
454470
// length of common prefix
455471
let i = 0;
456472
while (

src/test/suite/format.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,33 @@ describe("Test format Document", () => {
5151
formatSameAsPrettier("formatTest/htmlWithLiterals.html"));
5252
it("formats Vue", () => formatSameAsPrettier("formatTest/ugly.vue"));
5353
it("formats HBS", () => formatSameAsPrettier("formatTest/ugly.hbs"));
54+
it("formats Markdown", () => formatSameAsPrettier("formatTest/ugly.md"));
55+
56+
it("formats Markdown without adding extra empty lines", async () => {
57+
// This test checks for the issue where formatting MD files multiple times
58+
// would add empty lines at the end (up to 2 empty lines)
59+
const { actual: firstFormat } = await format("project", "formatTest/ugly.md");
60+
61+
// Count trailing newlines
62+
const countTrailingNewlines = (str: string) => {
63+
let count = 0;
64+
for (let i = str.length - 1; i >= 0; i--) {
65+
if (str[i] === '\n') count++;
66+
else if (str[i] === '\r') continue; // Skip \r in CRLF
67+
else break;
68+
}
69+
return count;
70+
};
71+
72+
const firstCount = countTrailingNewlines(firstFormat);
73+
74+
// Format a second time via the extension to verify idempotency
75+
const { actual: secondFormat } = await format("project", "formatTest/ugly.md");
76+
const secondCount = countTrailingNewlines(secondFormat);
77+
78+
// Should have exactly 1 trailing newline after both formats
79+
assert.equal(firstCount, 1, "First format should have exactly 1 trailing newline");
80+
assert.equal(secondCount, 1, "Second format should have exactly 1 trailing newline");
81+
assert.equal(firstFormat, secondFormat, "Formatting via extension should be idempotent");
82+
});
5483
});
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Hello
2+
3+
This is a test.

0 commit comments

Comments
 (0)