Fix inline comments mangling CREATE FOREIGN TABLE formatting#43
Conversation
CREATE FOREIGN TABLE mishandled inline `-- ...` comments the same way CREATE TABLE did before #40: comments between columns were treated as bogus columns (alignment padding plus a trailing comma that mangled the comment text), and a comment after the last element was dropped. Factor the comment-association logic added for CREATE TABLE into a shared `collect_table_elements` helper and use it from both statement formatters. `format_create_foreign_table_stmt` now renders trailing comments at the end of the element they follow (aligned to a common column in river style, single-space in left-aligned styles), keeping the original element order. Also registers the previously unwired CREATE FOREIGN TABLE fixtures in the test harness alongside the new inline-comment fixtures. Closes #41 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI 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)
📝 WalkthroughWalkthroughThis change fixes inline ChangesInline comment association for table elements
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/formatter/stmt.rs (1)
642-657: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated non-river rendering loop.
The non-river branch of
format_create_foreign_table_stmt(Line 1362-1378) is identical to the corresponding branch informat_create_table_stmt(Line 642-657). Consider extracting a shared helper (e.g.render_grouped_elements_left_aligned(&self, grouped: &[(Node<'a>, Vec<String>)], indent: &str) -> Vec<String>) that both callers can use, mirroring howcollect_table_elementswas already extracted for comment association.♻️ Proposed extraction
+ fn render_grouped_elements_left_aligned( + &self, + grouped: &[(Node<'a>, Vec<String>)], + indent: &str, + ) -> Vec<String> { + let mut lines = Vec::new(); + let total = grouped.len(); + for (i, (elem_node, comments)) in grouped.iter().enumerate() { + let elem = self.format_table_element(*elem_node); + let comma = if i < total - 1 { "," } else { "" }; + let line = format!("{indent}{elem}{comma}"); + if let Some((first, rest)) = comments.split_first() { + lines.push(format!("{line} {first}")); + for extra in rest { + lines.push(format!("{indent}{extra}")); + } + } else { + lines.push(line); + } + } + lines + }Also applies to: 1362-1378
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/formatter/stmt.rs` around lines 642 - 657, The non-river rendering loop is duplicated between format_create_table_stmt and format_create_foreign_table_stmt, so extract the shared left-aligned grouped-element rendering into a helper such as render_grouped_elements_left_aligned on the formatter and have both branches call it. Keep the existing behavior for commas, indentation, and attached comments intact, and reuse the same grouped input shape used by collect_table_elements to avoid the repeated logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/formatter/stmt.rs`:
- Around line 642-657: The non-river rendering loop is duplicated between
format_create_table_stmt and format_create_foreign_table_stmt, so extract the
shared left-aligned grouped-element rendering into a helper such as
render_grouped_elements_left_aligned on the formatter and have both branches
call it. Keep the existing behavior for commas, indentation, and attached
comments intact, and reuse the same grouped input shape used by
collect_table_elements to avoid the repeated logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0df342c9-6ba5-4280-8761-555e9cd38470
📒 Files selected for processing (6)
src/formatter/stmt.rstests/fixtures/mozilla/create_foreign_table_inline_comments.expectedtests/fixtures/mozilla/create_foreign_table_inline_comments.sqltests/fixtures/river/create_foreign_table_inline_comments.expectedtests/fixtures/river/create_foreign_table_inline_comments.sqltests/fixtures_test.rs
The non-river rendering loop in format_create_foreign_table_stmt was byte-identical to the one in format_create_table_stmt. Extract it into a render_grouped_elements_left_aligned helper that both callers use, mirroring the earlier collect_table_elements extraction. Addresses CodeRabbit nitpick on PR #43. Behavior is unchanged; all fixtures and unit tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🤖 This comment was posted by Claude on behalf of @gmr Addressed the CodeRabbit nitpick on Behavior is unchanged; |
|
Automated PR monitoring complete — this PR is ready to merge.
All checks passing and review feedback addressed. |
1 participant
Closes #41.
Problem
CREATE FOREIGN TABLEmishandled inline-- ...comments the same wayCREATE TABLEdid before #40. For:CREATE FOREIGN TABLE films ( code CHAR(5) NOT NULL, -- primary identifier title VARCHAR(40) NOT NULL, -- film title did INTEGER NOT NULL --director id ) SERVER film_server OPTIONS (schema_name 'public', table_name 'films');it produced (aweber/river):
Comments got alignment padding and a trailing comma (mangling the comment text, breaking idempotency), and the final comment (
--director id, before the closing paren) was dropped.Fix
The comment-association logic added for
CREATE TABLEin #40 is factored into a sharedcollect_table_elementshelper, now used by bothformat_create_table_stmtandformat_create_foreign_table_stmt. The foreign-table formatter renders trailing comments at the end of the element they follow — aligned to a common column in river style, single-space in left-aligned styles — while keeping the original element order.Result:
Tests
create_foreign_table_inline_commentsfixtures (river + mozilla).create_foreign_tableriver fixtures for regression coverage of the rewritten path.cargo fmt --checkandcargo clippy -D warningsclean; verified idempotent.🤖 Generated with Claude Code
Summary by CodeRabbit
-- ...comments inCREATE TABLEandCREATE FOREIGN TABLE, ensuring comments remain associated with the correct table elements.