fix: keep whitespace inside fenced code blocks when normalizing results - #2529
Manohar Paturi (ManoharPaturi) wants to merge 1 commit into
Conversation
Sylvester Kaczmarek (sylvesterkaczmarek)
left a comment
Sylvester Kaczmarek (sylvesterkaczmarek)
left a comment
There was a problem hiding this comment.
There's still a boundary case here: a blank-line run immediately before a fence can escape the collapse. With para\n\n\n```..., flush_segment() leaves para\n\n, then the final "\n".join(out) adds another newline before the fence, so it stays at three newlines instead of two. Could you add an adjacent-fence case and avoid adding that extra separator?
MarkItDown._convert applied its whitespace cleanup (rstrip every line, collapse 3+ newlines to 2) to the entire converted document, which corrupted fenced code blocks: blank-line runs inside a fence collapsed and trailing spaces were stripped, changing code content. Track code fences while normalizing and leave their contents untouched. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
b069ae1 to
7e6ed5c
Compare
|
good catch, reproduced it exactly as you described (three newlines surviving before the fence). fixed by stripping the trailing newlines when a segment flushes because a fence follows, and keeping exactly one blank line between preceding prose and the fence. the EOF flush keeps the old behavior so trailing newlines at end of document are untouched. added the adjacent-fence case as a test, plus 2-and-4-newline variants. the whitespace suite is green and the only failure in the full run (test_speech_transcription) is the same on main, it needs a model download in this environment. |
Sylvester Kaczmarek (sylvesterkaczmarek)
left a comment
Sylvester Kaczmarek (sylvesterkaczmarek)
left a comment
There was a problem hiding this comment.
Rechecked 7e6ed5c. The fence boundary case is fixed: a prose segment is stripped of trailing newlines before the separator is added, so runs immediately before a fence collapse to exactly one blank line. The adjacent-fence regression covers the case I raised. No blocker from me.
|
thanks for the quick recheck and the thorough review, the boundary catch made the fix properly correct. the workflows still need a maintainer approval to run, so whenever someone with write access can green-light those, this should be ready to go. |
Fixes #2528.
the normalizer now tracks fenced regions (backtick and tilde fences) and skips line-level rstrip and newline-run collapsing inside them, so code blocks come through byte-exact while prose outside fences keeps the existing normalization.
tests cover blank-line runs inside fences, trailing spaces, tilde fences, and that outside-fence normalization still applies. all fail on main, pass here.