Skip to content

fix(tui): trim trailing whitespace in wrapped lines to prevent width overflow#457

Closed
robinwander wants to merge 1 commit intobadlogic:mainfrom
robinwander:fix/wrap-trailing-whitespace-overflow
Closed

fix(tui): trim trailing whitespace in wrapped lines to prevent width overflow#457
robinwander wants to merge 1 commit intobadlogic:mainfrom
robinwander:fix/wrap-trailing-whitespace-overflow

Conversation

@robinwander
Copy link
Copy Markdown
Contributor

@robinwander robinwander commented Jan 5, 2026

Bug

When you paste text with content that triggers Markdown.render (``` ````, >, ``) (bold and italics seem fine though) which also has trailing whitespace that exceeds the width of the terminal, it causes pi to crash with the following error:

Error: Rendered line 1855 exceeds terminal width.

This happened to me a lot when copy-pasting from another terminal (so it had a lot of trailing white space on every line as well as markdown content).

Root cause / fix

It took me awhile to figure out why I could only repro sometimes even though in this code it seemed liked the bug was clear. But we are actually protected in most scenarios due to Editor.wordWrapLine trimming.

And for non whitespace characters we are protected by breakLongWord .

So this bug only happens for specific cases when you paste with content that goes through Markdown.render and has white space beyond the width of terminal you are pasting into.

The wrapping logic in wrapTextWithAnsi intentionally skips breaking whitespace-only tokens to avoid generating lines of individual spaces. But this means if you paste something like 120 spaces into a 100-character-wide terminal, the line passes through unchanged and exceeds the width limit.

It seems like a safe enough fix is to trimEnd() all wrapped lines before returning, but I could be missing something here. But it looks like trailing whitespace doesn't need to be preserved since padding is added later by the rendering layer. There was already a trimEnd() call at one wrap point in the code, so this just consolidates the behavior to catch all cases.

Testing

Added two tests: one for a minimal repro of the original crash case, and one to guard the existing behavior where trailing whitespace must be trimmed before adding ANSI underline reset codes (otherwise the reset code ends up after the whitespace instead of immediately after the content).

I also manually repro'ed with , >, and '' and confirmed they were fixed with this change by running pi built from this branch's source.

assert.ok(wrapped[1].includes("https://"));
});

it("should not have whitespace before underline reset code", () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can remove this test if you think its too much, but i initially refactored to move all trimming of lines to the end, but this causes a bug due to the underline reset, and thought it could be nice to have this covered with a test


return wrapped.length > 0 ? wrapped : [""];
// Trailing whitespace can cause lines to exceed the requested width
return wrapped.length > 0 ? wrapped.map((line) => line.trimEnd()) : [""];
Copy link
Copy Markdown
Contributor Author

@robinwander robinwander Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opus and I couldn't really think of a scenario where it would be meaningful to try to preserve as much white space as could fit in the requested width, so just moved to trimming all lines but I definitely could be missing something here.

}
});

it("should truncate trailing whitespace that exceeds width", () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verified this test case breaks without the fix.

@badlogic
Copy link
Copy Markdown
Owner

badlogic commented Jan 5, 2026

Ouch, i'll take a looksy tomorrow!

@badlogic badlogic closed this in 817221b Jan 5, 2026
@badlogic
Copy link
Copy Markdown
Owner

badlogic commented Jan 5, 2026

Merged into main with changelog entry. Thanks for the fix!

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