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

Previous Next

Full log

Message #74 received at 62994 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: mohkale <at> kisara.moe
Cc: Eli Zaretskii <eliz <at> gnu.org>, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v2 1/1] Add support for colored and styled
 underlines on tty frames
Date: Mon, 24 Apr 2023 11:21:43 +0200

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

    Mohsin> From: Mohsin Kaleem <mohkale <at> 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
-- 

Previous Next

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