TB_DEFAULT shouldn't be checked in output modes other than TB_OUTPUT_NORMAL?

[mitchellwrosen]

Hey,

In both the original termbox and termbox2, it seems to me that the named colors TB_BLACK, TB_RED, etc. and TB_DEFAULT are only meant to be used in TB_OUTPUT_NORMAL mode.

Yet, in both source codes we still check if the user passed TB_DEFAULT for either foreground or background, and bail early if so:

https://github.com/termbox/termbox2/blob/693bfca38cf0f5347db1050cdfbe76e97e0d4d16/termbox.h#L2874-L2881

https://github.com/termbox/termbox2/blob/693bfca38cf0f5347db1050cdfbe76e97e0d4d16/termbox.h#L2905

https://github.com/termbox/termbox2/blob/693bfca38cf0f5347db1050cdfbe76e97e0d4d16/termbox.h#L2912

That seems wrong to me. In TB_OUTPUT_256, for example, I think I should be able to set a black foreground with number 0, but this instead gets interpreted as TB_DEFAULT. Maybe I misunderstand, though :)

Thanks!

[adsr]

Thanks for filing this. I agree this is broken.

Ideally TB_DEFAULT is supposed to represent no attributes, i.e., whatever the terminal defaults to after sending an sgr0. This is useful in all output modes.

I can think of a few options:

1. As you suggest, only support TB_DEFAULT in TB_OUTPUT_NORMAL. This is simple but makes it difficult to reset terminal attrs in other output modes. (It's possible but you have to fight the API and use tb_sendf.)
2. Alter the API to include is_default params alongside fg and bg params. Annoying.
3. Redefine TB_DEFAULT as one of the bitwise attrs like TB_BOLD. We can check for this in all output modes without blocking use of 0. For convenience, we can interpret 0 as TB_DEFAULT in TB_OUTPUT_NORMAL.

I like option 3. What do you think?

[mitchellwrosen]

Nice, option 3 sounds good to me, too! So would <any color> | TB_DEFAULT be equivalent to TB_DEFAULT?

[adsr]

Added in 846bba4.

Nice, option 3 sounds good to me, too! So would <any color> | TB_DEFAULT be equivalent to TB_DEFAULT?

Exactly.

I updated the inline docs for tb_set_output_mode. In summary:

mode	effect of fg/bg == 0	explicit default bitwise attr
TB_OUTPUT_NORMAL	default	TB_DEFAULT
TB_OUTPUT_216	default	TB_DEFAULT
TB_OUTPUT_GRAYSCALE	default	TB_DEFAULT
TB_OUTPUT_256	black	TB_DEFAULT
TB_OUTPUT_TRUECOLOR	black	TB_TRUECOLOR_DEFAULT

Note: While the color portion of <any color> | TB_DEFAULT is ignored, combining other attrs with default like TB_ITALIC | TB_DEFAULT or TB_TRUECOLOR_ITALIC | TB_TRUECOLOR_DEFAULT etc is valid.