awesome-gocui / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
344 stars 39 forks source link

Add support for specifying both foreground and background colors. #49

Closed cswank closed 4 years ago

cswank commented 4 years ago

for example:

\033[48;5;200;38;5;100mHello

results in a foreground color (100) and a background color (200)

Since this also introduces the first test, what would you think about using testify/assert in the tests?

cswank commented 4 years ago

Yes, I'm pretty sure that specifying foreground and background at the same time is supported on major terminals. I tested a few with

printf '\033[48;5;130;38;5;40mHi\033[0m\n'

cswank commented 4 years ago

This PR should not be merged yet.

I just thought of an edge case and recovered escape_test.go that you asked me to remove and added another test case in which the background color being set is 38.

"\033[48;5;38;38;5;100mHi!!"

38 is a valid color but also the code for setting the foreground color. It causes escapeInterpreter to panic with my changes.

Would you prefer I close this PR and open another one or just ping you when I think I have this edge case fixed?

cswank commented 4 years ago

I think that latest commit fixes the edge cases I thought of.

cswank commented 4 years ago

I also realized that this only supports 8 bit colors (ei.output256()). I think outputNormal() needs the same behavior.

I'll work on that now.

cswank commented 4 years ago

It looks like OutputNormal already supports specifying foreground and background at the same time. So this should be good to go.

mjarkk commented 4 years ago

Thanks for making the PR