simplify: unify stepRichInlineLine/Stats and remove containsCJKText wrapper#138
Open
shaun0927 wants to merge 1 commit intochenglou:mainfrom
Open
simplify: unify stepRichInlineLine/Stats and remove containsCJKText wrapper#138shaun0927 wants to merge 1 commit intochenglou:mainfrom
shaun0927 wants to merge 1 commit intochenglou:mainfrom
Conversation
…rapper Remove stepRichInlineLineStats, a ~130-line near-duplicate of stepRichInlineLine. The only difference was whether the optional collectFragment callback fired — passing undefined achieves the same result. PR chenglou#132 demonstrated the drift risk by having to apply the same overflow guard to both functions in parallel. Also inline the single-call-site containsCJKText() wrapper, which was a passthrough to the already-public isCJK(). Similar to the isCJK dedup in PR chenglou#118. Neither change affects behavior. All 84 tests pass.
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.
Fixes #136
Changes
Two small simplifications, bundled together since they are each one-liners with no cross-dependencies. Neither changes behavior — all 84 tests pass.
1. Remove
stepRichInlineLineStats(~130 lines deleted)stepRichInlineLineStatswas a near-duplicate ofstepRichInlineLinewhose only difference was skipping the optionalcollectFragmentcallback. Since the callback is already optional (called viacollectFragment?.()), passingundefinedachieves the same result.Why this matters: PR #132 had to apply the same overflow guard to both functions in parallel — the exact duplication pattern that makes one-sided patches easy to miss. Unifying the two functions ensures future fixes apply to both the materializing and stats-only paths automatically.
2. Inline
containsCJKText()wrapper (4 lines deleted)containsCJKText()was a single-call-site passthrough to the already-publicisCJK(). Replaced the call at line 1210 withisCJK(text)directly. Similar to theisCJKdedup in PR #118.Test plan
bun test src/layout.test.ts— 84 tests pass, 0 failmeasureRichInlineStatsstill returns identical results towalkRichInlineLineRangesacross all existing test cases