Pipe escaping fix, Unicode width handling, tests, ADR 0001 - #270
Conversation
…nuation rows - Protect leading empty continuation cells with a private marker before row splitting. - Re-escape literal pipe characters in continuation row cells to preserve boundaries. - Measure column widths using Unicode display width for proper alignment. - Ensure separator columns have minimum width of three dashes with correct alignment markers. - Apply ellipsis replacements before reflow to align by final rendered width. This change fixes malformed table regressions affecting continuation rows, escaped pipes, and ellipsis handling, maintaining deterministic and testable behavior as documented in ADR 0001. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideRefactors the Markdown table reflow pipeline to preserve continuation-row structure and escaped pipes while making width calculations Unicode-aware, adds focused helpers and tests around separator handling, and updates architecture/ADR/docs to describe the new behavior and keep examples consistent. Class diagram for updated reflow and table helpersclassDiagram
class reflow_module {
+parse_rows(trimmed_lines) Vec_Vec_String
+clean_rows(rows) Vec_Vec_String
+calculate_widths(rows, max_cols) Vec_usize
+format_rows(rows, widths, indent) Vec_String
+insert_separator(out_lines, sep_cells, indent) Vec_String
+detect_separator(sep_line, rows, max_cols) SeparatorDetection
-split_into_rows(cells) Vec_Vec_String
-protect_leading_empty_cells(line) String
-pad_cell_to_width(cell, width) String
}
class table_module {
+split_cells(line) Vec_String
+reflow_table(lines) Vec_String
+format_separator_cells(widths, sep_cells) Vec_String
-extract_indent_and_trim(lines) IndentAndTrimmed
-extract_separator_line(lines) Option_String
-rows_mismatched(rows, allow_short) bool
}
class process_module {
+process_stream_inner(lines, options) Vec_String
}
class frontmatter_module {
+split_frontmatter(lines) FrontmatterSplit
}
process_module --> frontmatter_module : uses
process_module --> table_module : calls reflow_table
table_module --> reflow_module : uses helpers
reflow_module --> table_module : uses split_cells
class UnicodeWidthStr {
+width(text) usize
}
reflow_module --> UnicodeWidthStr : calculates_display_widths
table_module --> UnicodeWidthStr : formats_separator_cells
Flow diagram for the updated table reflow pipelineflowchart TD
subgraph StreamProcessing
process_stream_inner["process_stream_inner"]
ellipsis_replacement["Apply ellipsis replacement (if enabled)"]
reflow_table["reflow_table"]
end
process_stream_inner --> ellipsis_replacement
ellipsis_replacement --> reflow_table
subgraph TableReflowStages
extract_indent_and_trim["extract_indent_and_trim"]
parse_rows["parse_rows"]
protect_leading_empty_cells["protect_leading_empty_cells"]
clean_rows["clean_rows"]
detect_separator["detect_separator"]
calculate_widths["calculate_widths"]
format_separator_cells["format_separator_cells"]
format_rows["format_rows"]
insert_separator["insert_separator"]
end
reflow_table --> extract_indent_and_trim
extract_indent_and_trim --> parse_rows
parse_rows --> protect_leading_empty_cells
protect_leading_empty_cells --> parse_rows
parse_rows --> clean_rows
clean_rows --> detect_separator
detect_separator --> calculate_widths
calculate_widths --> format_separator_cells
format_separator_cells --> format_rows
format_rows --> insert_separator
insert_separator --> formatted_table["Aligned table output"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34f89a5768
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| LEADING_EMPTY_CELL_MARKER.to_string() | ||
| } else { | ||
| cell | ||
| cell.replace('|', r"\|") |
There was a problem hiding this comment.
Preserve escaped pipes in emitted continuation cells
Re-escaping at protect_leading_empty_cells is only temporary here: split_cells unescapes \| back to | before formatting, so continuation rows with escaped pipes are emitted with a raw pipe character. In inputs like | | keep \| literal in continuation |, the output row becomes | ... | keep | literal ... |, which Markdown parsers read as an extra column delimiter and therefore still corrupts table structure. The fix needs to keep these pipes escaped in final output (or re-escape during row formatting), not only during intermediate parsing.
Useful? React with 👍 / 👎.
- Changed ADR title to full form 'Architecture Decision Record (ADR) 0001' in the ADR file and references. - Replaced 'CJK' abbreviation with full 'Chinese, Japanese, and Korean (CJK)' in architecture.md for clarity. - Updated a test in table_continuations.rs to use process_stream_opts with ellipsis option disabled, improving test accuracy. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
|
@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. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 696
Comment 1 still requires attention. The function at line 70 retains the name #[test]
fn process_stream_preserves_literal_ellipsis_in_table_cells_when_disabled() {
let opts = Opts {
ellipsis: false,
..Opts::default()
};
let input = lines_vec![
"| Module | Notes |",
"| ------ | --------------------------- |",
"| `foo` | preserves literal ... here |",
"| `bar` | and also ... in this cell |",
];
let expected = lines_vec![
"| Module | Notes |",
"| ------ | -------------------------- |",
"| `foo` | preserves literal ... here |",
"| `bar` | and also ... in this cell |",
];
assert_eq!(process_stream_opts(&input, &opts), expected);
}Adjust the |
Previously, literal pipe characters in table cells were not being escaped correctly when calculating widths and formatting output rows, causing misalignment and rendering issues. This change introduces functions to escape literal pipes and account for their width correctly when calculating cell widths and padding. Tests have been added and updated to verify escaped pipes are handled properly. Also fixed table continuation rendering to preserve escaped pipes in output. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
74178f3
into
fix-mdtable-markdownlint-errors-dp20i0
* feat(table-reflow): preserve table structure during reflow with continuation rows - Protect leading empty continuation cells with a private marker before row splitting. - Re-escape literal pipe characters in continuation row cells to preserve boundaries. - Measure column widths using Unicode display width for proper alignment. - Ensure separator columns have minimum width of three dashes with correct alignment markers. - Apply ellipsis replacements before reflow to align by final rendered width. This change fixes malformed table regressions affecting continuation rows, escaped pipes, and ellipsis handling, maintaining deterministic and testable behavior as documented in ADR 0001. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(adr): clarify ADR title and terminology; update ellipsis test - Changed ADR title to full form 'Architecture Decision Record (ADR) 0001' in the ADR file and references. - Replaced 'CJK' abbreviation with full 'Chinese, Japanese, and Korean (CJK)' in architecture.md for clarity. - Updated a test in table_continuations.rs to use process_stream_opts with ellipsis option disabled, improving test accuracy. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * fix(reflow): properly escape literal pipes in table cell formatting Previously, literal pipe characters in table cells were not being escaped correctly when calculating widths and formatting output rows, causing misalignment and rendering issues. This change introduces functions to escape literal pipes and account for their width correctly when calculating cell widths and padding. Tests have been added and updated to verify escaped pipes are handled properly. Also fixed table continuation rendering to preserve escaped pipes in output. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
* feat(table): preserve leading empty cells in continuation rows and add ellipsis support - Added support to preserve leading empty cells in table continuation rows by marking them during parsing and cleaning up markers before output. - Improved table reflow to calculate column widths using unicode widths and pad cells accordingly. - Added an option to replace ellipsis occurrences in tables during processing. - Introduced tests covering Markdown tables with continuation rows and ellipsis handling. - Ensured separator rows have a minimum column width for consistent formatting. These changes improve Markdown table formatting fidelity, especially for continued rows and truncated content representation. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * Pipe escaping fix, Unicode width handling, tests, ADR 0001 (#270) * feat(table-reflow): preserve table structure during reflow with continuation rows - Protect leading empty continuation cells with a private marker before row splitting. - Re-escape literal pipe characters in continuation row cells to preserve boundaries. - Measure column widths using Unicode display width for proper alignment. - Ensure separator columns have minimum width of three dashes with correct alignment markers. - Apply ellipsis replacements before reflow to align by final rendered width. This change fixes malformed table regressions affecting continuation rows, escaped pipes, and ellipsis handling, maintaining deterministic and testable behavior as documented in ADR 0001. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(adr): clarify ADR title and terminology; update ellipsis test - Changed ADR title to full form 'Architecture Decision Record (ADR) 0001' in the ADR file and references. - Replaced 'CJK' abbreviation with full 'Chinese, Japanese, and Korean (CJK)' in architecture.md for clarity. - Updated a test in table_continuations.rs to use process_stream_opts with ellipsis option disabled, improving test accuracy. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * fix(reflow): properly escape literal pipes in table cell formatting Previously, literal pipe characters in table cells were not being escaped correctly when calculating widths and formatting output rows, causing misalignment and rendering issues. This change introduces functions to escape literal pipes and account for their width correctly when calculating cell widths and padding. Tests have been added and updated to verify escaped pipes are handled properly. Also fixed table continuation rendering to preserve escaped pipes in output. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * feat(table-reflow): add advanced table reflow pipeline and user/developer guides - Implement a staged table reflow pipeline in `src/reflow.rs` supporting continuation rows, separator detection, Unicode width-based column sizing, escaping of literal pipes, and reinsertion of separator rows. - Add detailed internal API documentation and design rationale in `docs/developers-guide.md`. - Add a comprehensive user guide on table reflow and ellipsis handling in `docs/users-guide.md`. - Enhance process functions with examples for table reflow and ellipsis flag. - Improve formatting of separator cells preserving Markdown alignment markers in `src/table.rs`. - Fix minor markdown lint and formatting updates in execution plans documentation. - Include tests verifying escaping logic, width calculations, padding, and ellipsis transformations. This feature improves Markdown table formatting robustness, ensuring visually aligned columns with proper handling of Unicode and special characters, while documenting the implementation and usage thoroughly. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * feat(table/reflow): reject tables with invalid separator rows during reflow - Changed format_separator_cells to return empty vector if length mismatch, so invalid separator rows are detected. - Added handling in calculate_and_format to return None when separator formatting is invalid, preventing table reflow. - Made reflow_table return original input lines if separator row is invalid, preserving original table. - Refactored tests by moving them out of reflow.rs into reflow/tests.rs with expanded coverage. - Minor docs fix in ADR title case. This prevents malformed Markdown tables with invalid separator rows from being reflowed incorrectly, improving stability and correctness of the table reflow pipeline. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * feat(table): add regex to detect Markdown table separator lines Introduce `SEP_RE`, a static regular expression matching Markdown table separator lines composed only of pipes, colons, dashes, and whitespace. This enables accurate detection and extraction of the alignment row during table parsing. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Changes
Core logic
Table formatting
Tests
Documentation
Test plan
📎 Task: https://www.devboxer.com/task/eb7e6e3d-b663-4f6d-92ed-d23061ed6106