Conversation
Reviewer's GuideThis PR overhauls the process module by enriching Rust documentation, refactoring the core processing loop into dedicated helpers, eliminating redundant buffer clones, and streamlining control flow. Class diagram for refactored process module helpersclassDiagram
class Options {
+bool wrap
+bool ellipsis
+bool fences
+bool footnotes
}
class process_stream_inner {
+process_stream_inner(lines: &[String], opts: Options) Vec<String>
}
class process_stream {
+process_stream(lines: &[String]) Vec<String>
}
class process_stream_no_wrap {
+process_stream_no_wrap(lines: &[String]) Vec<String>
}
class process_stream_opts {
+process_stream_opts(lines: &[String], opts: Options) Vec<String>
}
class flush_buffer {
+flush_buffer(buf: &mut Vec<String>, in_table: &mut bool, out: &mut Vec<String>)
}
class handle_fence_line {
+handle_fence_line(line: &str, buf: &mut Vec<String>, in_code: &mut bool, in_table: &mut bool, out: &mut Vec<String>) bool
}
class handle_table_line {
+handle_table_line(line: &str, buf: &mut Vec<String>, in_table: &mut bool) bool
}
process_stream_inner --> flush_buffer : uses
process_stream_inner --> handle_fence_line : uses
process_stream_inner --> handle_table_line : uses
process_stream --> process_stream_inner : calls
process_stream_no_wrap --> process_stream_inner : calls
process_stream_opts --> process_stream_inner : calls
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
No changes to existing behaviour or output. WalkthroughRefactor the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Process as process_stream_opts
participant Helpers as Helper Functions
participant Output
Caller->>Process: Call with lines and options
loop For each line
Process->>Helpers: handle_fence_line(line, ...)
alt Fence line handled
Helpers->>Output: Flush buffer, toggle code fence
else Not a fence
Process->>Helpers: handle_table_line(line, ...)
alt Table line handled
Helpers->>Output: Buffer or flush table lines
else Not a table
Process->>Helpers: Buffer for wrapping
end
end
end
Process->>Helpers: flush_buffer (at end)
Helpers->>Output: Output remaining lines
Process-->>Caller: Return processed lines
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rsInstructions used from: Sources:
⚙️ CodeRabbit Configuration File 🔇 Additional comments (10)
✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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/process.rs:117` </location>
<code_context>
+ buf.push(line.trim_end().to_string());
+ return true;
+ }
+ if *in_table && !line.trim().is_empty() {
+ buf.push(line.trim_end().to_string());
+ return true;
+ }
+ false
</code_context>
<issue_to_address>
Non-table lines within a table block may be incorrectly buffered.
Currently, non-table lines are added to the table buffer if in_table is true, which may cause incorrect table parsing. Please add stricter checks to ensure only valid table lines are buffered or improve table block termination handling.
</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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
src/process.rs (2)
134-137: Address potential issue with non-table line buffering.The logic here may incorrectly buffer non-table lines when
in_tableis true, as identified in the past review. Add stricter validation to ensure only valid table content is buffered.- if *in_table && (line.contains('|') || crate::table::SEP_RE.is_match(line.trim())) { + if *in_table && (line.contains('|') || crate::table::SEP_RE.is_match(line.trim())) { + // Ensure this is actually table content, not just any line containing '|' buf.push(line.trim_end().to_string()); return true; }
149-150: Clarify why non-table lines are buffered in table context.This logic buffers any line when
in_tableis true, which could lead to incorrect table parsing. Add a comment explaining why this is intentional or fix the logic.Add a comment explaining the rationale:
+ // Buffer continuation lines that are part of table cells buf.push(line.trim_end().to_string()); return true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/process.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🔇 Additional comments (1)
src/process.rs (1)
12-28: Fix inconsistent American spelling in documentation.The documentation uses American spelling "behavior" instead of the required en-GB spelling "behaviour".
-/// Processing options controlling the behaviour of [`process_stream_inner`]. +/// Processing options controlling the behaviour of [`process_stream_inner`].Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
src/process.rs (6)
60-75: Remove documentation for private helper function.Same issue as
flush_buffer- private helper functions don't need Rustdoc comments according to the guidelines.-/// Detects fence lines and toggles code mode, flushing buffered content. fn handle_fence_line(
77-114: Remove documentation for private helper function.Private helper functions should not have Rustdoc comments according to the coding guidelines.
-/// Buffers table lines, returning `true` when a line was consumed. fn handle_table_line(
45-58: Remove documentation entirely for private helper functions.Private helper functions don't require Rustdoc comments according to the coding guidelines. The current documentation doesn't add value and creates maintenance overhead.
-/// Flushes buffered lines to `out`, formatting as a table when required. -#[allow(clippy::extend_with_drain)] // maintain consistency across helpers fn flush_buffer(buf: &mut Vec<String>, in_table: &mut bool, out: &mut Vec<String>) {
185-199: Relocate function attribute after documentation.The
#[must_use]attribute appears before the documentation, violating the coding guidelines which specify that function attributes must be placed after doc comments.-/// Processes a Markdown stream with all default options enabled. -#[must_use] -/// +/// Processes a Markdown stream with all default options enabled. +/// /// This is the primary convenience function used by the command-line /// interface. Paragraphs are wrapped and tables are reflowed. /// @@ -198,6 +197,7 @@ /// let out = process_stream(&lines); /// assert!(out.iter().any(|l| l.contains("| a | b |"))); /// ``` +#[must_use] pub fn process_stream(lines: &[String]) -> Vec<String> {
212-224: Relocate function attribute after documentation.The
#[must_use]attribute appears before the documentation, violating the coding guidelines.-/// Processes Markdown without wrapping paragraphs. -#[must_use] -/// +/// Processes Markdown without wrapping paragraphs. +/// /// Useful when only table reflow and code fence normalisation are required. /// @@ -223,6 +222,7 @@ /// let out = process_stream_no_wrap(&lines); /// assert!(out.iter().any(|l| l.contains("| a | b |"))); /// ``` +#[must_use] pub fn process_stream_no_wrap(lines: &[String]) -> Vec<String> {
237-256: Relocate function attribute after documentation.The
#[must_use]attribute appears before the documentation, violating the coding guidelines.-/// Runs [`process_stream_inner`] with custom [`Options`]. -#[must_use] -/// +/// Runs [`process_stream_inner`] with custom [`Options`]. +/// /// This is exposed for advanced use cases where callers want precise /// control over the processing pipeline. /// @@ -255,6 +254,7 @@ /// let out = process_stream_opts(&lines, opts); /// assert_eq!(out, vec!["text"]); /// ``` +#[must_use] pub fn process_stream_opts(lines: &[String], opts: Options) -> Vec<String> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/process.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
⏰ Context from checks skipped due to timeout of 240000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (1)
src/process.rs (1)
12-28: Documentation example violates coding guidelines.The example demonstrates usage of a private helper function
process_stream_optsthat hasn't been defined yet at this point in the file. This creates forward references and doesn't clearly demonstrate theOptionsstruct usage itself.Replace the example to focus on the struct construction and field usage rather than function calls:
-/// # Examples -/// -/// ``` -/// use mdtablefix::process::{Options, process_stream_opts}; -/// -/// let lines = vec!["example".to_string()]; -/// let opts = Options { -/// wrap: false, -/// ellipsis: false, -/// fences: false, -/// footnotes: false, -/// }; -/// let out = process_stream_opts(&lines, opts); -/// assert_eq!(out, vec!["example"]); -/// ``` +/// # Examples +/// +/// ``` +/// use mdtablefix::process::Options; +/// +/// let opts = Options { +/// wrap: true, +/// ellipsis: false, +/// fences: true, +/// footnotes: false, +/// }; +/// assert!(opts.wrap); +/// assert!(opts.fences); +/// ```Likely an incorrect or invalid review comment.
leynos
force-pushed
the
codex/add-doc-comments-and-refactor-process.rs
branch
from
8cf2630 to
4568644
Compare
1 participant
Summary
Optionsand stream processing functionsprocess_stream_innerinto smaller helpersTesting
make fmtmake lintmake testmake markdownlinthttps://chatgpt.com/codex/tasks/task_e_687ceafcc8a083229ca3990093c97e7d
Summary by Sourcery
Refactor the Markdown stream processing module by breaking down the core loop into reusable helper functions, eliminating buffer clones, and enriching the public API with detailed Rustdoc and usage examples.
Enhancements:
Documentation: