fix: escape pipe and backslash in Jira/Confluence tables (#17)#45
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
cmeans
left a comment
There was a problem hiding this comment.
QA Review — PR #45
Verdict: Zero findings. Ready for maintainer signoff.
Code review
_escape_jira_cell() (parser.py:428): Same escape order as the markdown helper — backslash first, then pipe. Correct. ✓
Header and data rows: Both header (|| delimited) and data (| delimited) rows use escaped values. The escaped array replaces normalized throughout. ✓
Confluence routing: format_table() dispatches confluence to _format_jira() (test_format_confluence_same_as_jira confirms). ✓
Double-pipe safety: A cell containing || becomes \|\|, which Jira renders as literal || in a data cell rather than interpreting it as header-row syntax. Confirmed by test_format_jira_escapes_double_pipe. This is the key Jira-specific edge case. ✓
Tests
Full suite: 232 passed, 0 failed, 0 deselected (226 base + 6 new).
Parser tests: 67 passed (61 existing + 6 new).
New tests (all PASS):
test_format_jira_escapes_pipe_in_data— pipe in data cell, structural checktest_format_jira_escapes_pipe_in_header— pipe in header celltest_format_jira_escapes_double_pipe—||doesn't create accidental header markuptest_format_jira_escapes_backslash— lone backslash escapedtest_format_jira_leading_trailing_pipe— edge case: leading/trailing pipestest_format_confluence_escapes_same_as_jira— Confluence equivalence confirmed
Manual verification
||Cmd||Desc||
|cat \| grep|filter|
Pipe in data cell correctly escaped. || in data cell: |a\|\|b|c| — no accidental header promotion. ✓
Other checks
- CHANGELOG:
### Fixedentry under[Unreleased]. Correct category, accurate description,Closes #17. ✓ - CI:
test (3.11/3.12/3.13),codecov/patchall SUCCESS.QA Gatepending. ✓
Merge coordination note (not a finding)
This PR should merge after #44 (which carries the design spec for the series). After #44 merges, rebase this PR and move its ### Fixed CHANGELOG entry into the existing ### Fixed section (currently it's in a separate position and will create a duplicate section if auto-merged). Adding merge-order: 2 label.
Findings
None.
|
Applying |
Cells containing | or \ now produce valid Jira wiki markup. Prevents || in data cells from being misinterpreted as header syntax. Closes #17 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Strengthen double-pipe assertion to check exact escaped form - Add CHANGELOG entry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
87f416f to
3a8a52a
Compare
## Summary - New `tests/test_escaping.py` with parametrized test matrix - Covers 20 special character variants across all 8 output formats (160 structure tests + 40 round-trip tests) - HTML `<>&"` tests xfail pending #15 - Slack `*` and backtick tests xfail pending #19 - Newline-in-cell tests xfail for line-based formats (no escape exists) - Moves #18 CHANGELOG entry to Added section (tests are additions) Depends on #44 and #45 being merged first (branch includes both). ## Test plan - [ ] `uv run pytest tests/test_escaping.py -v` shows 189 passed, 0 skipped, 11 xfailed - [ ] `uv run pytest -q` full suite passes with 0 skipped Closes #18 --------- Co-authored-by: cmeans-claude-dev[bot] <3223881+cmeans-claude-dev[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
_escape_jira_cell()helper that escapes\to\\then|to\|_format_jira()to all cells (header and data rows)_format_jira, so it gets the fix automaticallyPart of the escaping series (#16, #18).
Test plan
uv run pytest tests/test_parser.py -vpasses all 67 tests (6 new + 61 existing)|produces\|in jira output||in a data cell does not create accidental header markupCloses #17