From 15d9762eb4b371da2d3c7db155c700a024a11c39 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 4 Jul 2025 10:55:56 +0200 Subject: [PATCH 1/6] Sort text edits by descending start position --- apps/vscode/src/providers/format.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/apps/vscode/src/providers/format.ts b/apps/vscode/src/providers/format.ts index 92c078e5..263340d3 100644 --- a/apps/vscode/src/providers/format.ts +++ b/apps/vscode/src/providers/format.ts @@ -138,9 +138,13 @@ class FormatCellCommand implements Command { const edits = await formatActiveCell(editor, this.engine_); if (edits) { editor.edit((editBuilder) => { - edits.forEach((edit) => { - editBuilder.replace(edit.range, edit.newText); - }); + // Sort edits by descending start position to avoid range shifting issues + edits + .slice() + .sort((a, b) => b.range.start.compareTo(a.range.start)) + .forEach((edit) => { + editBuilder.replace(edit.range, edit.newText); + }); }); } else { window.showInformationMessage( @@ -204,15 +208,21 @@ async function formatActiveCell(editor: TextEditor, engine: MarkdownEngine) { } async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock, language: EmbeddedLanguage) { + // Create virtual document containing the block const blockLines = lines(codeForExecutableLanguageBlock(block)); blockLines.push(""); const vdoc = virtualDocForCode(blockLines, language); + const edits = await executeFormatDocumentProvider( vdoc, doc, formattingOptions(doc.uri, vdoc.language) ); + if (edits) { + // Because we format with the block code copied in an empty virtual + // document, we need to adjust the ranges to match the edits to the block + // cell in the original file. const blockRange = new Range( new Position(block.range.start.line, block.range.start.character), new Position(block.range.end.line, block.range.end.character) From d192ab530dfe25f5f4a80828f7d331d1aa3f84e7 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 4 Jul 2025 12:24:26 +0200 Subject: [PATCH 2/6] Don't append lines to block code before formatting --- apps/vscode/src/providers/format.ts | 3 +-- packages/quarto-core/src/markdown/language.ts | 7 +++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/vscode/src/providers/format.ts b/apps/vscode/src/providers/format.ts index 263340d3..51e53170 100644 --- a/apps/vscode/src/providers/format.ts +++ b/apps/vscode/src/providers/format.ts @@ -209,8 +209,7 @@ async function formatActiveCell(editor: TextEditor, engine: MarkdownEngine) { async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock, language: EmbeddedLanguage) { // Create virtual document containing the block - const blockLines = lines(codeForExecutableLanguageBlock(block)); - blockLines.push(""); + const blockLines = lines(codeForExecutableLanguageBlock(block, false)); const vdoc = virtualDocForCode(blockLines, language); const edits = await executeFormatDocumentProvider( diff --git a/packages/quarto-core/src/markdown/language.ts b/packages/quarto-core/src/markdown/language.ts index ea3f88fe..201fc259 100644 --- a/packages/quarto-core/src/markdown/language.ts +++ b/packages/quarto-core/src/markdown/language.ts @@ -38,11 +38,14 @@ export function isExecutableLanguageBlock(token: Token) : token is TokenMath | T } } -export function codeForExecutableLanguageBlock(token: TokenMath | TokenCodeBlock) { +export function codeForExecutableLanguageBlock( + token: TokenMath | TokenCodeBlock, + appendNewline = true, +) { if (isMath(token)) { return token.data.text; } else if (isCodeBlock(token)) { - return token.data + "\n"; + return token.data + (appendNewline ? "\n" : ""); } else { return ""; } From 200b53a3f2d73dbc8e637c405af25a65ff9ec033 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 4 Jul 2025 12:33:37 +0200 Subject: [PATCH 3/6] Don't apply partial edits --- apps/vscode/src/providers/format.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/vscode/src/providers/format.ts b/apps/vscode/src/providers/format.ts index 51e53170..c6a5eb84 100644 --- a/apps/vscode/src/providers/format.ts +++ b/apps/vscode/src/providers/format.ts @@ -233,8 +233,17 @@ async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock, new Position(edit.range.end.line + block.range.start.line + 1, edit.range.end.character) ); return new TextEdit(range, edit.newText); - }) - .filter(edit => blockRange.contains(edit.range)); + }); + + // Bail if any edit is out of range. We used to filter these edits out but + // this could bork the cell. + if (edits.some(edit => !blockRange.contains(edit.range))) { + window.showInformationMessage( + "Formatting edits were out of range and could not be applied to the code cell." + ); + return []; + } + return adjustedEdits; } } From 84757b286ca76fd5db663eb6308bad8d150bb067 Mon Sep 17 00:00:00 2001 From: elliot Date: Fri, 5 Sep 2025 14:56:36 -0400 Subject: [PATCH 4/6] WIP add formatting test --- apps/vscode/src/providers/format.ts | 11 +----- apps/vscode/src/test/examples/cell-format.qmd | 11 ++++++ apps/vscode/src/test/quartoDoc.test.ts | 36 ++++++++++++++++--- 3 files changed, 43 insertions(+), 15 deletions(-) create mode 100644 apps/vscode/src/test/examples/cell-format.qmd diff --git a/apps/vscode/src/providers/format.ts b/apps/vscode/src/providers/format.ts index c6a5eb84..36466b26 100644 --- a/apps/vscode/src/providers/format.ts +++ b/apps/vscode/src/providers/format.ts @@ -233,16 +233,7 @@ async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock, new Position(edit.range.end.line + block.range.start.line + 1, edit.range.end.character) ); return new TextEdit(range, edit.newText); - }); - - // Bail if any edit is out of range. We used to filter these edits out but - // this could bork the cell. - if (edits.some(edit => !blockRange.contains(edit.range))) { - window.showInformationMessage( - "Formatting edits were out of range and could not be applied to the code cell." - ); - return []; - } + }).filter(edit => blockRange.contains(edit.range)); return adjustedEdits; } diff --git a/apps/vscode/src/test/examples/cell-format.qmd b/apps/vscode/src/test/examples/cell-format.qmd new file mode 100644 index 00000000..c5b1ae30 --- /dev/null +++ b/apps/vscode/src/test/examples/cell-format.qmd @@ -0,0 +1,11 @@ +See https://github.com/quarto-dev/quarto/issues/745 "Reformating R cell in VSCODE with air does not correctly close chunk." + +```{r} +list(foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar) +``` + +```{r} + +list(foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar, foobar) + +``` diff --git a/apps/vscode/src/test/quartoDoc.test.ts b/apps/vscode/src/test/quartoDoc.test.ts index aeb88526..7e47c997 100644 --- a/apps/vscode/src/test/quartoDoc.test.ts +++ b/apps/vscode/src/test/quartoDoc.test.ts @@ -30,17 +30,43 @@ suite("Quarto basics", function () { assert.equal(before, after); }); - roundtripSnapshotTest('valid-basics.qmd'); + // roundtripSnapshotTest('valid-basics.qmd'); - roundtripSnapshotTest('valid-basics-2.qmd'); + // roundtripSnapshotTest('valid-basics-2.qmd'); - roundtripSnapshotTest('valid-nesting.qmd'); + // roundtripSnapshotTest('valid-nesting.qmd'); - roundtripSnapshotTest('invalid.qmd'); + // roundtripSnapshotTest('invalid.qmd'); - roundtripSnapshotTest('capsule-leak.qmd'); + // roundtripSnapshotTest('capsule-leak.qmd'); + + test("cell formamtmtamt", async function () { + const { doc } = await openAndShowTextDocument("cell-format.qmd"); + + //await vscode.commands.executeCommand("quarto.formatCell"); + setCursorPosition(3, 1); + await vscode.commands.executeCommand("quarto.formatCell"); + await wait(6300); + // const { before, after } = await roundtrip(doc); + + // assert.equal(before, after); + }); + + suiteTeardown(() => { + vscode.window.showInformationMessage('All tests done!'); + }); }); +function setCursorPosition(line: number, character: number) { + const editor = vscode.window.activeTextEditor; + if (editor) { + const position = new vscode.Position(line, character); + const newSelection = new vscode.Selection(position, position); + editor.selection = newSelection; + editor.revealRange(newSelection, vscode.TextEditorRevealType.InCenter); // Optional: scroll to the new position + } +} + /** * * When the test is run on the dev's machine for the first time, saves the roundtripped file as a snapshot. From f7fc2fbe0695571a7c41c97e48ca66c3b49131b5 Mon Sep 17 00:00:00 2001 From: elliot Date: Wed, 10 Sep 2025 16:17:50 -0400 Subject: [PATCH 5/6] Add test with custom formatter registered --- apps/vscode/src/test/quartoDoc.test.ts | 29 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/apps/vscode/src/test/quartoDoc.test.ts b/apps/vscode/src/test/quartoDoc.test.ts index 7e47c997..980489e6 100644 --- a/apps/vscode/src/test/quartoDoc.test.ts +++ b/apps/vscode/src/test/quartoDoc.test.ts @@ -22,13 +22,13 @@ suite("Quarto basics", function () { // Note: this test runs after the previous test, so `hello.qmd` can already be touched by the previous // test. That's okay for this test, but could cause issues if you expect a qmd to look how it // does in `/examples`. - test("Roundtrip doesn't change hello.qmd", async function () { - const { doc } = await openAndShowTextDocument("hello.qmd"); + // test("Roundtrip doesn't change hello.qmd", async function () { + // const { doc } = await openAndShowTextDocument("hello.qmd"); - const { before, after } = await roundtrip(doc); + // const { before, after } = await roundtrip(doc); - assert.equal(before, after); - }); + // assert.equal(before, after); + // }); // roundtripSnapshotTest('valid-basics.qmd'); @@ -43,8 +43,25 @@ suite("Quarto basics", function () { test("cell formamtmtamt", async function () { const { doc } = await openAndShowTextDocument("cell-format.qmd"); - //await vscode.commands.executeCommand("quarto.formatCell"); + vscode.languages.registerDocumentFormattingEditProvider( + { scheme: 'file', language: 'r' }, + { + provideDocumentFormattingEdits(document: vscode.TextDocument): + vscode.ProviderResult { + const sourceText = document.getText(); + + const fileStart = new vscode.Position(0, 0); + const fileEnd = document.lineAt(document.lineCount - 1).range.end; + + const formattedSourceText = sourceText + 'hello!' + + return [new vscode.TextEdit(new vscode.Range(fileStart, fileEnd), formattedSourceText)]; + } + } + ) + setCursorPosition(3, 1); + await wait(300); await vscode.commands.executeCommand("quarto.formatCell"); await wait(6300); // const { before, after } = await roundtrip(doc); From 874a7533790e18f867a4d9c6d2d902dabff56451 Mon Sep 17 00:00:00 2001 From: elliot Date: Thu, 25 Sep 2025 16:06:25 -0400 Subject: [PATCH 6/6] clean up test --- apps/vscode/src/test/quartoDoc.test.ts | 59 +++++++++++++++++--------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/apps/vscode/src/test/quartoDoc.test.ts b/apps/vscode/src/test/quartoDoc.test.ts index 980489e6..0515b298 100644 --- a/apps/vscode/src/test/quartoDoc.test.ts +++ b/apps/vscode/src/test/quartoDoc.test.ts @@ -41,32 +41,40 @@ suite("Quarto basics", function () { // roundtripSnapshotTest('capsule-leak.qmd'); test("cell formamtmtamt", async function () { - const { doc } = await openAndShowTextDocument("cell-format.qmd"); - vscode.languages.registerDocumentFormattingEditProvider( - { scheme: 'file', language: 'r' }, - { - provideDocumentFormattingEdits(document: vscode.TextDocument): - vscode.ProviderResult { - const sourceText = document.getText(); + async function testFormatter(filename: string, [line, character]: [number, number], format: (sourceText: string) => string) { + const { doc } = await openAndShowTextDocument(filename); - const fileStart = new vscode.Position(0, 0); - const fileEnd = document.lineAt(document.lineCount - 1).range.end; + const formattingEditProvider = vscode.languages.registerDocumentFormattingEditProvider( + { scheme: 'file', language: 'r' }, + createFormatterFromStringFunc(format) + ) - const formattedSourceText = sourceText + 'hello!' + setCursorPosition(line, character); + await wait(300); + await vscode.commands.executeCommand("quarto.formatCell"); + await wait(300) - return [new vscode.TextEdit(new vscode.Range(fileStart, fileEnd), formattedSourceText)]; - } - } - ) + const result = doc.getText() + + formattingEditProvider.dispose() + await vscode.commands.executeCommand("workbench.action.closeActiveEditor"); + + return result + } - setCursorPosition(3, 1); - await wait(300); - await vscode.commands.executeCommand("quarto.formatCell"); - await wait(6300); - // const { before, after } = await roundtrip(doc); + const newlineFormatterResult = await testFormatter( + "cell-format.qmd", + [3, 1], + (sourceText) => sourceText + '\n' + ) + const noopFormatterResult = await testFormatter( + "cell-format.qmd", + [3, 1], + (sourceText) => sourceText + ) - // assert.equal(before, after); + assert.equal(newlineFormatterResult, noopFormatterResult) }); suiteTeardown(() => { @@ -74,6 +82,17 @@ suite("Quarto basics", function () { }); }); +function createFormatterFromStringFunc(format: (sourceText: string) => string) { + return { + provideDocumentFormattingEdits(document: vscode.TextDocument): vscode.ProviderResult { + const fileStart = new vscode.Position(0, 0); + const fileEnd = document.lineAt(document.lineCount - 1).range.end; + + return [new vscode.TextEdit(new vscode.Range(fileStart, fileEnd), format(document.getText()))]; + } + } +} + function setCursorPosition(line: number, character: number) { const editor = vscode.window.activeTextEditor; if (editor) {