diff --git a/apps/vscode/CHANGELOG.md b/apps/vscode/CHANGELOG.md index 24eb7e93..df388ea4 100644 --- a/apps/vscode/CHANGELOG.md +++ b/apps/vscode/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.132.0 (Unreleased) +- Fixed a bug where formatting a code cell stripped leading empty lines. Leading empty lines between option directives and code are now preserved, and two or more leading empty lines are collapsed to one (). - Added clickable document links for file paths in `_quarto.yml` files. File paths are now clickable and navigate directly to the referenced file (). - Added filepath autocompletion in `_quarto.yml` files. When editing YAML values, the extension now suggests project files as you type (). - In an empty document, Positron's active runtime is now used to choose the language for a new code cell (). diff --git a/apps/vscode/src/providers/format.ts b/apps/vscode/src/providers/format.ts index 0c48e259..0c55d8fa 100644 --- a/apps/vscode/src/providers/format.ts +++ b/apps/vscode/src/providers/format.ts @@ -15,6 +15,7 @@ import { commands, + EndOfLine, FormattingOptions, Position, Range, @@ -311,14 +312,51 @@ async function formatBlock( // Create virtual document containing only the code portion of the block // so the formatter never sees the option directives. - const codeLines = blockLines.slice(optionLines); + // + // Leading empty lines are also hidden: formatters like Air (R) strip empty + // lines at position 0 of a file, which would delete them from the cell. + // We track the count so formatter edits can be shifted past them and any + // excess collapsed to one (see normalizeEdit below). + let leadingEmptyLines = 0; + for (let i = optionLines; i < blockLines.length; i++) { + if (blockLines[i].trim() === "") { + leadingEmptyLines++; + } else { + break; + } + } + const codeLines = blockLines.slice(optionLines + leadingEmptyLines); + + const blockRange = new Range( + new Position(block.range.start.line, block.range.start.character), + new Position(block.range.end.line, block.range.end.character) + ); + + // Collapsing multiple leading empty lines to one is a Quarto-level + // formatting operation: it fires even when no language formatter is active, + // so we build this edit before the early-returns below. + // Use the document's line ending to avoid introducing mixed EOL in CRLF files. + const eol = doc.eol === EndOfLine.CRLF ? "\r\n" : "\n"; + const normalizeEdit: TextEdit | undefined = leadingEmptyLines > 1 + ? new TextEdit( + new Range( + new Position(block.range.start.line + 1 + optionLines, 0), + new Position(block.range.start.line + 1 + optionLines + leadingEmptyLines, 0) + ), + eol + ) + : undefined; // Nothing to format if the block is entirely option directives (or only // trailing whitespace after them, which `lines()` may produce from a - // final newline in `token.data`). + // final newline in `token.data`). Still apply normalizeEdit if present. if (codeLines.every(l => l.trim() === "")) { - return undefined; + if (normalizeEdit && !blockRange.contains(normalizeEdit.range)) { + return undefined; + } + return normalizeEdit ? [normalizeEdit] : undefined; } + const vdoc = virtualDocForCode(codeLines, language); const edits = await executeFormatDocumentProvider( @@ -330,19 +368,18 @@ async function formatBlock( if (!edits || edits.length === 0) { // Either no formatter picked us up, or there were no edits required. // We can't determine the difference though! - return undefined; + if (normalizeEdit && !blockRange.contains(normalizeEdit.range)) { + return undefined; + } + return normalizeEdit ? [normalizeEdit] : undefined; } // 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. The `+ 1` skips the opening fence line and + // cell in the original file. The `+ 1` skips the opening fence line, // `+ optionLines` skips the leading option directives we hid from the - // formatter. - const lineOffset = block.range.start.line + 1 + optionLines; - const blockRange = new Range( - new Position(block.range.start.line, block.range.start.character), - new Position(block.range.end.line, block.range.end.character) - ); + // formatter, and `+ leadingEmptyLines` skips the leading empty lines. + const lineOffset = block.range.start.line + 1 + optionLines + leadingEmptyLines; const adjustedEdits = edits.map(edit => { const range = new Range( new Position(edit.range.start.line + lineOffset, edit.range.start.character), @@ -351,6 +388,12 @@ async function formatBlock( return new TextEdit(range, edit.newText); }); + // Include normalizeEdit in the guard so it is validated along with formatter + // edits — all edits must be in range or none are applied. + if (normalizeEdit) { + adjustedEdits.push(normalizeEdit); + } + // Bail if any edit is out of range. We used to filter these edits out but // this could bork the cell. Return `[]` to indicate that we tried. if (adjustedEdits.some(edit => !blockRange.contains(edit.range))) { diff --git a/apps/vscode/src/test/examples/format-python-leading-empty-lines.qmd b/apps/vscode/src/test/examples/format-python-leading-empty-lines.qmd new file mode 100644 index 00000000..4ae072f6 --- /dev/null +++ b/apps/vscode/src/test/examples/format-python-leading-empty-lines.qmd @@ -0,0 +1,17 @@ +--- +title: Leading Empty Lines in Python Code Cells +format: html +--- + +```{python} +#| label: one-empty-line + +x=1;y=2 +``` + +```{python} +#| label: two-empty-lines + + +x=3;y=4 +``` diff --git a/apps/vscode/src/test/examples/format-r-leading-empty-lines.qmd b/apps/vscode/src/test/examples/format-r-leading-empty-lines.qmd new file mode 100644 index 00000000..0233ca22 --- /dev/null +++ b/apps/vscode/src/test/examples/format-r-leading-empty-lines.qmd @@ -0,0 +1,22 @@ +--- +title: Leading Empty Lines in R Code Cells +format: html +--- + +```{r} +#| label: one-empty-line + +x<-1 +``` + +```{r} +#| label: two-empty-lines + + +x<-2 +``` + +```{r} +#| label: no-empty-lines +x<-3 +``` diff --git a/apps/vscode/src/test/formatting.test.ts b/apps/vscode/src/test/formatting.test.ts index 26b4e6b3..2496725e 100644 --- a/apps/vscode/src/test/formatting.test.ts +++ b/apps/vscode/src/test/formatting.test.ts @@ -163,6 +163,19 @@ function mangleHashPipeLines(sourceText: string): string { ); } +function rAssignmentFormatter(sourceText: string): string { + return sourceText.replace(/(\w)<-(\w)/g, "$1 <- $2"); +} + +/** + * Hostile formatter that mangles any leading newline in the virtual document. + * If leading empty lines leak into the virtual document, this formatter will + * corrupt the cell with a detectable marker. + */ +function leadingNewlineMangler(sourceText: string): string { + return sourceText.replace(/^\n/, "LEAKED_EMPTY_LINE\n"); +} + /** * Hostile R formatter that rewrites `#|` directives and normalises the * assignment arrow. @@ -512,6 +525,183 @@ suite("Code Block Formatting", function () { } }); + test("single leading empty line in R cell is preserved after formatting", async function () { + const formattedResult = await testFormatter( + "format-r-leading-empty-lines.qmd", + [8, 0], + rAssignmentFormatter, + "r" + ); + + assert.ok( + formattedResult.includes("#| label: one-empty-line"), + "Option directive should be preserved" + ); + assert.ok( + formattedResult.includes("x <- 1"), + "Code should be reformatted" + ); + assert.ok( + /one-empty-line\n\n/.test(formattedResult), + "Single leading empty line should be preserved" + ); + assert.ok( + !/one-empty-line\n\n\n/.test(formattedResult), + "No extra empty line should be introduced" + ); + }); + + test("multiple leading empty lines in R cell are collapsed to one after formatting", async function () { + const formattedResult = await testFormatter( + "format-r-leading-empty-lines.qmd", + [13, 0], + rAssignmentFormatter, + "r" + ); + + assert.ok( + formattedResult.includes("#| label: two-empty-lines"), + "Option directive should be preserved" + ); + assert.ok( + formattedResult.includes("x <- 2"), + "Code should be reformatted" + ); + assert.ok( + /two-empty-lines\n\n/.test(formattedResult), + "Exactly one leading empty line should remain" + ); + assert.ok( + !/two-empty-lines\n\n\n/.test(formattedResult), + "Second leading empty line should be collapsed" + ); + }); + + test("no leading empty lines in R cell — unaffected by the normalisation", async function () { + const formattedResult = await testFormatter( + "format-r-leading-empty-lines.qmd", + [20, 0], + rAssignmentFormatter, + "r" + ); + + assert.ok( + formattedResult.includes("#| label: no-empty-lines"), + "Option directive should be preserved" + ); + assert.ok( + formattedResult.includes("x <- 3"), + "Code should be reformatted" + ); + assert.ok( + /no-empty-lines\nx <- 3/.test(formattedResult), + "No empty line should be introduced" + ); + }); + + test("leading empty lines in Python cell are preserved after formatting", async function () { + const formattedResult = await testFormatter( + "format-python-leading-empty-lines.qmd", + [8, 0], + spaceAssignments + ); + + assert.ok( + formattedResult.includes("#| label: one-empty-line"), + "Option directive should be preserved" + ); + assert.ok( + formattedResult.includes("x = 1"), + "Code should be reformatted" + ); + assert.ok( + /one-empty-line\n\n/.test(formattedResult), + "Single leading empty line should be preserved" + ); + assert.ok( + !/one-empty-line\n\n\n/.test(formattedResult), + "No extra empty line should be introduced" + ); + }); + + test("multiple leading empty lines in Python cell are collapsed to one after formatting", async function () { + const formattedResult = await testFormatter( + "format-python-leading-empty-lines.qmd", + [13, 0], + spaceAssignments + ); + + assert.ok( + formattedResult.includes("#| label: two-empty-lines"), + "Option directive should be preserved" + ); + assert.ok( + formattedResult.includes("x = 3"), + "Code should be reformatted" + ); + assert.ok( + /two-empty-lines\n\n/.test(formattedResult), + "Exactly one leading empty line should remain" + ); + assert.ok( + !/two-empty-lines\n\n\n/.test(formattedResult), + "Second leading empty line should be collapsed" + ); + }); + + test("leading empty lines are hidden from the formatter", async function () { + // Target the two-empty-lines cell: without stripping, the virtual doc + // would start with "\n\nx<-2" and the mangler would inject LEAKED_EMPTY_LINE. + // With stripping, the virtual doc starts with "x<-2" and the mangler is silent. + const formattedResult = await testFormatter( + "format-r-leading-empty-lines.qmd", + [13, 0], + leadingNewlineMangler, + "r" + ); + + assert.ok( + !formattedResult.includes("LEAKED_EMPTY_LINE"), + "Leading empty lines must not be visible to the formatter" + ); + assert.ok( + /two-empty-lines\n\n/.test(formattedResult), + "Exactly one leading empty line should remain in the cell after formatting" + ); + assert.ok( + !/two-empty-lines\n\n\n/.test(formattedResult), + "Second leading empty line should be collapsed" + ); + }); + + test("multiple leading empty lines are collapsed without a language formatter", async function () { + const { doc } = await openAndShowExamplesTextDocument( + "format-r-leading-empty-lines.qmd" + ); + + try { + // No formatter is registered for "r" — only the Quarto-level + // normalisation edit should fire. + setCursorPosition(13, 0); + await wait(450); + await vscode.commands.executeCommand("quarto.formatCell"); + await wait(450); + + const result = doc.getText(); + + assert.ok( + /two-empty-lines\n\n/.test(result), + "Exactly one leading empty line should remain" + ); + assert.ok( + !/two-empty-lines\n\n\n/.test(result), + "Second leading empty line should be collapsed even without a formatter" + ); + } finally { + await vscode.commands.executeCommand("workbench.action.closeActiveEditor"); + } + }); + test("formatter returning multiple discrete edits is applied correctly", async function () { const { doc } = await openAndShowExamplesTextDocument( "format-python-multiple-options.qmd"