Fix FixInvalidItalicTags leaving some two-line stray-tag shapes unrepaired#12593
Merged
Conversation
…aired The two-line branches for "only four </i> tags" / "only four <i> tags" had an asymmetric condition (lines[1].EndsWith checked twice, lines[0]'s end never) - identical in SE 4, and load-bearing: shapes it matched were repaired to whole-line italics, but near-miss shapes (e.g. second line not starting with the tag, or all four tags on the first line) fell through and the function returned the invalid tags unchanged. Rather than tightening the condition (which would repair fewer shapes - see PR #12579 review), generalize it: under the count guard the stray tags carry no pairing information, so strip them from each line that contains one and wrap that line in proper <i>...</i>; tag-less lines are kept as-is. Output is byte-identical for every previously matched shape, and the fall-through shapes now come out as valid italics. Tests pin both the long-standing shapes and the newly repaired ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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.
Follow-up to the #12579 review, which showed that tightening the "duplicated" condition in the two-line stray-italic-tag branches was a regression. This PR fixes the underlying issue correctly.
The problem: the two-line branches for only four
</i>tags / only four<i>tags use an asymmetric condition (lines[1].EndsWithchecked twice,lines[0]'s end never checked — identical in SE 4). Shapes matching the loose condition are repaired to whole-line italics, but near-miss shapes fall through andFixInvalidItalicTagsreturns the invalid tags unchanged:</i>Hello</i>a</i>+World</i><i>Helloa</i>+<i>World</i>a</i>b</i>+c</i>d</i><i>ab</i>+<i>cd</i></i>Hello</i>b</i>c</i>+plain<i>Hellobc</i>+plain<i>)The fix: under these count guards (exactly four copies of a single tag, two lines) the stray tags carry no pairing information — so drop the positional condition entirely and repair per line: strip the stray tags from each line containing one and wrap that line in proper
<i>...</i>; lines without a tag are kept as-is (so a plain second line does not become italic). This matches the method's established heuristic for the single-stray-end-tag case (Foo.</i>→<i>Foo.</i>).Compatibility: output is byte-identical for every shape the old condition matched (verified against current main on all six such shapes) — this only turns previously-unfixed invalid output into valid italics.
Tests: 12 new theory cases pin both the long-standing shapes and the newly repaired ones. All 520 libse, 237 seconv, and 14 libuilogic tests pass.
🤖 Generated with Claude Code