Subject: bug#62994: [PATCH v5] Add support for colored and styled underlines on tty frames

From: Mohsin Kaleem <[email-masked]>
Date: Sun, 10 Mar 2024 17:15:25 +0000

Po Lu <[email-masked]> writes:

> [email-masked] writes:
>
>> ++++
>
> Where's the documentation for this change?

Referring to the pluses themselves? Or I might be misunderstanding? I
was trying to match the format of other records in this file. Many
headings seemed to be followed by a mix of +++ or --- so I thought it
was required? If not I'll just remove it.

> We prefer to punctuate sentences within NEWS headings.

Added trailing period.

> What about GUI frames?  I don't want to see a display feature installed
> before it is also implemented for the likes of X.

Is this absolutely necessary? I had thought if the framework for
supporting this was in place then someone could contribute it down the
line but didn't want to prevent a possible terminal feature based on GUI
support (and vice versa). Currently if one tries to set a unsupported
underline style on GUI frames they'll be met with a single underline as
if they didn't specify a style at all. This is the same behaviour we'd
get if a you tried setting a face attribute on terminal frames that's
only supported by GUI frames.

> #define TTY_CAP_UNDERLINE_STYLED (0x32 & TTY_CAP_UNDERLINE)

Nice catch. Fixed.

> In Emacs, we format code with a mixture of tabs and spaces, using tabs
> to indent by whole tab stops and spaces to indent to the desired column,
> which is a multiple of 2 columns for most statements or the column after
> the opening paren where applicable.  Please also insert a space between
> function identifiers and argument lists, and confine all text to 80
> columns.
>
> Here you have evidently typed `M-x tabify' with a comment selected.  Do
> not insert tabs by whatever means within the body of a comment, although
> it is best to indent the body itself with them, whenever possible.
>
> More indentation problems.

I had thought `M-x tabify` was supposed to normalize this to the Emacs
code standards. Should I manually reindent this to match or is there
another function that can do this for me? Sorry if this is unusual, I'm
accustomed to formatting and linting tools that do this in bulk for me :-).

> Please don't link to Github, and explain
> instead the reasoning behind the sequence itself.
> Please also avoid inserting redundant braces in if statements.

Fixed and I've looked through a few other places where braces could be remvoed.

-- 
Mohsin Kaleem