#296 Status should be colored for kcctl get connectors#301
Conversation
harveyyue
force-pushed
the
kcctl-296
branch
3 times, most recently
from
01fb21e to
b829fa9
Compare
| })) | ||
| .map(String::trim) | ||
| .containsExactly( | ||
| "NAME TYPE STATE TASKS", |
There was a problem hiding this comment.
It looks like we're falling prey to freva/ascii-table#21 here, where ANSI escape codes are being treated as visible non-zero-width characters by the ASCII table library when determining how to space out its headers.
Could we use the Styler interface introduced in freva/ascii-table@0cd4887 to add color to state strings after the table's dimensions have already been determined?
I was initially tempted to suggest sticking with the current String::replace approach, but that fails when connectors' names match state names.
C0urante
left a comment
C0urante
left a comment
There was a problem hiding this comment.
Thanks @harveyyue! Tested this out locally and it works pretty nicely. One thought about the implications w/r/t ASCII table dimensions but apart from that this is good to go.
|
Hey @harveyyue, any thoughts on Chris' comments above? |
harveyyue
force-pushed
the
kcctl-296
branch
5 times, most recently
from
c742184 to
afd844d
Compare
|
@gunnarmorling @C0urante
|
C0urante
left a comment
C0urante
left a comment
There was a problem hiding this comment.
Thanks @harveyyue, this looks great! Two small questions about the integration tests but apart from that everything LGTM.
C0urante
left a comment
C0urante
left a comment
There was a problem hiding this comment.
LGTM, thanks @harveyyue!
|
Woot, woot! Thanks a lot, guys!
|
3 participants
Refer #296