bug#62994: [PATCH v2 1/1] Add support for colored and styled underlines on tty frames

[Robert Pluim]

>>>>> On Fri, 21 Apr 2023 20:24:33 +0100, mohkale@kisara.moe said:

    Mohsin> From: Mohsin Kaleem <mohkale@kisara.moe>
    Mohsin> * src/dispextern.h (face, face_underline_type,
    Mohsin> syms_of_xfacse,

This typo tell me youʼre not using 'C-x 4 a' to generate your
ChangeLog entries.
 
    Mohsin> +---
    Mohsin> +** Support for 'styled' and 'colored' underline face
    Mohsin> attributes on TTY frames

Full stop at the end of the sentence please.

    Mohsin> +  if (face->underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
    Mohsin> +    {
    Mohsin> +   if (face->underline == FACE_UNDER_LINE
    Mohsin> +       || !tty->TF_set_underline_style)
    Mohsin> +           OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
    Mohsin> +   else if (tty->TF_set_underline_style)
    Mohsin> +     {
    Mohsin> +           char *p;
    Mohsin> +           p = tparam(tty->TF_set_underline_style, NULL, 0, 
face->underline, 0, 0, 0);
    Mohsin> +           OUTPUT (tty, p);
    Mohsin> +           xfree (p);
    Mohsin> +     }
    Mohsin> +    }

space before '(' please.


    Mohsin>                else if (EQ (key, QCstyle)
    Mohsin> -                       && !(EQ (val, Qline) || EQ (val, Qwave)))
    Mohsin> +                       && !(EQ (val, Qline)
    Mohsin> +                            || EQ (val, Qdouble)
    Mohsin> +                            || EQ (val, Qwave)
    Mohsin> +                            || EQ (val, Qdotted)
    Mohsin> +                            || EQ (val, Qdashed)))

If this was in lisp Iʼd expect to see `memq', but we work with what we
have :-)

    Mohsin> +  /* Check supported underline styles. */
    Mohsin> +  val = attrs[LFACE_UNDERLINE_INDEX];
    Mohsin> +  if (!UNSPECIFIEDP (val))
    Mohsin> +    {
    Mohsin> +   if (EQ (CAR_SAFE (val), QCstyle))
    Mohsin> +     {
    Mohsin> +           if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
    Mohsin> +                 || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)))
    Mohsin> +             {
    Mohsin> +                   return false; /* Unsupported underline style */
    Mohsin> +             }
    Mohsin> +     }
    Mohsin> +    }
    Mohsin> +
    Mohsin>    /* Everything checks out, this face is supported.  */
    Mohsin>    return true;
    Mohsin>  }

Your TAB usage here still looks wrong

    Mohsin> @@ -5390,15 +5409,26 @@ tty_supports_face_attributes_p (struct 
frame *f,
    Mohsin>    val = attrs[LFACE_UNDERLINE_INDEX];
    Mohsin>    if (!UNSPECIFIEDP (val))
    Mohsin>      {
    Mohsin> -      if (STRINGP (val))
    Mohsin> -   return false;           /* ttys can't use colored underlines */
    Mohsin> -      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE 
(CDR_SAFE (val)), Qwave))
    Mohsin> -   return false;           /* ttys can't use wave underlines */
    Mohsin> -      else if (face_attr_equal_p (val, 
def_attrs[LFACE_UNDERLINE_INDEX]))
    Mohsin> -   return false;           /* same as default */
    Mohsin> -      else
    Mohsin> -   test_caps |= TTY_CAP_UNDERLINE;
    Mohsin> -    }
    Mohsin> +   if (STRINGP (val))
    Mohsin> +           test_caps |= TTY_CAP_UNDERLINE_STYLED;
    Mohsin> +   else if (EQ (CAR_SAFE (val), QCstyle))
    Mohsin> +     {
    Mohsin> +   if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
    Mohsin> +         || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
    Mohsin> +         || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
    Mohsin> +         || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
    Mohsin> +         || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
    Mohsin> +     {
    Mohsin> +           return false; /* Face uses an unsupported underline 
style.  */
    Mohsin> +     }
    Mohsin> +
    Mohsin> +   test_caps |= TTY_CAP_UNDERLINE_STYLED;
    Mohsin> +     }
    Mohsin> +   else if (face_attr_equal_p (val, 
def_attrs[LFACE_UNDERLINE_INDEX]))
    Mohsin> +           return false;  /* same as default */
    Mohsin> +   else
    Mohsin> +           test_caps |= TTY_CAP_UNDERLINE;
    Mohsin> +  }

And here.

Robert
--