Fix word wrap misalignment at the end of the document (#948)

[Xiao Di (xiaoditx)]

Problem

Fixes #948. With word wrap enabled, the cursor and line-number margin drift on the last line of a document: an extra empty row appears below the text. Moving into a word on the last line can trigger it; repeated Left/Right adds more rows, and debug builds may hit:

assertion failed: cursor.visual_pos.y <= self.stats.visual_lines

Root cause

The word-wrap lookahead in measure_forward() did not stop at the end of the text, so it re-measured the last cluster until it falsely reported a wrap. It also moved words wider than a row as whole words, although hard wrap already handled them. Forward cursor movement could also measure from the cursor's current wrapped position instead of the line start, so the same logical position could map to different visual positions.

Changes

• measurement.rs: stop lookahead at end of text; only wrap whole words that fit on a row.
• • buffer/mod.rs: re-anchor forward cursor moves to line start when word wrap is enabled; count the extra row render() uses when the cursor sits on the wrap column at end of document; update line stats before validating the cursor in edit_end().
• • Tests: width sweep 2..14, idempotent Left/Right, lookahead edge cases, extra-row bookkeeping.
• • No new dependencies.

Verification

• cargo test --workspace all green.
• • cargo fmt --all -- --check no diff for changed files.
• • cargo clippy -p edit --all-targets no new warnings.

Notes

I filed this issue a few weeks ago; since it had not been picked up, I fixed it with AI assistance and manually reviewed the changes. My English is not very good, and I left many comments mostly unchanged, so some may be inaccurate or verbose. Please point out anything that should be corrected during review.

[Xiao Di (xiaoditx)]

@microsoft-github-policy-service agree

[user77 (hexbinoct)]

Thanks for digging into this. I checked the branch out and tested it against main (b767559).

The new tests do catch the bug: with only the tests applied on top of main, all 7 fail, and word_wrap_cursor_stays_on_counted_rows stops on the same cursor.visual_pos.y <= self.stats.visual_lines assertion as in #948. On this branch all 52 lib tests pass.

I also looked at what actually ends up on screen: I rendered the buffer into a Framebuffer, then moved the cursor through every position of the line and compared where the cursor is drawn with the drawn text (release build, since main trips the debug assert). For normal words this branch fixes it. With the quick brown fox at width 8, main draws the cursor for the last two positions on an empty row below fox, and here every position lines up.

For a word that is wider than the row, the cursor still ends up away from its text, and that also happens when the word is not on the last line:

"hello world" at width 4, this branch
|hell|
|o wo|
|rld |
cursor after 'r' is drawn at row 2 col 3, next to 'd'
cursor after 'l' is drawn at row 2 col 4
cursor after 'd' is drawn at row 3 col 1

With "hello world\nnext" the buffer also counts 6 visual lines where 5 are needed (the 4 drawn rows plus the extra row for the cursor after next). Main gets these cursor positions wrong as well, so it isn't new, but test_lookahead_word_wider_than_row expects the misplaced positions (for x=7 it expects {2, 3}, while the drawn row 3 is co).

The interesting part is that your buffer/mod.rs changes (seeking forward from the line start) already fix these on their own. I kept buffer/mod.rs exactly as it is here and reduced the measure_forward() change to just the end of text stop, the at_end_of_text flag without the word_width rule. With that, every case above has the cursor next to its text, and 51 of the 52 tests pass. The one that fails is test_lookahead_word_wider_than_row, since it expects the wide word to be hard wrapped where it is. So dropping the word_width part might be worth it, unless you had a case in mind that needs it.

A smaller note: seeking forward from the line start makes Right cost about as much as Left already does on a long wrapped line. Time per keypress with the cursor near the end of one long line, width 80, release build, two runs:

line length	Right, main	Right, this PR	Left, main
10 KB	0.06 µs	0.14 to 0.21 ms	0.14 ms
100 KB	0.07 µs	1.6 to 1.9 ms	1.4 to 1.5 ms
1 MB	0.07 µs	17 to 19 ms	15 to 16 ms

That seems fine for normal text, I only mention it because it changes what one key costs.

Happy to share the probe code if it helps.

[Xiao Di (xiaoditx)]

user77 (@hexbinoct) Thanks for the thorough investigation! I've dropped the word_width rule from measure_forward() and updated the test expectations. All 52 tests pass locally now.

The Right key performance hit on long lines seems acceptable for typical text, so I'll leave it for now. Appreciate your help!

[user77 (hexbinoct)]

Thanks for the update. I reran everything on 0ae22ba: all 52 lib tests pass, clippy is clean, and the rendered check now has every cursor position next to its text in all the cases from before, the wide word ones included.

One thing changed along the way. The wrap column check moved from inside the lookahead loop to after it, so the loop now always reads to the end of the word. For a word much longer than a row, that makes every key cost the length of the word while the cursor is on the row where the word starts. Text ab followed by one long run of x, cursor at x=5, width 80, release build, time per Right keypress over two runs (Left shows the same pattern):

word length	main	this PR	check kept inside the loop
1 KB	0.5 to 0.9 µs	15 to 20 µs	1.1 to 1.8 µs
10 KB	0.5 µs	0.16 to 0.2 ms	1.1 µs
100 KB	0.5 to 0.6 µs	1.6 to 1.7 ms	1.2 to 1.7 µs
1 MB	0.5 to 0.7 µs	13 to 18 ms	1.2 to 1.6 µs

Typing there is slower than main as well (1 MB: 58 to 83 ms per character, main 15 to 23 ms).

Keeping the check inside the loop, the way main has it, gives the same results: 52/52 with your updated test expectations, and the rendered rows are identical to this commit for every case I have. Since the lookahead position only grows, breaking as soon as it passes the column can't change the answer. It also shrinks the measurement.rs change to just the at_end_of_text stop. Suggestion inline.

[user77 (hexbinoct)]

Same result without reading the rest of a long word. This also drops the comment about words that don't need scanning to their end, which no longer matches the code.

Suggested change

	// Words that can't move to the next row as a whole don't need to be
	// scanned to their end either.
	if at_end_of_text
	|| !ucd_line_break_joins(props_current_cluster, props_next_cluster)
	{
	// The word ends here.
	break;
	}
	}
	// A word that would cross the word wrap column is moved to the next row
	// as a whole, even if the word itself is wider than a row.
	if visual_pos_x_lookahead > self.word_wrap_column {
	visual_pos_x -= wrap_opp_visual_pos_x;
	visual_pos_y += 1;
	}
	if visual_pos_x_lookahead > self.word_wrap_column {
	visual_pos_x -= wrap_opp_visual_pos_x;
	visual_pos_y += 1;
	break;
	} else if at_end_of_text
	|| !ucd_line_break_joins(props_current_cluster, props_next_cluster)
	{
	break;
	}
	}

[Xiao Di (xiaoditx)]

user77 (@hexbinoct) Thanks for the suggestion, and sorry for the confusion — I only saw your comment initially and missed the inline code suggestion, so I edited it myself and squashed the commits into one. Applying your suggestion directly would likely conflict, so I adapted it based on your description. The word_width logic is removed, the check is back inside the loop, and test_lookahead_word_wider_than_row is updated to match the actual layout. All 52 tests pass and clippy is clean.

[user77 (hexbinoct)]

No worries about the squash, that's fine. I reran everything on f6d58f5: all 52 lib tests pass, clippy and fmt are clean, and with only the new tests on top of main, 6 of them fail, including word_wrap_cursor_stays_on_counted_rows on the same assert as in #948. In the rendered check every cursor position is next to its text in all 9 cases (main has misplaced positions in 7 of them).

The long word case is back to main's speed. Right with the cursor on the row where a long word starts, width 80, release build, two runs:

word length	main	0ae22ba	f6d58f5
1 KB	0.49 µs	15 to 20 µs	1.1 to 1.3 µs
1 MB	0.49 to 0.51 µs	13 to 18 ms	1.1 µs

Typing at the start of the 1 MB word is 15.4 ms per character here, main 15.1 to 15.2 ms.

One small thing, in a comment only. The expected positions in test_lookahead_word_wider_than_row are right (they match what gets drawn, and the test passes on main too), but the diagram above them doesn't match. What is drawn at width 2 is:

|te|
|xt|
|  |
|`c|
|od|
|e`|

So the word does move to the next row and is then hard wrapped there. Suggestion inline. Looks good to me otherwise.

Main's #970 changed the same None => break lines in measurement.rs (comments only), so this needs a rebase now. Happy to rerun the checks on the rebased head.

[Xiao Di (xiaoditx)]

user77 (@hexbinoct) All done — rebased onto latest main (with #970) and squashed into one commit. Also took your diagram suggestion, so the comment now matches what's actually drawn. Ready for you to rerun the checks whenever you get a chance. Thanks!

[user77 (hexbinoct)]

Thanks, I reran everything on 4f29008. Apart from the rebase, the only change from f6d58f5 is the diagram, and the conflict with #970 is resolved as None => { at_end_of_text = true; break; // End of document }, which keeps its comment.

All 72 lib tests pass, clippy and fmt are clean, and with only the new tests on top of main (25fcee5), 6 of them fail, including word_wrap_cursor_stays_on_counted_rows on the #948 assert. In the rendered check every cursor position is next to its text in all 9 cases (main has misplaced positions in 7 of them).

Since #970 lets a chunk end inside a grapheme cluster, I also checked that at_end_of_text is only set at the real end of the text. The lookahead refills chunk_iter before it calls next(), so None still means read_forward returned an empty slice. To confirm it, I compared every cursor position on texts cut into 2 or 3 chunks at every codepoint boundary (CRLF and combining marks included) with the same text in one chunk, for wrap columns 2 to 8:

chunk probe: 371028 cursor queries checked, 0 differ from the single-chunk text

The same check on a copy that treats the end of a chunk as the end of the text finds 1006 differences, so it does catch that.

Right at the start of a 1 MB word is 1.2 to 2.0 µs (main 0.5 µs), the same as before the rebase. Typing there took 15.4 and 20.0 ms per character in two runs, against 15.2 and 15.6 ms on main.

Looks good to me.

[Xiao Di (xiaoditx)]

Thanks! ^o^