Preserve fenced and indented blocks after headings with --wrap - #268
Merged
Conversation
Add CLI and unit tests to ensure the --wrap and --in-place options preserve the integrity of fenced shell code blocks after headings, preventing unintended modifications. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
SummaryThis PR fixes a bug where --wrap could corrupt fenced code blocks (notably those immediately after headings) by re-wrapping their contents, and ensures indented (four-space) shell blocks following headings are preserved verbatim. The core wrap logic now preserves fenced and indented code blocks unchanged. Comprehensive regression tests were added to prevent regressions and the PR closes issue No new execplan/design document was added or referenced. Changes
Behaviour
Tests / Verification
Related issues
WalkthroughAdd regression tests (unit and CLI) that verify Changes
Sequence Diagram(s)(omitted — changes are test additions and do not introduce new multi-component control flow) Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Reviewer's GuideAdds regression coverage (unit and CLI tests) to ensure Markdown wrapping preserves fenced and indented code blocks that appear after a heading, guarding against corruption when using --wrap / --wrap --in-place. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Add CLI and unit tests ensuring that wrapping with --in-place leaves indented shell code blocks byte-identical, guarding against issue #261. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
leynos
marked this pull request as ready for review
April 22, 2026 15:59
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/wrap_cli.rs`:
- Around line 11-70: The two tests duplicate temp-file setup, invoking
Command::cargo_bin("mdtablefix") with args ["--wrap","--in-place"], and
file-read/assert logic; extract that flow into a helper function (e.g.,
run_wrap_in_place_and_read_back) that takes input: &str, writes it to a
NamedTempFile, runs the CLI
(Command::cargo_bin("mdtablefix")...args(["--wrap","--in-place"]).arg(temp.path()).assert().success().stdout("").stderr("")),
reads the file back and returns the string, then replace the two tests with a
single parameterised rstest function named
cli_wrap_in_place_preserves_shell_block_verbatim using #[rstest] #[case(...,
"message")] cases for the fenced and indented inputs and assert_eq!(actual,
input, "{message}").
In `@tests/wrap_unit.rs`:
- Around line 108-136: Replace the two near-duplicate tests
wrap_text_preserves_fenced_shell_block_after_heading and
wrap_text_preserves_indented_shell_block_after_heading with a single
parameterized rstest named wrap_text_preserves_shell_block_after_heading that
takes a #[case] input: Vec<String> and asserts assert_eq!(wrap_text(&input, 80),
input); add two #[case] entries (one fenced ```bash block and one indented
block) and ensure the rstest attribute is imported/available (rstest crate in
dev-dependencies and use rstest::rstest) so the new parameterized test runs
correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ae21757-d4bb-476f-8409-1b6ae2da7068
📒 Files selected for processing (2)
tests/wrap_cli.rstests/wrap_unit.rs
…apping Preserve fenced and indented code blocks verbatim when `--wrap` is used, so commands inside code examples are not joined or re-wrapped. This change addresses issue #261 by ensuring code blocks remain unchanged during wrapping operations, improving content accuracy and readability. Includes new unit tests guarding this behavior. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/wrap_unit.rs (1)
116-145: 🛠️ Refactor suggestion | 🟠 MajorCollapse duplicated preservation tests into one
rstestmatrix.Remove repetition across Line 116-129 and Line 134-145. Parameterize both inputs through one assertion path.
Proposed refactor
+use rstest::rstest; @@ -#[test] -fn wrap_text_preserves_fenced_shell_block_after_heading() { - let input = vec![ - "## Verification".to_string(), - String::new(), - "```bash".to_string(), - "set -o pipefail".to_string(), - "make check-fmt 2>&1 | tee /tmp/fmt.log".to_string(), - "make lint 2>&1 | tee /tmp/lint.log".to_string(), - "make test 2>&1 | tee /tmp/test.log".to_string(), - "```".to_string(), - ]; - - assert_eq!(wrap_text(&input, 80), input); -} - -#[test] -fn wrap_text_preserves_indented_shell_block_after_heading() { - let input = vec![ - "## Verification".to_string(), - String::new(), - " set -o pipefail".to_string(), - " make check-fmt 2>&1 | tee /tmp/fmt.log".to_string(), - " make lint 2>&1 | tee /tmp/lint.log".to_string(), - " make test 2>&1 | tee /tmp/test.log".to_string(), - ]; - - assert_eq!(wrap_text(&input, 80), input); -} +#[rstest] +#[case(vec![ + "## Verification".to_string(), + String::new(), + "```bash".to_string(), + "set -o pipefail".to_string(), + "make check-fmt 2>&1 | tee /tmp/fmt.log".to_string(), + "make lint 2>&1 | tee /tmp/lint.log".to_string(), + "make test 2>&1 | tee /tmp/test.log".to_string(), + "```".to_string(), +])] +#[case(vec![ + "## Verification".to_string(), + String::new(), + " set -o pipefail".to_string(), + " make check-fmt 2>&1 | tee /tmp/fmt.log".to_string(), + " make lint 2>&1 | tee /tmp/lint.log".to_string(), + " make test 2>&1 | tee /tmp/test.log".to_string(), +])] +fn wrap_text_preserves_shell_block_after_heading(#[case] input: Vec<String>) { + assert_eq!(wrap_text(&input, 80), input); +}#!/bin/bash # Verify duplicate test bodies and rstest availability before refactor. set -euo pipefail echo "== Duplicate assertion path candidates ==" rg -n -C2 'assert_eq!\(wrap_text\(&input, 80\), input\);' tests/wrap_unit.rs echo "== Check whether rstest is already declared in manifest files ==" fd -i 'Cargo.toml' --exec rg -n -C2 'rstest'As per coding guidelines, "Use
rstestfixtures for shared setup in Rust tests." and "Replace duplicated Rust tests with#[rstest(...)]parameterized cases."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/wrap_unit.rs`:
- Around line 113-116: The doc comments (///) must be placed above the test
function signatures and the #[test] attribute must come after the docs; for the
test function wrap_text_preserves_fenced_shell_block_after_heading (and the
other test in this file that currently has its /// blocks below the attribute),
move each triple-slash Rustdoc block so it sits immediately above the fn
declaration and then put #[test] directly below those docs and above the fn to
conform to Rust guideline "place function attributes after doc comments."
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a3f8b39c-f88c-422c-ab56-195f18943b5e
📒 Files selected for processing (2)
CHANGELOG.mdtests/wrap_unit.rs
Add comprehensive tests for issue #261 guarding that both fenced and indented shell code blocks remain byte-identical after applying wrap_text and the CLI's --wrap --in-place option. Refactor existing tests using rstest for parameterized testing to cover various edge cases, including blocks without trailing newlines and blocks following headings without blank lines. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/wrap_cli.rs`:
- Around line 9-23: The helper run_wrap_in_place_and_read_back currently panics
via expect() and lives outside test scope; change it to return Result<String,
Box<dyn std::error::Error>> and replace NamedTempFile::new().expect(...),
fs::write(...).expect(...), Command::cargo_bin(...).expect(...), and
fs::read_to_string(...).expect(...) with ? propagation, then update callers
(e.g., cli_wrap_in_place_preserves_shell_block_verbatim,
cli_wrap_in_place_preserves_fenced_block_without_final_newline) to return
Result<(), Box<dyn std::error::Error>> and consume the helper with ? and return
Ok(()) after assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 635f1159-f19e-471f-888a-70d87fe929e8
📒 Files selected for processing (2)
tests/wrap_cli.rstests/wrap_unit.rs
Owner
Author
|
@coderabbitai Have the following now been resolved? Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 7662
None of the original suggestions involved en-GB-oxendict spelling replacements or cyclomatic-complexity increases, so nothing needed annulment. |
Refactored the tests in wrap_cli.rs to return Result<(), Box<dyn std::error::Error>> instead of panicking on errors. This improves error propagation and integrates better with Rust's test framework, making tests more robust and idiomatic. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Core Functionality
Tests
--wrap --in-placeleaves fenced and indented shell blocks verbatim after a heading.wrap_text_preserves_fenced_shell_block_after_headingandwrap_text_preserves_indented_shell_block_after_headingverifying that wrapping at 80 columns does not modify these blocks.Test plan
cargo test.cargo test --tests(or run the mdtablefix CLI regression suite).◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/5d86b2fd-6326-4240-a4a0-346e3af9237c
📝 Closes #261
Summary by Sourcery
Add regression coverage to ensure Markdown wrapping preserves verbatim code blocks following headings.
Tests:
mdtablefix --wrap --in-placeleaves fenced and indented shell code blocks byte-identical after a heading.