Skip to content

Commit 0be30fc

Browse files
authored
Refactor formatting tooling and don't fire message when code cell is already formatted (#933)
* More accurately type functions * Remove message about not being in a formattable cell We can't actually tell the difference between a cell that doesn't have a formatter and a cell with a formatter that said "there is nothing to do, it's already formatted" * CHANGELOG bullet * Dramatically improve readability * Add tests
1 parent 9bbc272 commit 0be30fc

5 files changed

Lines changed: 311 additions & 115 deletions

File tree

apps/vscode/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## 1.131.0 (Unreleased)
44

5-
5+
- Fixed a bug where `Quarto: Format Cell` would notify you that no formatter was available for code cells that were already formatted (<https://github.com/quarto-dev/quarto/pull/933>).
66

77
## 1.130.0 (Release on 2026-02-18)
88

apps/vscode/src/providers/format.ts

Lines changed: 151 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,20 @@ import {
2424
workspace,
2525
CancellationToken,
2626
Uri,
27-
TextEditor,
2827
} from "vscode";
2928
import {
3029
ProvideDocumentFormattingEditsSignature,
3130
ProvideDocumentRangeFormattingEditsSignature,
3231
} from "vscode-languageclient/node";
3332
import { lines } from "core";
34-
import { TokenCodeBlock, TokenMath, codeForExecutableLanguageBlock, languageBlockAtPosition } from "quarto-core";
33+
import { TokenCodeBlock, TokenMath, codeForExecutableLanguageBlock, languageBlockAtLine } from "quarto-core";
3534

3635
import { Command } from "../core/command";
3736
import { isQuartoDoc } from "../core/doc";
3837
import { MarkdownEngine } from "../markdown/engine";
3938
import { EmbeddedLanguage, languageCanFormatDocument } from "../vdoc/languages";
4039
import {
41-
languageAtPosition,
40+
languageFromBlock,
4241
mainLanguage,
4342
unadjustedRange,
4443
VirtualDoc,
@@ -47,7 +46,6 @@ import {
4746
withVirtualDocUri,
4847
} from "../vdoc/vdoc";
4948

50-
5149
export function activateCodeFormatting(engine: MarkdownEngine) {
5250
return [new FormatCellCommand(engine)];
5351
}
@@ -59,41 +57,54 @@ export function embeddedDocumentFormattingProvider(engine: MarkdownEngine) {
5957
token: CancellationToken,
6058
next: ProvideDocumentFormattingEditsSignature
6159
): Promise<TextEdit[] | null | undefined> => {
62-
if (isQuartoDoc(document, true)) {
63-
// ensure we are dealing w/ the active document
64-
const editor = window.activeTextEditor;
65-
const activeDocument = editor?.document;
66-
if (
67-
editor &&
68-
activeDocument?.uri.toString() === document.uri.toString()
69-
) {
70-
const line = editor.selection.active.line;
71-
const position = new Position(line, 0);
72-
const tokens = engine.parse(document);
73-
let language = languageAtPosition(tokens, position);
74-
if (!language || !language.canFormat) {
75-
language = mainLanguage(tokens, (lang) => !!lang.canFormat);
76-
}
77-
if (language) {
78-
if (languageCanFormatDocument(language)) {
79-
const vdoc = virtualDocForLanguage(document, tokens, language);
80-
if (vdoc) {
81-
return executeFormatDocumentProvider(
82-
vdoc,
83-
document,
84-
formattingOptions(document.uri, vdoc.language, options)
85-
);
86-
}
87-
} else {
88-
return (await formatActiveCell(editor, engine)) || [];
89-
}
90-
}
91-
}
92-
// ensure that other formatters don't ever run over qmd files
60+
if (!isQuartoDoc(document, true)) {
61+
// Delegate if we don't handle it
62+
return next(document, options, token);
63+
}
64+
65+
// Ensure we are dealing w/ the active document
66+
const activeEditor = window.activeTextEditor;
67+
if (!activeEditor) {
68+
// Ensure that other formatters don't ever run over qmd files
69+
return [];
70+
}
71+
if (activeEditor.document.uri.toString() !== document.uri.toString()) {
72+
return [];
73+
}
74+
75+
const tokens = engine.parse(document);
76+
77+
// Figure out language to use. Try selection's block, then fall back to main doc language.
78+
const includeFence = false;
79+
const line = activeEditor.selection.active.line;
80+
const block = languageBlockAtLine(tokens, line, includeFence);
81+
82+
let language = block ? languageFromBlock(block) : undefined;
83+
84+
if (!language || !language.canFormat) {
85+
language = mainLanguage(tokens, (lang) => !!lang.canFormat);
86+
}
87+
88+
if (!language) {
89+
// No language that can format in any way
9390
return [];
91+
}
92+
93+
if (languageCanFormatDocument(language)) {
94+
// Full document formatting support
95+
const vdoc = virtualDocForLanguage(document, tokens, language);
96+
return executeFormatDocumentProvider(
97+
vdoc,
98+
document,
99+
formattingOptions(document.uri, vdoc.language, options)
100+
);
101+
} else if (block) {
102+
// Just format the selected block if there is one
103+
const edits = await formatBlock(document, block);
104+
return edits ? edits : [];
94105
} else {
95-
// delegate if we didn't handle it
96-
return next(document, options, token);
106+
// Nothing we can format
107+
return [];
97108
}
98109
};
99110
}
@@ -108,22 +119,38 @@ export function embeddedDocumentRangeFormattingProvider(
108119
token: CancellationToken,
109120
next: ProvideDocumentRangeFormattingEditsSignature
110121
): Promise<TextEdit[] | null | undefined> => {
111-
if (isQuartoDoc(document, true)) {
112-
const tokens = engine.parse(document);
113-
const beginBlock = languageBlockAtPosition(tokens, range.start, false);
114-
const endBlock = languageBlockAtPosition(tokens, range.end, false);
115-
if (beginBlock && (beginBlock.range.start.line === endBlock?.range.start.line)) {
116-
const editor = window.activeTextEditor;
117-
if (editor?.document?.uri.toString() === document.uri.toString()) {
118-
return await formatActiveCell(editor, engine);
119-
}
120-
}
121-
// ensure that other formatters don't ever run over qmd files
122-
return [];
123-
} else {
124-
// if we don't perform any formatting, then call the next handler
122+
if (!isQuartoDoc(document, true)) {
123+
// If we don't perform any formatting, then call the next handler
125124
return next(document, range, options, token);
126125
}
126+
127+
const includeFence = false;
128+
const tokens = engine.parse(document);
129+
130+
const block = languageBlockAtLine(tokens, range.start.line, includeFence);
131+
if (!block) {
132+
// Don't let anyone else format qmd files
133+
return [];
134+
}
135+
136+
const endBlock = languageBlockAtLine(tokens, range.end.line, includeFence);
137+
if (!endBlock) {
138+
// Selection extends outside of a single block and into ambiguous non-block editor space
139+
// (possibly spanning multiple blocks in the process)
140+
return [];
141+
}
142+
143+
if (block.range.start.line !== endBlock.range.start.line) {
144+
// Selection spans multiple blocks
145+
return [];
146+
}
147+
148+
const edits = await formatBlock(document, block);
149+
if (!edits) {
150+
return [];
151+
}
152+
153+
return edits;
127154
};
128155
}
129156

@@ -133,27 +160,41 @@ class FormatCellCommand implements Command {
133160

134161
public async execute(): Promise<void> {
135162
const editor = window.activeTextEditor;
136-
const doc = editor?.document;
137-
if (doc && isQuartoDoc(doc)) {
138-
const edits = await formatActiveCell(editor, this.engine_);
139-
if (edits) {
140-
editor.edit((editBuilder) => {
141-
// Sort edits by descending start position to avoid range shifting issues
142-
edits
143-
.slice()
144-
.sort((a, b) => b.range.start.compareTo(a.range.start))
145-
.forEach((edit) => {
146-
editBuilder.replace(edit.range, edit.newText);
147-
});
148-
});
149-
} else {
150-
window.showInformationMessage(
151-
"Editor selection is not within a code cell that supports formatting."
152-
);
153-
}
154-
} else {
163+
if (!editor) {
164+
// No active text editor
165+
return;
166+
}
167+
168+
const document = editor.document;
169+
if (!isQuartoDoc(document)) {
155170
window.showInformationMessage("Active editor is not a Quarto document");
171+
return;
172+
}
173+
174+
const includeFence = false;
175+
176+
const tokens = this.engine_.parse(document);
177+
const block = languageBlockAtLine(tokens, editor.selection.start.line, includeFence);
178+
if (!block) {
179+
window.showInformationMessage("Editor selection is not within a code cell.");
180+
return;
181+
}
182+
183+
const edits = await formatBlock(document, block);
184+
if (!edits) {
185+
// Nothing to do! Already formatted, or no formatter picked us up, or this language doesn't support formatting.
186+
return;
156187
}
188+
189+
editor.edit((editBuilder) => {
190+
// Sort edits by descending start position to avoid range shifting issues
191+
edits
192+
.slice()
193+
.sort((a, b) => b.range.start.compareTo(a.range.start))
194+
.forEach((edit) => {
195+
editBuilder.replace(edit.range, edit.newText);
196+
});
197+
});
157198
}
158199
}
159200

@@ -175,14 +216,13 @@ function formattingOptions(
175216
};
176217
}
177218

178-
179219
async function executeFormatDocumentProvider(
180220
vdoc: VirtualDoc,
181221
document: TextDocument,
182222
options: FormattingOptions
183223
): Promise<TextEdit[] | undefined> {
184224
const edits = await withVirtualDocUri(vdoc, document.uri, "format", async (uri: Uri) => {
185-
return await commands.executeCommand<TextEdit[]>(
225+
return await commands.executeCommand<TextEdit[] | undefined>(
186226
"vscode.executeFormatDocumentProvider",
187227
uri,
188228
options
@@ -195,19 +235,18 @@ async function executeFormatDocumentProvider(
195235
}
196236
}
197237

198-
async function formatActiveCell(editor: TextEditor, engine: MarkdownEngine) {
199-
const doc = editor?.document;
200-
const tokens = engine.parse(doc);
201-
const line = editor.selection.start.line;
202-
const position = new Position(line, 0);
203-
const language = languageAtPosition(tokens, position);
204-
const block = languageBlockAtPosition(tokens, position, false);
205-
if (language?.canFormat && block) {
206-
return formatBlock(doc, block, language);
238+
async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock): Promise<TextEdit[] | undefined> {
239+
// Extract language
240+
const language = languageFromBlock(block);
241+
if (!language) {
242+
return undefined;
243+
}
244+
245+
// Refuse to format if not supported by this language
246+
if (!language.canFormat) {
247+
return undefined;
207248
}
208-
}
209249

210-
async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock, language: EmbeddedLanguage) {
211250
// Create virtual document containing the block
212251
const blockLines = lines(codeForExecutableLanguageBlock(block, false));
213252
const vdoc = virtualDocForCode(blockLines, language);
@@ -218,36 +257,39 @@ async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock,
218257
formattingOptions(doc.uri, vdoc.language)
219258
);
220259

221-
if (edits) {
222-
// Because we format with the block code copied in an empty virtual
223-
// document, we need to adjust the ranges to match the edits to the block
224-
// cell in the original file.
225-
const blockRange = new Range(
226-
new Position(block.range.start.line, block.range.start.character),
227-
new Position(block.range.end.line, block.range.end.character)
228-
);
229-
const adjustedEdits = edits
230-
.map(edit => {
231-
const range = new Range(
232-
new Position(edit.range.start.line + block.range.start.line + 1, edit.range.start.character),
233-
new Position(edit.range.end.line + block.range.start.line + 1, edit.range.end.character)
234-
);
235-
return new TextEdit(range, edit.newText);
236-
});
237-
238-
// Bail if any edit is out of range. We used to filter these edits out but
239-
// this could bork the cell.
240-
if (adjustedEdits.some(edit => !blockRange.contains(edit.range))) {
241-
window.showInformationMessage(
242-
"Formatting edits were out of range and could not be applied to the code cell."
260+
if (!edits) {
261+
// Either no formatter picked us up, or there were no edits required.
262+
// We can't determine the difference though!
263+
return undefined;
264+
}
265+
266+
// Because we format with the block code copied in an empty virtual
267+
// document, we need to adjust the ranges to match the edits to the block
268+
// cell in the original file.
269+
const blockRange = new Range(
270+
new Position(block.range.start.line, block.range.start.character),
271+
new Position(block.range.end.line, block.range.end.character)
272+
);
273+
const adjustedEdits = edits
274+
.map(edit => {
275+
const range = new Range(
276+
new Position(edit.range.start.line + block.range.start.line + 1, edit.range.start.character),
277+
new Position(edit.range.end.line + block.range.start.line + 1, edit.range.end.character)
243278
);
244-
return [];
245-
}
279+
return new TextEdit(range, edit.newText);
280+
});
246281

247-
return adjustedEdits;
282+
// Bail if any edit is out of range. We used to filter these edits out but
283+
// this could bork the cell. Return `[]` to indicate that we tried.
284+
if (adjustedEdits.some(edit => !blockRange.contains(edit.range))) {
285+
window.showInformationMessage(
286+
"Formatting edits were out of range and could not be applied to the code cell."
287+
);
288+
return [];
248289
}
249-
}
250290

291+
return adjustedEdits;
292+
}
251293

252294
function unadjustedEdits(
253295
edits: TextEdit[],
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
title: "Format Test"
3+
format: html
4+
---
5+
6+
## Markdown Section
7+
8+
Some regular text here.
9+
10+
```{python}
11+
x = 1 + 1
12+
```
13+
14+
More markdown text.
15+
16+
```{r}
17+
y <- 1 + 1
18+
```
19+
20+
Final line.

0 commit comments

Comments
 (0)