Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions src/breaks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//! Thematic break formatting utilities.

use regex::Regex;

use crate::wrap::is_fence;

pub const THEMATIC_BREAK_LEN: usize = 70;

static THEMATIC_BREAK_RE: std::sync::LazyLock<Regex> = std::sync::LazyLock::new(|| {
Regex::new(r"^[ ]{0,3}((?:[ \t]*\*){3,}|(?:[ \t]*-){3,}|(?:[ \t]*_){3,})[ \t]*$").unwrap()
});

static THEMATIC_BREAK_LINE: std::sync::LazyLock<String> =
std::sync::LazyLock::new(|| "_".repeat(THEMATIC_BREAK_LEN));

#[must_use]
pub fn format_breaks(lines: &[String]) -> Vec<String> {
let mut out = Vec::with_capacity(lines.len());
let mut in_code = false;

for line in lines {
if is_fence(line) {
in_code = !in_code;
out.push(line.clone());
continue;
}

if !in_code && THEMATIC_BREAK_RE.is_match(line.trim_end()) {
out.push(THEMATIC_BREAK_LINE.clone());
} else {
out.push(line.clone());
}
}

out
}
Comment on lines +16 to +36
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Optimize to avoid unnecessary string clones.

The function clones every line regardless of whether it needs modification. Use Cow<str> or conditionally clone only when replacing thematic breaks.

+use std::borrow::Cow;
+
 #[must_use]
-pub fn format_breaks(lines: &[String]) -> Vec<String> {
+pub fn format_breaks(lines: &[String]) -> Vec<String> {
     let mut out = Vec::with_capacity(lines.len());
     let mut in_code = false;

     for line in lines {
         if is_fence(line) {
             in_code = !in_code;
-            out.push(line.clone());
+            out.push(line.clone());
             continue;
         }

         if !in_code && THEMATIC_BREAK_RE.is_match(line.trim_end()) {
             out.push(THEMATIC_BREAK_LINE.clone());
         } else {
             out.push(line.clone());
         }
     }

     out
 }

Alternatively, return Vec<Cow<'_, str>> to avoid cloning unchanged lines entirely.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/breaks.rs between lines 16 and 36, the function format_breaks clones
every line even when no modification is needed, causing unnecessary allocations.
Refactor the function to use Cow<str> so that unchanged lines are borrowed
rather than cloned, and only lines replaced with THEMATIC_BREAK_LINE are owned
strings. Update the return type to Vec<Cow<'_, str>> and adjust the logic to
push either a borrowed line or a cloned thematic break line accordingly.


#[cfg(test)]
mod tests {
use super::*;

#[test]
fn basic_formatting() {
let input = vec!["foo", "***", "bar"]
.into_iter()
.map(str::to_string)
.collect::<Vec<_>>();
let expected = vec![
"foo".to_string(),
"_".repeat(THEMATIC_BREAK_LEN),
"bar".to_string(),
];
assert_eq!(format_breaks(&input), expected);
}

#[test]
fn ignores_fenced_code() {
let input = vec!["```", "---", "```"]
.into_iter()
.map(str::to_string)
.collect::<Vec<_>>();
assert_eq!(format_breaks(&input), input);
Comment on lines +44 to +62
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Simplify test data construction.

Use vec! macro directly with .to_string() for cleaner test setup.

 #[test]
 fn basic_formatting() {
-    let input = vec!["foo", "***", "bar"]
-        .into_iter()
-        .map(str::to_string)
-        .collect::<Vec<_>>();
+    let input = vec!["foo".to_string(), "***".to_string(), "bar".to_string()];
     let expected = vec![
         "foo".to_string(),
         "_".repeat(THEMATIC_BREAK_LEN),
         "bar".to_string(),
     ];
     assert_eq!(format_breaks(&input), expected);
 }

 #[test]
 fn ignores_fenced_code() {
-    let input = vec!["```", "---", "```"]
-        .into_iter()
-        .map(str::to_string)
-        .collect::<Vec<_>>();
+    let input = vec!["```".to_string(), "---".to_string(), "```".to_string()];
     assert_eq!(format_breaks(&input), input);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let input = vec!["foo", "***", "bar"]
.into_iter()
.map(str::to_string)
.collect::<Vec<_>>();
let expected = vec![
"foo".to_string(),
"_".repeat(THEMATIC_BREAK_LEN),
"bar".to_string(),
];
assert_eq!(format_breaks(&input), expected);
}
#[test]
fn ignores_fenced_code() {
let input = vec!["```", "---", "```"]
.into_iter()
.map(str::to_string)
.collect::<Vec<_>>();
assert_eq!(format_breaks(&input), input);
#[test]
fn basic_formatting() {
let input = vec!["foo".to_string(), "***".to_string(), "bar".to_string()];
let expected = vec![
"foo".to_string(),
"_".repeat(THEMATIC_BREAK_LEN),
"bar".to_string(),
];
assert_eq!(format_breaks(&input), expected);
}
#[test]
fn ignores_fenced_code() {
let input = vec!["
🤖 Prompt for AI Agents
In src/breaks.rs around lines 44 to 62, the test data construction uses an
iterator and map to convert string slices to Strings, which is unnecessarily
verbose. Simplify this by directly using the vec! macro with each element
converted to a String using .to_string(), for example, replace the iterator and
map chain with vec!["```".to_string(), "---".to_string(), "```".to_string()] to
make the test setup cleaner and more readable.

}
}
2 changes: 1 addition & 1 deletion src/html.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use html5ever::{driver::ParseOpts, parse_document, tendril::TendrilSink};
use markup5ever_rcdom::{Handle, NodeData, RcDom};
use regex::Regex;

use crate::is_fence;
use crate::wrap::is_fence;

/// Matches the start of an HTML `<table>` tag, ignoring case.
static TABLE_START_RE: LazyLock<Regex> =
Expand Down
54 changes: 54 additions & 0 deletions src/io.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//! File helpers for rewriting Markdown documents.

use std::{fs, path::Path};

use crate::process::{process_stream, process_stream_no_wrap};

/// Rewrite a file in place with wrapped tables.
///
/// # Errors
/// Returns an error if reading or writing the file fails.
pub fn rewrite(path: &Path) -> std::io::Result<()> {
let text = fs::read_to_string(path)?;
let lines: Vec<String> = text.lines().map(str::to_string).collect();
let fixed = process_stream(&lines);
fs::write(path, fixed.join("\n") + "\n")
}

/// Rewrite a file in place without wrapping text.
///
/// # Errors
/// Returns an error if reading or writing the file fails.
pub fn rewrite_no_wrap(path: &Path) -> std::io::Result<()> {
let text = fs::read_to_string(path)?;
let lines: Vec<String> = text.lines().map(str::to_string).collect();
let fixed = process_stream_no_wrap(&lines);
fs::write(path, fixed.join("\n") + "\n")
}
Comment on lines +11 to +27
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Extract common logic to reduce duplication.

Both functions share identical logic except for the processing function call. Extract the common pattern to adhere to DRY principles.

+fn rewrite_impl(path: &Path, wrap: bool) -> std::io::Result<()> {
+    let text = fs::read_to_string(path)?;
+    let lines: Vec<String> = text.lines().map(str::to_string).collect();
+    let fixed = if wrap {
+        process_stream(&lines)
+    } else {
+        process_stream_no_wrap(&lines)
+    };
+    fs::write(path, fixed.join("\n") + "\n")
+}
+
 pub fn rewrite(path: &Path) -> std::io::Result<()> {
-    let text = fs::read_to_string(path)?;
-    let lines: Vec<String> = text.lines().map(str::to_string).collect();
-    let fixed = process_stream(&lines);
-    fs::write(path, fixed.join("\n") + "\n")
+    rewrite_impl(path, true)
 }

 pub fn rewrite_no_wrap(path: &Path) -> std::io::Result<()> {
-    let text = fs::read_to_string(path)?;
-    let lines: Vec<String> = text.lines().map(str::to_string).collect();
-    let fixed = process_stream_no_wrap(&lines);
-    fs::write(path, fixed.join("\n") + "\n")
+    rewrite_impl(path, false)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn rewrite(path: &Path) -> std::io::Result<()> {
let text = fs::read_to_string(path)?;
let lines: Vec<String> = text.lines().map(str::to_string).collect();
let fixed = process_stream(&lines);
fs::write(path, fixed.join("\n") + "\n")
}
/// Rewrite a file in place without wrapping text.
///
/// # Errors
/// Returns an error if reading or writing the file fails.
pub fn rewrite_no_wrap(path: &Path) -> std::io::Result<()> {
let text = fs::read_to_string(path)?;
let lines: Vec<String> = text.lines().map(str::to_string).collect();
let fixed = process_stream_no_wrap(&lines);
fs::write(path, fixed.join("\n") + "\n")
}
fn rewrite_impl(path: &Path, wrap: bool) -> std::io::Result<()> {
let text = fs::read_to_string(path)?;
let lines: Vec<String> = text.lines().map(str::to_string).collect();
let fixed = if wrap {
process_stream(&lines)
} else {
process_stream_no_wrap(&lines)
};
fs::write(path, fixed.join("\n") + "\n")
}
pub fn rewrite(path: &Path) -> std::io::Result<()> {
rewrite_impl(path, true)
}
/// Rewrite a file in place without wrapping text.
///
/// # Errors
/// Returns an error if reading or writing the file fails.
pub fn rewrite_no_wrap(path: &Path) -> std::io::Result<()> {
rewrite_impl(path, false)
}
🤖 Prompt for AI Agents
In src/io.rs around lines 11 to 27, both rewrite and rewrite_no_wrap functions
duplicate the logic of reading the file, converting lines, processing, and
writing back. Extract this common pattern into a single helper function that
takes the processing function as a parameter, then have rewrite and
rewrite_no_wrap call this helper with their respective processing functions to
eliminate duplication and follow DRY principles.


#[cfg(test)]
mod tests {
use tempfile::tempdir;

use super::*;

#[test]
fn rewrite_roundtrip() {
let dir = tempdir().unwrap();
let file = dir.path().join("sample.md");
fs::write(&file, "|A|B|\n|1|2|").unwrap();
rewrite(&file).unwrap();
let out = fs::read_to_string(&file).unwrap();
assert!(out.contains("| A | B |"));
}

#[test]
fn rewrite_no_wrap_roundtrip() {
let dir = tempdir().unwrap();
let file = dir.path().join("sample.md");
fs::write(&file, "|A|B|\n|1|2|").unwrap();
rewrite_no_wrap(&file).unwrap();
let out = fs::read_to_string(&file).unwrap();
assert_eq!(out, "| A | B |\n| 1 | 2 |\n");
}
}
Comment on lines +29 to +54
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

🛠️ Refactor suggestion

Add tests for error scenarios.

The current tests only cover successful operations. Add tests for I/O error scenarios such as non-existent files and permission errors.

Would you like me to generate comprehensive error handling tests for file I/O operations?

🤖 Prompt for AI Agents
In src/io.rs between lines 29 and 54, the existing tests only cover successful
file operations. To improve test coverage, add new test cases that simulate
error scenarios such as attempting to rewrite a non-existent file and handling
permission denied errors. Use Rust's error handling to assert that the functions
return appropriate errors when these conditions occur. This will ensure the code
gracefully handles I/O failures.

Loading