fix: break words longer than twice the wrap width [patch] - #118
Merged
Merged
Conversation
TextElement.WrapText sliced exactly one maxWidth-sized chunk off a word that exceeded the wrap width and buffered the rest in currentLine, where it was only ever re-tested against the next word. A word longer than 2x maxWidth therefore left an oversized remainder that was appended to the output as a single long line, so OnRender wrote past the element's content area into adjacent UI and OnCalculateRequiredDimensions reported a width wider than the one requested. Keep slicing the remainder until what is left fits. The same path also covers the case where a short word is already buffered when the overlong word arrives: the buffer is flushed first, then the long word is broken rather than carried whole into the next line. Covered by three render tests: a word longer than 2x the width, an overlong word following a short one, and a word that divides exactly into the width (which must not emit a trailing empty line). Fixes #114 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UN7rvv6XDzL6zW4J8NYAhj
MSTEST0037: Assert.HasCount is the intended assertion for a collection count, and it matches the MSTest 4 style the rest of the file already uses (Assert.IsGreaterThan, Assert.ContainsSingle). Flagged by SonarCloud on #118. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UN7rvv6XDzL6zW4J8NYAhj
|
matt-edmondson
pushed a commit
that referenced
this pull request
Sep 16, 2026
#113 (for #109) and #118 (for #114) landed on main while this was open. UIApplication.cs merged cleanly: #113 rewrote Render's dirty-tracking model, which this branch does not touch, and this branch's changes are confined to RunAsync's interrupt registration and ProcessInputAsync's read. UIApplicationTests.cs conflicted because both branches filled what had been an empty file, with two unrelated sets of tests. Neither side is redundant, so main's render-loop tests stay in UIApplicationTests.cs unchanged and this branch's run-lifecycle tests move to UIApplicationLifecycleTests.cs. That matches how the suite already splits BorderElementTests from BorderElementRenderTests, and keeps either class readable on its own. 129/129 tests pass and the solution builds clean across net10.0;net9.0;net8.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01671tcpA4zkfbgPcJm8cTsB
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
2 participants
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 #114
The bug
TextElement.WrapTextsliced exactly onemaxWidth-sized chunk off a word that exceeded the wrap width and buffered the rest incurrentLine— but that remainder was never re-sliced. It was only re-examined against the next word, so a word longer than2 * maxWidthended up appended to the output as a single oversized line.WrapText("abcdefghij", 3)returned["abc", "defghij"]: a 7-character line in a 3-column box.OnRenderthen wrote that 7-character string into a 3-column area, overflowing into adjacent UI, andOnCalculateRequiredDimensionsreported amaxWidthwider than the content width it was asked for.A second case had the same root cause: when a short word is already buffered and an overlong word arrives, the old code flushed the buffer and assigned the long word to
currentLinewhole, never breaking it at all.WrapText("ab abcdefghij", 3)returned["ab", "abcdefghij"].The fix
Keep slicing the remainder in a loop until what is left actually fits, and flush any buffered line before starting to break the long word — which handles both cases through one path.
Tests
Three render tests in
TextElementTests, usingRecordingConsoleProviderper the repo's render-test convention:WordWrapFullyBreaksAWordLongerThanTwiceTheWidth— a 10-character word at width 3; asserts every line is<= 3and that no characters are dropped or reorderedWordWrapBreaksAnOverlongWordThatFollowsAShortOne— covers the buffered-line pathWordWrapLeavesNoEmptyLineWhenAWordDividesExactly— regression guard so the new loop doesn't emit a trailing empty line when the word length is an exact multiple of the widthVerified by reverting the fix and re-running: the first two fail with
[abc, defghij]and[ab, abcdefghij]respectively, matching the issue's failure scenario exactly. With the fix, the full suite is 116/116 green and the build is clean with 0 warnings.The pre-existing
WordWrapSplitsTextAcrossLinestest did not catch this because it only uses words shorter than 2x the width.Acceptance criteria
Met.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UN7rvv6XDzL6zW4J8NYAhj
Generated by Claude Code