Skip to content

Fix cell formatting issues #754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions apps/vscode/src/providers/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated but I thought we should make this robust to providers returning unsorted edits.

edits
.slice()
.sort((a, b) => b.range.start.compareTo(a.range.start))
.forEach((edit) => {
editBuilder.replace(edit.range, edit.newText);
});
});
} else {
window.showInformationMessage(
Expand Down Expand Up @@ -204,15 +208,20 @@ async function formatActiveCell(editor: TextEditor, engine: MarkdownEngine) {
}

async function formatBlock(doc: TextDocument, block: TokenMath | TokenCodeBlock, language: EmbeddedLanguage) {
const blockLines = lines(codeForExecutableLanguageBlock(block));
blockLines.push("");
// Create virtual document containing the block
const blockLines = lines(codeForExecutableLanguageBlock(block, false));
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)
Expand All @@ -224,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))) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated but I'm pretty sure applying edits partially can bork our cells 😬

window.showInformationMessage(
"Formatting edits were out of range and could not be applied to the code cell."
);
return [];
}

return adjustedEdits;
}
}
Expand Down
7 changes: 5 additions & 2 deletions packages/quarto-core/src/markdown/language.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 "";
}
Expand Down