Skip to content

Fix indentation handling for comments in bracketed expressions#2429

Open
benfdking wants to merge 1 commit intomainfrom
claude/fix-comment-formatting-TxWky
Open

Fix indentation handling for comments in bracketed expressions#2429
benfdking wants to merge 1 commit intomainfrom
claude/fix-comment-formatting-TxWky

Conversation

@benfdking
Copy link
Copy Markdown
Collaborator

Summary

Fixed a bug where comments inside parentheses were causing incorrect indentation in SQL formatting. The issue was in the indent point tracking logic where untaken_indents was being moved instead of cloned, causing loss of indent state information.

Changes

  • reindent.rs: Changed take(&mut untaken_indents) to untaken_indents.clone() when creating indent points at line breaks. This ensures the indent state is preserved for subsequent processing rather than being consumed.
  • LT02-indent.yml: Added test cases for GitHub issue My example formatted weirdly if a comment is in a specific place #1967:
    • test_pass_comment_in_bracketed_expression: Validates that correctly indented SQL with comments in parentheses passes validation
    • test_fix_comment_in_bracketed_expression: Validates that incorrectly indented SQL with comments in parentheses is properly fixed

Implementation Details

The root cause was that take() was transferring ownership of untaken_indents, leaving it empty for subsequent indent point processing. By cloning instead, the indent tracking state is properly maintained throughout the crawl of indent points, allowing comments within bracketed expressions to be handled without disrupting the indentation logic.

https://claude.ai/code/session_016UQdggVzpod5MEX18Gnya5

When a comment appeared inside parentheses (e.g., before an expression),
the formatter would add extra indentation to subsequent lines. This was
caused by `take()` emptying the `untaken_indents` vector during cache
flush in `crawl_indent_points`, which meant the subsequent balance
update via `update_crawl_balances` started with an empty vector instead
of preserving the untaken indents. Changed to `clone()` to match the
behavior of the normal (non-cached) code path.

https://claude.ai/code/session_016UQdggVzpod5MEX18Gnya5
@github-actions
Copy link
Copy Markdown
Contributor

Benchmark for 046e219

Click to view benchmark
Test Base PR %
DepthMap::from_parent 53.0±0.43µs 52.7±0.50µs -0.57%
fix_complex_query 11.8±0.11ms 11.8±0.40ms 0.00%
fix_superlong 156.9±4.85ms 158.2±5.73ms +0.83%
parse_complex_query 4.2±0.05µs 4.2±0.07µs 0.00%
parse_expression_recursion 7.2±0.08µs 7.2±0.10µs 0.00%
parse_simple_query 1033.6±11.43ns 1057.0±20.93ns +2.26%

@jason-nance
Copy link
Copy Markdown

Hi! Any plans to merge this? We've noticed this problem as well in several SQL files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants