GNU bug report logs - #62994
Support styled underlines on TTY frames

Previous Next

Full log

View this message in rfc822 format

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: bug#62994: [PATCH v6] Add support for colored and styled underlines on tty frames
Date: Sun, 21 Apr 2024 15:51:16 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> The Subject line is longer than 78 characters, so our commit hooks
> didn't allow me to apply this.  Please make the Subject line shorter,
> possibly by dropping the bug number (mention the bug number somewhere
> in the rest of the log message).

Sorry, I'm not quite sure what you mean. Are you referring to the
subject line of the email message? That's 85 characters, but removing re
and the [PATCH X] part condenses it to 70 characters. The original
commit I'm making in git is only 58 characters. For reference I've gone
ahead and added a reference to the bug number in the commit message body
but I'm not sure that's enough to fix this. Would you like me to
re-submit with a custom --subject string matching the commit message in
git-send-email. I presume that would exclude the re, patch and bug
sections?

>> * src/xterm.c (x_draw_glyph_string): Updated to use renamed
>> FACE_UNDERLINE_SINGLE and FACE_UNDERLINE_WAVE face_underline_type
>> enumerations.
>
> These lines are also too long, and don't follow our conventions.  I
> suggest to use change-log-mode to format and fill those correctly.

To confirm this is what I did, I'm not sure if it's exactly what you
were recommending:

1. emacs -Q .git/COMMIT_EDITMSG
2. M-x change-log-mode
3. C-SPC to end of buffer message
4. C-M-\
5. Manually remove the leading indentation Emacs seems to have inserted
(other commit messages didn't seem to have this).

The end result doesn't seem to have changed the line length of anything,
although it did highlight one point where the entry was malformed.

> Period missing at the end of the comment.  Also, please leave two
> spaces between the end of the comment and the closing "*/", per our
> conventions.

Added.

>
>> +  /* Check supported underline styles. */
>> +  val = attrs[LFACE_UNDERLINE_INDEX];
>> +  if (!UNSPECIFIEDP (val))
>> +    if (EQ (CAR_SAFE (val), QCstyle))
>> +      if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
>> +	    || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)))
>> +	return false; /* Unsupported underline style.  */
>
> Can this be written in a cleaner way, as a single 'if' with all the
> conditions combined together, instead of nesting them?
>

I think so, yes. This was added from the corresponding tty code which
had more nesting because it also supported more checks against the face
attribute.

Updated.

>> -	return false;		/* same as default */
>> +	return false;  /* same as default */
>
> Why this whitespace change?

I suspect this was a byproduct of me re-indenting comments in an earlier patch.

Removed now.

-- 
Mohsin Kaleem

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.