Refactor formatting tooling and don't fire message when code cell is already formatted#933
Merged
DavisVaughan merged 5 commits intoquarto-dev:mainfrom Mar 13, 2026
Conversation
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"
DavisVaughan
commented
Mar 13, 2026
DavisVaughan
commented
Mar 13, 2026
cscheid
reviewed
Mar 13, 2026
cscheid
approved these changes
Mar 13, 2026
Contributor
cscheid
left a comment
There was a problem hiding this comment.
LGTM, and the flatter structure with early returns is so much cleaner to read, thank you.
juliasilge
approved these changes
Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #932
The intent of the
window.showInformationMessage("Editor selection is not within a code cell.");message seems to have mainly been about catching whenQuarto: Format Cellwas being called when you weren't actually in a code cell. I've done some refactoring to now fire this message when we fail to find ablockat the user's cursor position, rather than later on when we fail to geteditsfor that block. It's perfectly legal for a formatter to returnundefinedaseditswhen there is nothing to do, and we shouldn't take that as a signal that we aren't in a code cell.I've added a test for this specific case by mocking
vscode.window.showInformationMessage()for the duration of the test so we can check that it gets called. Pretty neat way to test this.While I was working on this I felt the need to pay down some tech debt on how complicated these formatting functions have become. They are made up of extremely nested if statements and don't need to be (I've felt this pain every time I touch these functions). I think these are way easier to manage if you write them as a series of early exits.
Most of this PR is this refactor, but I don't expect it to change any actual behavior. I realize this is a bit messy though.
I think you should mostly approach this PR as a refactor, with a very minor change that I'll call out below.
I test this with this qmd