Add a truncater to prevent overlength output cascades in spingroup #36
Conversation
burke
commented
Jun 29, 2018
burke
commented
lib/cli/ui/truncater.rb
Outdated
| end | ||
| end | ||
|
|
||
| return text if !truncation_index || width <= printing_width |
There was a problem hiding this comment.
why would we need to check width <= printing_width, shouldnt we just need to check truncation index?
There was a problem hiding this comment.
Removing this case causes Truncater.call("foo\x1b[0m", 3) to truncate. It's specifically for the case where we decided "Yes, this is the point at which we'd have to add a truncation!" but it's actually the end of the string.
There was a problem hiding this comment.
That might be a useful comment to leave
lib/cli/ui/truncater.rb
Outdated
| truncation_index ||= index | ||
| when 1 | ||
| truncation_index ||= index | ||
| end |
There was a problem hiding this comment.
Could this be if width >= printing_width instead of width <=> printing_width?
Should we break out of the block at this point?
| # | ||
| def render(index, force = true) | ||
| return full_render(index) if force || @force_full_render | ||
| def render(index, force = true, width: CLI::UI::Terminal.width) |
There was a problem hiding this comment.
Not really for this PR,, but can we dynamically evaluate the terminal width? I'm thinking, what if people resize while we're spinning.
There was a problem hiding this comment.
We dont really ever support resizing once you've started since we don't redraw anything above, though since this does redraw the line, maybe it does make sense to do this
lib/cli/ui/truncater.rb
Outdated
| SEMICOLON = 0x3b | ||
|
|
||
| # EMOJI_RANGE in particular is super inaccurate. This is best-effort. | ||
| # Make it better if you need it better. |
There was a problem hiding this comment.
Open source project: "PRs to make this better will be gratefully accepted."
lib/cli/ui/truncater.rb
Outdated
| when NUMERIC_RANGE, SEMICOLON | ||
| when LC_ALPHA_RANGE, UC_ALPHA_RANGE | ||
| mode = PARSE_ROOT | ||
| end |
There was a problem hiding this comment.
else raise? Or can we drop when NUMERIC_RANGE, SEMICOLON?
|
|
||
| def width(printable_codepoint) | ||
| case printable_codepoint | ||
| when EMOJI_RANGE |
There was a problem hiding this comment.
There was a problem hiding this comment.
It seems close enough for now. Maybe if this starts seeming inadequate.
|
Comments addressed |
