fix: break over-long words in one pass instead of re-queuing the tail - #26
Open
BrianWillows wants to merge 1 commit into
Open
BrianWillows wants to merge 1 commit into
BrianWillows wants to merge 1 commit into
Conversation
When a word is longer than the wrap width, the loop split off one line-sized piece and pushed the remainder back onto the queue. Each pass then spread the whole remainder into an array ([...word]), measured it with wcwidth(word), and sliced/joined it again, so a word of n characters cost O(n) work per ~width characters emitted - quadratic overall. A single long token (a URL, hash or base64 blob) in otherwise ordinary text is enough to trigger it: chars size before after 4,000 4KB 16ms 2ms 32,000 31KB 1,267ms 5ms 128,000 125KB 24,637ms 14ms Narrower widths are worse, so an 80-column terminal sits in the bad part of the curve: a 60KB URL at width 80 took ~1.4s. Break the word into all its chunks in one pass instead. Output is unchanged: verified byte-for-byte identical against 2,405 inputs covering CJK wide characters, tabs, width-1 through width-80, and long words, plus the existing 25 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
Nudge on this one - still merges cleanly against master as of today, CI green. It replaces the re-queue-the-tail loop with a single pass, so wrapping one long token stops being quadratic. Existing tests pass unchanged. Small enough to read in a minute; glad to adjust the style to match the rest of the file if it's off. |
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
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.
Summary
When a word is longer than the wrap width, the loop splits off one line-sized piece
and pushes the remainder back onto the queue:
Every pass then spreads the whole remainder (
[...word]), measures it(
wcwidth(word)at the top of the loop), and slices/joins it again — so emitting~
widthcharacters costs O(remaining). For a word of n characters that isO(n²).
A single long token in otherwise ordinary text is enough to trigger it — a URL, a
hash, a base64 blob:
Narrower widths mean more iterations, so an ordinary 80-column terminal is in the
bad part of the curve: a 60 KB URL embedded in normal prose took ~1.4 s at
width: 80.Fix
Break the over-long word into all of its chunks in a single pass, rather than
re-queuing the tail and re-scanning it each time. The chunk boundaries are computed
the same way (accumulate
wcwidthuntil the next character would exceedlineLength), so the output is unchanged.words = chunks.concat(words)rather thanunshift(...chunks)— a long word canproduce more chunks than the argument limit allows to be spread.
This removes the last use of
breakword, so the now-unused import is dropped. Happyto leave the dependency in
package.jsonor remove it there too, whichever youprefer.
Verification
generated inputs — CJK wide characters, tabs, mixed alphabets, widths 1/5/20/80,
and explicit long-word cases (
'a'.repeat(500)at width 20,'日'.repeat(60)atwidth 8, single characters wider than the line, etc.).
config.errorCharhandling.npm run lintoutput is unchanged apart from being one line shorter (thepre-existing
no-fallthrougherror at the deliberate fall-through remains).Notes
Found and fixed with AI assistance (Claude). Happy to add a regression test that
asserts a long word wraps in bounded time if you'd like one.