Conversation
Reviewer's GuideImplements core markdown table reflow in a library, exposes a clap-based CLI ( Sequence Diagram:
|
| Change | Details | Files |
|---|---|---|
| Add markdown table reflow logic in library crate |
|
src/lib.rs |
| Introduce CLI tool with in-place and stdin support |
|
src/main.rs |
| Update Cargo.toml with new dependencies and dev-dependencies |
|
Cargo.toml |
| Add integration tests for table reflow and CLI behavior |
|
tests/integration.rs |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe changes introduce a Rust library and command-line tool for reflowing and fixing broken markdown tables. New dependencies are added, the main logic for table processing and file rewriting is implemented, and a CLI interface is created using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (main.rs)
participant Library (lib.rs)
participant FileSystem
User->>CLI (main.rs): Run with [--in-place] [files]
alt No files
CLI (main.rs)->>User: Read from stdin
CLI (main.rs)->>Library (lib.rs): process_stream(lines)
Library (lib.rs)-->>CLI (main.rs): processed_lines
CLI (main.rs)->>User: Output to stdout
else With files and --in-place
loop For each file
CLI (main.rs)->>Library (lib.rs): rewrite(file_path)
Library (lib.rs)->>FileSystem: Read & Write file
end
else With files, no --in-place
loop For each file
CLI (main.rs)->>FileSystem: Read file
FileSystem-->>CLI (main.rs): file_content
CLI (main.rs)->>Library (lib.rs): process_stream(lines)
Library (lib.rs)-->>CLI (main.rs): processed_lines
CLI (main.rs)->>User: Output to stdout
end
end
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/main.rs:20` </location>
<code_context>
+fn main() -> anyhow::Result<()> {
+ let cli = Cli::parse();
+
+ if cli.in_place && cli.files.is_empty() {
+ anyhow::bail!("--in-place requires at least one file");
+ }
</code_context>
<issue_to_address>
Use clap's validation to enforce --in-place requires files
Consider using `.requires` on the `in_place` argument or an `ArgGroup` so clap enforces this constraint and provides consistent error messages.
Suggested implementation:
```rust
/// Rewrite files in place
#[arg(long = "in-place", requires = "files")]
in_place: bool,
```
```rust
```
</issue_to_address>
### Comment 2
<location> `src/lib.rs:11` </location>
<code_context>
+
+/// Split a markdown table line into its cells.
+#[must_use]
+fn split_cells(line: &str) -> Vec<String> {
+ let mut s = line.trim();
+ if let Some(stripped) = s.strip_prefix('|') {
</code_context>
<issue_to_address>
split_cells may mis-handle escaped pipes
Consider using a parser or regex that correctly handles escaped pipes (\|) to avoid incorrect cell splitting.
</issue_to_address>
### Comment 3
<location> `src/lib.rs:27` </location>
<code_context>
+/// # Panics
+/// Panics if the internal regex fails to compile.
+#[must_use]
+pub fn reflow_table(lines: &[String]) -> Vec<String> {
+ let raw = lines.iter().map(|l| l.trim()).collect::<Vec<_>>().join(" ");
+ let sentinel_re = Regex::new(r"\|\s*\|\s*").unwrap();
</code_context>
<issue_to_address>
Header separator rows are not preserved
Update the logic to identify and retain header separator rows (e.g., `|---|---|`) to maintain valid markdown formatting after reflow.
</issue_to_address>
### Comment 4
<location> `src/lib.rs:29` </location>
<code_context>
+#[must_use]
+pub fn reflow_table(lines: &[String]) -> Vec<String> {
+ let raw = lines.iter().map(|l| l.trim()).collect::<Vec<_>>().join(" ");
+ let sentinel_re = Regex::new(r"\|\s*\|\s*").unwrap();
+ let chunks: Vec<&str> = sentinel_re.split(&raw).collect();
+ let mut cells = Vec::new();
</code_context>
<issue_to_address>
Avoid recompiling regex on every call
Compiling the regex on each function call is inefficient; use `lazy_static` or `once_cell` to compile it once and reuse it.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
#[must_use]
pub fn reflow_table(lines: &[String]) -> Vec<String> {
let raw = lines.iter().map(|l| l.trim()).collect::<Vec<_>>().join(" ");
let sentinel_re = Regex::new(r"\|\s*\|\s*").unwrap();
let chunks: Vec<&str> = sentinel_re.split(&raw).collect();
let mut cells = Vec::new();
=======
use once_cell::sync::Lazy;
use regex::Regex;
static SENTINEL_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"\|\s*\|\s*").unwrap());
#[must_use]
pub fn reflow_table(lines: &[String]) -> Vec<String> {
let raw = lines.iter().map(|l| l.trim()).collect::<Vec<_>>().join(" ");
let chunks: Vec<&str> = SENTINEL_RE.split(&raw).collect();
let mut cells = Vec::new();
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `src/lib.rs:28` </location>
<code_context>
+/// Panics if the internal regex fails to compile.
+#[must_use]
+pub fn reflow_table(lines: &[String]) -> Vec<String> {
+ let raw = lines.iter().map(|l| l.trim()).collect::<Vec<_>>().join(" ");
+ let sentinel_re = Regex::new(r"\|\s*\|\s*").unwrap();
+ let chunks: Vec<&str> = sentinel_re.split(&raw).collect();
</code_context>
<issue_to_address>
Trimming lines drops indentation
`trim()` removes leading spaces, which may disrupt nested tables. Suggest preserving and restoring indentation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/main.rs (1)
10-14: Let clap enforce the--in-place→ files relationshipInstead of the manual runtime check below, add
.requires("files")to thein_placearg (or use anArgGroup).
That way clap gives a consistent error message and the early-return logic can be removed.- /// Rewrite files in place - #[arg(long = "in-place")] + /// Rewrite files in place + #[arg(long = "in-place", requires = "files")] in_place: bool,src/lib.rs (2)
29-31: Regex re-compiled on every call – use a static
Regex::new(..)insidereflow_tableis executed per invocation.
Move it to astaticviaonce_cell/lazy_staticto avoid repeated compilation.
12-13: 🛠️ Refactor suggestion
trim()drops indentationCalling
trim()on every line removes leading spaces, so tables nested in lists lose their indent.
Preserve leading whitespace and restore it when formatting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.toml(1 hunks)src/lib.rs(1 hunks)src/main.rs(1 hunks)tests/integration.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main.rs (1)
src/lib.rs (3)
process_stream(85-141)rewrite(147-152)lines(28-28)
tests/integration.rs (1)
src/lib.rs (3)
process_stream(85-141)reflow_table(27-78)lines(28-28)
🔇 Additional comments (1)
Cargo.toml (1)
4-4:edition = "2024"requires nightly – verify tool-chain or downgrade to 2021The 2024 edition is, at the moment, only available on nightly Rust.
Unless the project is intentionally targeting nightly, consider settingedition = "2021"to avoid breaking users who build with stable cargo.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/integration.rs:33` </location>
<code_context>
+}
+
+#[rstest]
+fn test_process_stream_ignores_code_fences() {
+ let lines = vec![
+ "```".to_string(),
</code_context>
<issue_to_address>
Add a test case for tilde-based code fences (`~~~`) in `process_stream`.
Please add a test case using `~~~` as the code fence to verify this variant is handled correctly.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
#[rstest]
fn test_process_stream_ignores_code_fences() {
let lines = vec![
"```".to_string(),
"| not | a | table |".to_string(),
"```".to_string(),
];
assert_eq!(process_stream(&lines), lines);
}
=======
#[rstest]
fn test_process_stream_ignores_code_fences() {
let lines = vec![
"```".to_string(),
"| not | a | table |".to_string(),
"```".to_string(),
];
assert_eq!(process_stream(&lines), lines);
// Test with tilde-based code fences
let tilde_lines = vec![
"~~~".to_string(),
"| not | a | table |".to_string(),
"~~~".to_string(),
];
assert_eq!(process_stream(&tilde_lines), tilde_lines);
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
…les` Docstrings generation was requested by @leynos. * #1 (comment) The following files were modified: * `src/lib.rs` * `src/main.rs` * `tests/integration.rs`
|
Note Generated docstrings for this pull request at #6 |
…D5esj42rZYjTqgG31ObYBIhCLJhgeBqlKaS4WfMG5ng8LGP4nFu6yBeIf1 📝 Add docstrings to `codex/implement-rust-tool-to-reflow-markdown-tables`
Summary
rstestTesting
cargo clippy -- -D warningscargo testnpx markdownlint-cli2 '**/*.md' '#node_modules'(fails: 339 errors)npx nixie '**/*.md' '#node_modules'(failed: Syntax error)https://chatgpt.com/codex/tasks/task_e_684c37f2c8a08322b6a26c4313108ab2
Summary by Sourcery
Implement a markdown table fixer by adding reflow logic in a library crate, wiring up a clap-based CLI for file or stdin processing, and covering behavior with comprehensive tests.
New Features:
Enhancements:
Build:
Tests:
Summary by CodeRabbit
New Features
Bug Fixes
Tests