fix(term): ansi: account for some wrap edge cases #59
Conversation
Properly count escape codes, better handling of breakpoints, and only break word/breakpoint when necessary. Fixes: #58
|
@mikelorant Pushed a bunch of fixes for edge cases and support for NBSP (non-breaking space) character. |
Breakpoints are now respected and wrapped properly. Support non-breaking spaces
aymanbagabas
force-pushed
the
term/wrap-reg
branch
from
5954450
to
5b3147b
Compare
exp/term/ansi/wrap.go
Outdated
| curWidth++ | ||
| default: | ||
| if wordLen+1 > limit { | ||
| if curWidth+wordLen+1 > limit { |
There was a problem hiding this comment.
Consider dropping the +1 and making it a >= comparison to simplify the logic?
| @@ -199,6 +202,8 @@ func Wordwrap(s string, limit int, breakpoints string) string { | |||
| case unicode.IsSpace(r): | |||
| addWord() | |||
| space.WriteByte(b[i]) | |||
| case r == '-': | |||
There was a problem hiding this comment.
Would there be value in having a small helper function IsDash to be consistent to how it is done for IsSpace?
There was a problem hiding this comment.
Perhaps, if we add support to other unicode dash characters 🤔
There was a problem hiding this comment.
Something to consider, certainly nothing to stop you merging once you do decide.
There was a problem hiding this comment.
Thinking more about this, there are many variants of dashes defined in unicode. People who wish to use custom breakpoints can do so using the breakpoints parameter
|
I'm not sure of the commit best practices used by the Charm team, but I generally recommend at this point to the developers I work with to squash their commits (seeing as there are now 8 commits). Your branch, so your rules 😄 |
Yes, that was what I'm going for. I don't think the commits should be split since they're all related and address the same issues |
|
I'd say while there might be some minor improvements possible, the current state is worth merging. LGTM. |
| @@ -199,6 +202,8 @@ func Wordwrap(s string, limit int, breakpoints string) string { | |||
| case unicode.IsSpace(r): | |||
| addWord() | |||
| space.WriteByte(b[i]) | |||
| case r == '-': | |||
There was a problem hiding this comment.
Thinking more about this, there are many variants of dashes defined in unicode. People who wish to use custom breakpoints can do so using the breakpoints parameter
Properly count escape codes, better handling of breakpoints, and only break word/breakpoint when necessary.
Fixes: #58