Subject: bug#75024: [PATCH] Fix check for underlining capability on ttys

From: Gerd Möllmann <[email-masked]>
Date: Sun, 05 Jan 2025 12:48:54 +0100

Mohsin Kaleem <[email-masked]> writes:

> Gerd Möllmann <[email-masked]> writes:
>
> Hi there,
>
> Sorry for the late response.
>
>>> I meanwhile found this, to my great surprise:
>>>
>>> #define TTY_CAP_UNDERLINE_STYLED	(0x32 & TTY_CAP_UNDERLINE)
>>>
>>> That makes TTY_CAP_UNDERLINE_STYLED == TTY_CAP_UNDERLINE. And this test
>>> in tty_capable_p
>>>
>>>   TTY_CAPABLE_P_TRY (tty,
>>> 		     TTY_CAP_UNDERLINE,	  tty->TS_enter_underline_mode,
>>> 		     NC_UNDERLINE);
>>>   TTY_CAPABLE_P_TRY (tty,
>>> 		     TTY_CAP_UNDERLINE_STYLED,	  tty->TF_set_underline_style,
>>>
>>> fails because it tests TTY_CAP_UNDERLINE twice, and requires both
>>> TS_enter_underline_mode and TF_set_underline_style to be usable for
>>> underline support. In Terminal.app, only TS_enter_underline_mode is
>>> available.
>>>
>>> Maybe this should have been 
>>>
>>> #define TTY_CAP_UNDERLINE_STYLED	0x40
>>>
>>> ?
>
> Ah, yep. The original intention was or a new bit flag with the existing
> underline bit flag so that styled underlines were only available in
> environments with at least regular underlines. In retrospect that was
> probably excessive and simply checking for styled underline support by
> itself (with a value of 0x40) is sufficient. I'd find it strange to have
> a terminal that supported styled underlines but not regular ones but
> there's no need to enforce this on the Emacs side.
>
>>>
>>> BTW, the 0x32 also also makes no sense to me because of 
>>>
>>> #define TTY_CAP_ITALIC  	0x10
>>> #define TTY_CAP_STRIKE_THROUGH	0x20
>>>
>>> CC to the original author to check.
>
> Correct here as well, I should've confirmed the binary representation
> :-(.
>
> 0b00000000000000000000000000010000 0o00000000020 0d0000000016 0x00000010
> 0b00000000000000000000000000100000 0o00000000040 0d0000000032 0x00000020
> 0b00000000000000000000000000110010 0o00000000062 0d0000000050 0x00000032
> 0b00000000000000000000000001000000 0o00000000100 0d0000000064 0x00000040
>
> 0x40 is what the next entry in the flag should have been.

Thanks for checking, Mohsin!

Would you perhaps have the time to prepare a patch that fixes this?