fix: keep leading block comment of clause item on its own line#952
Conversation
A block comment placed before the first item of a clause (e.g. `SELECT /* c */ foo FROM tbl`) was formatted inline on the first pass, but moved onto its own line when the result was formatted again, making formatting non-idempotent. The inline-vs-standalone decision for a block comment was based solely on its `precedingWhitespace`. The formatter always emits a newline before the first item of a clause body, so on a second pass that structural newline becomes the comment's preceding whitespace and flips the decision. Treat a block comment as standalone whenever it is already at the start of a line in the output being built, in addition to the existing checks. This makes the first pass place such comments on their own line directly, so the output is stable under repeated formatting. Inline block comments that follow other tokens on the same line (trailing comments, comments between a function name and its parentheses, or around the `.` of a qualified name) are unaffected.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR makes block comment emission position-aware: Layout gains ChangesBlock Comment Standalone Formatting
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks. This looks like a reasonable fix. At least on first look. I need to ponder it some more before merging. I would definitely appreciate not reading through a massive amount of AI-generated descriptions of this whole thing. I also would like to have some more tests for this. Currently we're only checking for one scenario: a comment right after SELECT foo /*comment */, /* comment */ bar FROM /*comment */ tbl1 JOIN /* comment */ tbl2;
UPDATE /* comment */ tbl /*comment */ SET /* comment */ x = 10;These are just random examples. I'm trying to come up with scenarios that could possibly cause problems. But mainly we should just have a few more tests to ensure that this fix does indeed work in general, not just for one single case. |
|
Good call — added a test that runs comments in more positions (between items, after FROM/JOIN, in UPDATE/SET) through formatting twice to confirm they're all idempotent; full suite passes. Also trimmed the description. Thanks for taking a look! |
|
OK. Thanks. It's merged now, and released in 15.8.1 |
A block comment before the first item of a clause (e.g.
SELECT /* c */ foo FROM tbl) was formatted inline on the first pass but moved to its own line on the second — so formatting wasn't idempotent.The inline-vs-standalone decision used only the comment's
precedingWhitespace. Since the formatter emits a newline before a clause's first item, the second pass sees that structural newline and flips the decision. Now a block comment is also treated as standalone when it's already at the start of the line in the output being built, so the first pass already matches the fixpoint. Genuinely-inline comments are unaffected.Added tests covering comments in more positions (between items, after FROM/JOIN, in UPDATE/SET) to confirm it generalizes.
Summary by CodeRabbit
Release Notes