Skip to content

fix: a word longer than the screen advances the wrap row by one - #1155

Merged
noborus merged 1 commit into
noborus:masterfrom
VXNCXNX:fix/wordwrap-long-word-row
Aug 17, 2026
Merged

fix: a word longer than the screen advances the wrap row by one#1155
noborus merged 1 commit into
noborus:masterfrom
VXNCXNX:fix/wordwrap-long-word-row

Conversation

@VXNCXNX

@VXNCXNX VXNCXNX commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

With --converter wordwrap, a word longer than the screen width advances the
internal row counter by exactly one, even though the word occupies several rows.
Every wrap decision after it is then made against a stale row, and the space
before the next word is lost.

40-column terminal, ov --converter wordwrap -X -F on a file whose second word
is 88 characters:

before:
aa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
bbbcc dd ee ff gg hh ii jj kk ll mm nn
oo pp qq rr ss tt uu vv

after:
aa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
bbb cc dd ee ff gg hh ii jj kk ll mm nn
oo pp qq rr ss tt uu vv

bbbcc on line 3 is the symptom: cc is glued onto the tail of the long word.

Cause

processWord handles the oversized case by appending the word and bumping the
row by one:

if len(srcWord) > proc.screenWidth {
    proc.dst = append(proc.dst, srcWord...)
    proc.row++
    return
}

The next word's fit test is len(proc.dst)+len(srcWord) <= proc.screenWidth*proc.row.
After an 88-cell word on a 40-cell screen, dst is 91 cells but row is 2, so
the budget is 80 and the test fails on a word that should have fit. finishLine
then computes negative padding and adds nothing, so the row boundary is never
written and the words run together.

The fix

Derive the row from the output length rather than incrementing it, since that is
the row the word actually ends on:

proc.row = (len(proc.dst) / proc.screenWidth) + 1

For a word that spans a single row this is identical to proc.row++, so the
common path is unchanged.

Verification

Two cases added to TestConvertWordwrap. Reverting only that line fails the
first as an assertion, not a build error:

--- FAIL: TestConvertWordwrap/long_word_spanning_multiple_rows_then_wrap_to_a_new_row
    convert_wordwrap_test.go:128: expected string "aa bbbbbbbbbbbbbbbbbbbbbbbbb  cc", got "aa bbbbbbbbbbbbbbbbbbbbbbbbbcc"

The second case passes both before and after by design: it pins the behaviour
when a long word is followed by a word that genuinely does not fit, so the fix
cannot over-correct in the other direction.

go test ./... is ok across all packages. gofumpt -l flags only pre-existing
files (draw.go, move_updown.go), and golangci-lint reports 9 pre-existing
issues, none in convert_wordwrap.go.

Disclosure: written with AI assistance (Claude Code). I built binaries from HEAD and from the patched tree, produced the output above in a real 40-column pty, and ran the mutation check myself.

@noborus

noborus commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Thank you.
This is a bug. I'll merge it.

@noborus
noborus merged commit 949d0ca into noborus:master Aug 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants