Skip to content

Remove wordwrap for links so they don't break (on mobile and smaller - #64

Closed
unkrich wants to merge 1 commit into
rootvc:mainfrom
unkrich:wordwrap
Closed

unkrich wants to merge 1 commit into
rootvc:mainfrom
unkrich:wordwrap

Conversation

@unkrich

@unkrich unkrich commented Sep 22, 2022

Copy link
Copy Markdown
Contributor

screen sizes)

The current implementation of wordwrap is only used in the case of a link. At which point it checks against a maxWidth of 76 and breaks the link, and/or inserts a newline character if whitespace is found.

Introducing a newline character breaks the link and is seemingly unnecessary (may have to do with addonFit).

Introducing a newline on whitespace character is also seemingly unecessary as a URL should not have a whitespace character anywhere but possibly the end of the string.

Possible fix for #38 and #42 (the latter issue is less clear).

One possible oversight is that wordwrap may be useful for the case where a string with multiple URLs passed in separated by a whitespace character that should actually be a newline is missed. This doesn't appear to be a problem.

screen sizes)

The current implementation of wordwrap is only used in the case of a
link. At which point it checks against a maxWidth of 76 and breaks the
link, and/or inserts a newline character if whitespace is found.

Introducing a newline character breaks the link and is seemingly unnecessary
(may have to do with addonFit).

Introducing a newline on whitespace character is also seemingly
unecessary as a URL should not have a whitespace character anywhere but
possibly the end of the string.
@ledwards

Copy link
Copy Markdown
Contributor

Closing, but the bug you reported did get fixed — just more narrowly than this patch.

js/terminal-ext.js now carries a guard right where the wrapping happens:

allowWrapping = match[0].length < 76; // don't wrap lines that contain long URLs

So a line containing a long URL skips _wordWrap entirely, which is the case that was breaking links on mobile. For shorter URLs, _wordWrap breaks at whitespace, so the URL moves to the next line intact rather than being split.

This PR removes _wordWrap and every call to it, which changes how all terminal output renders on narrow viewports, not just lines with links. That's a much larger behavioral change than the reported problem needs, and the targeted guard already covers it.

One edge case is still unhandled: a URL under 76 characters on a line with no whitespace before the wrap column would still get force-split mid-URL. If you ever hit that in practice, it's worth a small follow-up — but it isn't a reason to drop wrapping wholesale.

@ledwards ledwards closed this Aug 13, 2026
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