Skip to content

fix: escape pipe and backslash in Markdown/Notion tables (#16)#44

Merged
cmeans merged 3 commits into
mainfrom
fix/markdown-pipe-escaping
Apr 12, 2026
Merged

fix: escape pipe and backslash in Markdown/Notion tables (#16)#44
cmeans merged 3 commits into
mainfrom
fix/markdown-pipe-escaping

Conversation

@cmeans-claude-dev
Copy link
Copy Markdown
Contributor

Summary

  • Add _escape_md_cell() helper that escapes \ to \\ then | to \|
  • Apply escaping in _format_markdown() before width computation so column alignment is preserved
  • Notion routes through _format_markdown, so it gets the fix automatically
  • Add 7 tests covering pipe, double-pipe, backslash, backslash-pipe, leading/trailing pipe, and Notion equivalence

Part of the escaping series (#17, #18). Includes design spec for the full series.

Test plan

  • uv run pytest tests/test_parser.py -v passes all 68 tests (7 new + 61 existing)
  • Verify a cell containing | produces \| in markdown output
  • Verify column alignment is correct with escaped values

Closes #16

cmeans-claude-dev[bot] and others added 3 commits April 11, 2026 22:38
Cells containing | or \ now produce valid GFM pipe tables.
Backslash is escaped first to avoid ambiguity. Column widths
are computed from escaped values so alignment is preserved.

Note: test spec had delimiters==4 for a 2-column table (which
yields 3 structural pipes, not 4). Corrected to delimiters==3
to match actual GFM pipe table structure.

Closes #16

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move import re to module level in tests
- Add string-containment assertions for leading/trailing pipe test
- Add CHANGELOG entry

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 12, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cmeans cmeans added QA Active QA is actively reviewing; Dev should not push changes and removed Ready for QA Dev work complete — QA can begin review labels Apr 12, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Review — PR #44

Verdict: Zero findings. Ready for maintainer signoff.

Code review

_escape_md_cell() (parser.py:359): Escape order is correct — backslash first (\\\), then pipe (|\|). Reversing would double-escape the backslash introduced by pipe escaping. ✓

Width computation (parser.py:373–377): Column widths computed from escaped values, not raw. This preserves table alignment when escaped characters change the visual width. Subtle and correct. ✓

Notion routing: format_table() dispatches notion to _format_markdown() (line 352), so Notion automatically inherits the fix. Confirmed by test_format_notion_escapes_pipe. ✓

Tests

Full suite: 233 passed, 0 failed, 0 deselected (226 base + 7 new).
Parser tests: 68 passed (61 existing + 7 new).

New tests (all PASS):

  1. test_format_markdown_escapes_pipe_in_data — structural integrity via unescaped-pipe counting regex
  2. test_format_markdown_escapes_pipe_in_header — pipe in header cell
  3. test_format_markdown_escapes_double_pipe|| doesn't create extra columns
  4. test_format_markdown_escapes_backslash — lone backslash escaped
  5. test_format_markdown_escapes_backslash_pipe\|\\\| (escaped backslash + escaped pipe)
  6. test_format_markdown_leading_trailing_pipe — edge case: leading/trailing pipes
  7. test_format_notion_escapes_pipe — Notion equivalence confirmed

Tests verify structural integrity (unescaped pipe counts), specific escaped values, and edge cases. Covers the bug, not just the fix mechanics. ✓

Manual verification

| Cmd         | Desc   |
| ----------- | ------ |
| cat \| grep | filter |

Column alignment correct: 11-char separator matches cat \| grep width. \| appears in output, not a raw pipe. ✓

Other checks

  • CHANGELOG: ### Fixed entry under [Unreleased]. Correct category, accurate description, Closes #16. ✓
  • Design spec (docs/superpowers/specs/2026-04-11-table-formatter-escaping-design.md): Accurate coverage of the 3-PR series (#16, #17, #18). Correctly scopes #15 out (potential contributor). ✓
  • CI: test (3.11/3.12/3.13), codecov/patch all SUCCESS. QA Gate pending. ✓

Merge coordination note (not a finding)

PR #44 and #45 both add ### Fixed to CHANGELOG but at different positions (before vs after ### Changed). Git will auto-merge both without conflict, creating two ### Fixed sections. Recommend: merge #44 first (it carries the design spec), then rebase #45 to consolidate its Fixed entry into the existing section. Adding merge-order labels to make this explicit.

Findings

None.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 12, 2026

Applying Ready for QA Signoff. 233/233 tests pass (7 new), manual verification confirms correct escaping and column alignment. Zero findings. Recommend merging this before #45 (both add CHANGELOG Fixed sections — second PR needs rebase to consolidate).

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge merge-order: 1 Merge first in the current batch and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 12, 2026
@cmeans cmeans added QA Approved Manual QA testing completed and passed and removed Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 12, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

LGTM

@cmeans cmeans merged commit 455fe90 into main Apr 12, 2026
37 checks passed
@cmeans cmeans deleted the fix/markdown-pipe-escaping branch April 12, 2026 04:37
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 12, 2026
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-order: 1 Merge first in the current batch QA Approved Manual QA testing completed and passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Markdown/Notion table format does not escape pipe characters

1 participant