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

Previous Next

To reply to this bug, email your comments to 62994 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: bug-gnu-emacs <at> gnu.org
Subject: Support styled underlines on TTY frames
Date: Fri, 21 Apr 2023 15:29:03 +0100

Hi,

Support for styled underlines double, wave, dotted and dashed is a
relatively recent inclusion for newer terminal emulators. It's exposed
through either the Smulx or Su termcap (the former being an escape
sequence and the latter just a flag). Furthermore. despite not having a
dedicated termcap, these extensions also support colored underlines
through a separate escape sequence; making it possible to have an
underline with a different color to the foreground.

I'd like to add support for these to Emacs. Looking on the mailing list
it seems there's some prior efforts for this but none seem to have had
any traction. I've been working on a patch for these and will submit it
in reply to this message chain.

For testing I've been using kitty. If building with this patch series
you can view the underlines in the scratch buffer like so:

  ./src/emacs -nw -q --eval '(set-face-attribute (quote font-lock-comment-delimiter-face) nil :underline (list :style (quote wave) :color "blue"))'

-- 
Mohsin Kaleem

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH 3/3] Add support for colored underlines on tty frames
Date: Fri, 21 Apr 2023 15:34:48 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/term.c (turn_on_face, init_tty): Output escape sequence to set
underline color when set in the face and supported by the tty. Save
a default value for this sequence on init_tty when styled underlines
are supported.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline color on terminal frames.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face):
Assert whether colored underlines are supported by the current terminal
on display-supports-face-attributes-p checks. Load and save the color
of the underline from the face spec when realizing a face.
---
 etc/NEWS       | 11 +++++++----
 src/term.c     | 11 +++++++++++
 src/termchar.h |  3 +++
 src/xfaces.c   | 16 +++++++++++++---
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9f34927dfad..46b2b0e25c5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1917,20 +1917,23 @@ exist, the command creates it first.  You can use this command if you
 inadvertently delete the "*scratch*" buffer.
 
 ---
-** Support for 'styled-underline' face attributes on TTY frames
+** Support for 'styled' and 'colored' underline face attributes on TTY frames
 If your terminals termcap or terminfo database entry has the 'Su' or
 'Smulx' capability defined, Emacs will now emit the prescribed escape
 sequence necessary to render faces with styled underlines on TTY
 frames.
 
 Styled underlines are any underlines containing a non-default
-underline style.  The available underline styles for TTY frames are
-'double', 'wave', 'dotted', and 'dashed'.
+underline style or a color other than the foreground-color.
+The available underline styles for TTY frames are 'double', 'wave',
+'dotted', and 'dashed'.
 
 The 'Smulx' capability should define the actual sequence needed to
 render styled underlines. If ommitted, but the 'Su' flag is defined,
 then a default sequence will be used. It's recommended to use 'Smulx'
-instead of 'Su', with priority being given to 'Smulx'.
+instead of 'Su', with priority being given to 'Smulx'.  Support for
+colored underlines is automatically enabled with a default escape
+sequence when styled underline are supported.
 
 ** Debugging
 
diff --git a/src/term.c b/src/term.c
index 0f0393780eb..a1eb1961fad 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1984,6 +1984,14 @@ turn_on_face (struct frame *f, int face_id)
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
+
+      ts = tty->TF_set_underline_color;
+      if (ts && face->underline_color)
+	{
+          p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
+	  OUTPUT (tty, p);
+	  xfree (p);
+	}
     }
 }
 
@@ -4277,6 +4285,9 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   if (!tty->TF_set_underline_style && tgetflag("Su"))
     tty->TF_set_underline_style = "\x1b[4:%p1%dm";
 
+  if (tty->TF_set_underline_style)
+    tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 319c2319fba..563cde715ec 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -175,6 +175,9 @@ #define EMACS_TERMCHAR_H
                                            style based on the parameter. Param should
                                            be one of: 0 (none), 1 (straight), 2 (double),
                                            3 (wave), 4 (dotted), or 5 (dashed). */
+  const char *TF_set_underline_color;   /* Enabled when TF_set_underline_style is set:
+                                           Sets the color of the underline. Accepts a
+                                           single parameter, the color index. */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index 2c6c554d01d..c547a0b92f3 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5407,7 +5407,7 @@ tty_supports_face_attributes_p (struct frame *f,
   if (!UNSPECIFIEDP (val))
     {
       if (STRINGP (val))
-	return false;		/* ttys can't use colored underlines */
+	test_caps |= TTY_CAP_UNDERLINE_STYLED;
       else if (EQ (CAR_SAFE (val), QCstyle))
     {
         if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline) ||
@@ -6510,14 +6510,18 @@ realize_tty_face (struct face_cache *cache,
   underline = attrs[LFACE_UNDERLINE_INDEX];
   if (NILP (underline)) {
     face->tty_underline = FACE_NO_UNDERLINE;
+    face->underline_color = 0;
   } else if (EQ (underline, Qt)) {
     face->tty_underline = FACE_UNDER_LINE;
+    face->underline_color = 0;
   } else if (STRINGP (underline)) {
     face->tty_underline = FACE_UNDER_LINE;
+	face->underline_color = load_color (f, face, underline, LFACE_UNDERLINE_INDEX);
   } else if (CONSP (underline)) {
-    /* `(:style STYLE)'.
+    /* `(:color COLOR :style STYLE)'.
        STYLE being one of `line', `double', `wave', `dotted' or `dashed'. */
     face->tty_underline = FACE_UNDER_LINE;
+    face->underline_color = 0;
 
     while (CONSP (underline)) {
       Lisp_Object keyword, value;
@@ -6530,7 +6534,13 @@ realize_tty_face (struct face_cache *cache,
       value = XCAR (underline);
       underline = XCDR (underline);
 
-      if (EQ (keyword, QCstyle)) {
+      if (EQ (keyword, QCcolor)) {
+        if (EQ (value, Qforeground_color))
+          face->underline_color = 0;
+        else if (STRINGP (value))
+          face->underline_color = load_color (f, face, value, LFACE_UNDERLINE_INDEX);
+      }
+      else if (EQ (keyword, QCstyle)) {
         if (EQ (value, Qline))
           face->tty_underline = FACE_UNDER_LINE;
         else if (EQ (value, Qdouble))
-- 
2.40.0

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH 1/3] Add face definitions for more underline styles
Date: Fri, 21 Apr 2023 15:34:46 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face_underline_type, syms_of_xfacse,
internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
Add definitions for new underline styles of Double, Dotted and Dashed.
* lisp/cus-face.el (custom-face-attributes): Add entries for Double,
Dotted and Dashed so they can be set through `customize'.
---
 lisp/cus-face.el |  5 ++++-
 src/dispextern.h |  8 ++++++--
 src/xfaces.c     | 21 ++++++++++++++++++++-
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index ec89b4f7ff6..2d6e6c7b73e 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -141,7 +141,10 @@ custom-face-attributes
 		   (const :format "" :value :style)
 		   (choice :tag "Style"
 			   (const :tag "Line" line)
-			   (const :tag "Wave" wave))
+			   (const :tag "Double" double)
+			   (const :tag "Wave" wave)
+			   (const :tag "Dotted" dotted)
+			   (const :tag "Dashed" dashed))
                    (const :format "" :value :position)
                    (choice :tag "Position"
                            (const :tag "At Default Position" nil)
diff --git a/src/dispextern.h b/src/dispextern.h
index 4dcab113ea2..1dc84e32efc 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1653,9 +1653,13 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+  // Note: Order matches the order of the Smulx terminfo extension.
   FACE_NO_UNDERLINE = 0,
   FACE_UNDER_LINE,
-  FACE_UNDER_WAVE
+  FACE_DOUBLE_UNDER_LINE,
+  FACE_UNDER_WAVE,
+  FACE_DOTTED_UNDER_LINE,
+  FACE_DASHED_UNDER_LINE,
 };
 
 /* Structure describing a realized face.
@@ -1737,7 +1741,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline : 2;
+  ENUM_BF (face_underline_type) underline : 3;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
diff --git a/src/xfaces.c b/src/xfaces.c
index 37b703984be..cfbb89d2ae2 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3255,7 +3255,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
                 }
 
               else if (EQ (key, QCstyle)
-                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+                       && !(EQ (val, Qline) ||
+                            EQ (val, Qdouble) ||
+                            EQ (val, Qwave) ||
+                            EQ (val, Qdotted) ||
+                            EQ (val, Qdashed)))
                 {
                   valid_p = false;
                   break;
@@ -5204,6 +5208,7 @@ gui_supports_face_attributes_p (struct frame *f,
                                 Lisp_Object attrs[LFACE_VECTOR_SIZE],
                                 struct face *def_face)
 {
+  Lisp_Object val;
   Lisp_Object *def_attrs = def_face->lface;
   Lisp_Object lattrs[LFACE_VECTOR_SIZE];
 
@@ -5298,6 +5303,17 @@ gui_supports_face_attributes_p (struct frame *f,
       return false;
     }
 
+  /* 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 */
+      }
+    }
+  }
+
   /* Everything checks out, this face is supported.  */
   return true;
 }
@@ -7165,6 +7181,9 @@ syms_of_xfaces (void)
   DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
+  DEFSYM (Qdouble, "double");
+  DEFSYM (Qdotted, "dotted");
+  DEFSYM (Qdashed, "dashed");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qflat_button, "flat-button");
-- 
2.40.0

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH 0/3] Support styled underlines on tty Emacs frames
Date: Fri, 21 Apr 2023 15:34:45 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

Modern terminals (such as kitty) support setting the style of an
underline with the escape sequence exposed in the Smulx termcap.
This allows for (among others) wavy underlines on terminals.
These terminals also support setting the color of these underlines
using a separate escape sequence that to the best of my knowledge
is not exposed as a termcap but has been adopted by other terminal
supporting editors like neovim.

Mohsin Kaleem (3):
  Add face definitions for more underline styles
  Add support for styled underlines on tty frames
  Add support for colored underlines on tty frames

 etc/NEWS         | 19 ++++++++++
 lisp/cus-face.el |  5 ++-
 src/dispextern.h | 11 ++++--
 src/term.c       | 46 +++++++++++++++++++++---
 src/termchar.h   |  7 ++++
 src/xfaces.c     | 93 ++++++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 167 insertions(+), 14 deletions(-)

-- 
2.40.0

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH 2/3] Add support for styled underlines on tty frames
Date: Fri, 21 Apr 2023 15:34:47 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face): Convert tty_underline_p from bool to a
face_underline_type enumeration. Add a flag to check whether styled
underlines are available.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline style on terminal frames.
* src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
underline style escape sequence from the Smulx termcap (alternatively if
the Su flag is set use a default sequence). Allow checking for support
of styled underlines in the current terminal frame. Output the necessary
escape sequences to activate a styled underline on turn_on_face; this is
currently only used for the new special underline styles, a default
straight underline will still use the "us" termcap.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face):
Assert whether styled underlines are supported by the current terminal
on display-supports-face-attributes-p checks. Populate the correct
underline style in the face spec when realizing a face.
---
 etc/NEWS         | 16 +++++++++++++
 src/dispextern.h |  3 ++-
 src/term.c       | 35 ++++++++++++++++++++++++----
 src/termchar.h   |  4 ++++
 src/xfaces.c     | 60 ++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 62d2fdcd3a4..9f34927dfad 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1916,6 +1916,22 @@ This command switches to the "*scratch*" buffer.  If "*scratch*" doesn't
 exist, the command creates it first.  You can use this command if you
 inadvertently delete the "*scratch*" buffer.
 
+---
+** Support for 'styled-underline' face attributes on TTY frames
+If your terminals termcap or terminfo database entry has the 'Su' or
+'Smulx' capability defined, Emacs will now emit the prescribed escape
+sequence necessary to render faces with styled underlines on TTY
+frames.
+
+Styled underlines are any underlines containing a non-default
+underline style.  The available underline styles for TTY frames are
+'double', 'wave', 'dotted', and 'dashed'.
+
+The 'Smulx' capability should define the actual sequence needed to
+render styled underlines. If ommitted, but the 'Su' flag is defined,
+then a default sequence will be used. It's recommended to use 'Smulx'
+instead of 'Su', with priority being given to 'Smulx'.
+
 ** Debugging
 
 ---
diff --git a/src/dispextern.h b/src/dispextern.h
index 1dc84e32efc..6ea6f6170e4 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1773,7 +1773,7 @@ #define FONT_TOO_HIGH(ft)						\
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
   bool_bf tty_italic_p : 1;
-  bool_bf tty_underline_p : 1;
+  ENUM_BF (face_underline_type) tty_underline : 3;
   bool_bf tty_reverse_p : 1;
   bool_bf tty_strike_through_p : 1;
 
@@ -3365,6 +3365,7 @@ #define TTY_CAP_BOLD		0x04
 #define TTY_CAP_DIM		0x08
 #define TTY_CAP_ITALIC  	0x10
 #define TTY_CAP_STRIKE_THROUGH	0x20
+#define TTY_CAP_UNDERLINE_STYLED	0x32 & TTY_CAP_UNDERLINE
 
 
 /***********************************************************************
diff --git a/src/term.c b/src/term.c
index 53ba2a231e4..0f0393780eb 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1948,8 +1948,17 @@ turn_on_face (struct frame *f, int face_id)
 	OUTPUT1 (tty, tty->TS_enter_dim_mode);
     }
 
-  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
-    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+  if (face->tty_underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE)) {
+    if (face->tty_underline == FACE_UNDER_LINE ||
+        !tty->TF_set_underline_style) {
+      OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+    } else if (tty->TF_set_underline_style) {
+      char *p;
+      p = tparam(tty->TF_set_underline_style, NULL, 0, face->tty_underline, 0, 0, 0);
+      OUTPUT (tty, p);
+      xfree (p);
+    }
+  }
 
   if (face->tty_strike_through_p
       && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
@@ -1995,7 +2004,7 @@ turn_off_face (struct frame *f, int face_id)
       if (face->tty_bold_p
 	  || face->tty_italic_p
 	  || face->tty_reverse_p
-	  || face->tty_underline_p
+	  || face->tty_underline
 	  || face->tty_strike_through_p)
 	{
 	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
@@ -2007,7 +2016,7 @@ turn_off_face (struct frame *f, int face_id)
     {
       /* If we don't have "me" we can only have those appearances
 	 that have exit sequences defined.  */
-      if (face->tty_underline_p)
+      if (face->tty_underline)
 	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
     }
 
@@ -2036,6 +2045,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
   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,
+             NC_UNDERLINE);
   TTY_CAPABLE_P_TRY (tty,
 		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
   TTY_CAPABLE_P_TRY (tty,
@@ -4250,6 +4262,21 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   tty->TF_underscore = tgetflag ("ul");
   tty->TF_teleray = tgetflag ("xt");
 
+  // Styled underlines.
+  //
+  // Support for this is provided either by the escape sequence in
+  // Smulx or the Su flag. The latter results in a common default
+  // escape sequence and is not recommended.
+#ifdef TERMINFO
+  tty->TF_set_underline_style = tigetstr("Smulx");
+  if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
+    tty->TF_set_underline_style = NULL;
+#else
+  tty->TF_set_underline_style = tgetstr("Smulx", address);
+#endif
+  if (!tty->TF_set_underline_style && tgetflag("Su"))
+    tty->TF_set_underline_style = "\x1b[4:%p1%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 5c47679a994..319c2319fba 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -171,6 +171,10 @@ #define EMACS_TERMCHAR_H
                                    non-blank position.  Must clear before writing _.  */
   int TF_teleray;               /* termcap xt flag: many weird consequences.
                                    For t1061. */
+  const char *TF_set_underline_style;   /* termcap Smulx entry: Switches the underline
+                                           style based on the parameter. Param should
+                                           be one of: 0 (none), 1 (straight), 2 (double),
+                                           3 (wave), 4 (dotted), or 5 (dashed). */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index cfbb89d2ae2..2c6c554d01d 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5408,8 +5408,18 @@ tty_supports_face_attributes_p (struct frame *f,
     {
       if (STRINGP (val))
 	return false;		/* ttys can't use colored underlines */
-      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
-	return false;		/* ttys can't use wave underlines */
+      else if (EQ (CAR_SAFE (val), QCstyle))
+    {
+        if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qwave) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed))) {
+            return false; /* Unsupported underline style */
+        }
+
+	    test_caps |= TTY_CAP_UNDERLINE_STYLED;
+    }
       else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
 	return false;		/* same as default */
       else
@@ -6335,6 +6345,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
                 face->underline = FACE_UNDER_LINE;
               else if (EQ (value, Qwave))
                 face->underline = FACE_UNDER_WAVE;
+              else
+                face->underline = FACE_UNDER_LINE;
             }
 	  else if (EQ (keyword, QCposition))
 	    {
@@ -6469,6 +6481,7 @@ realize_tty_face (struct face_cache *cache,
 {
   struct face *face;
   int weight, slant;
+  Lisp_Object underline;
   bool face_colors_defaulted = false;
   struct frame *f = cache->f;
 
@@ -6488,13 +6501,52 @@ realize_tty_face (struct face_cache *cache,
     face->tty_bold_p = true;
   if (slant != 100)
     face->tty_italic_p = true;
-  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
-    face->tty_underline_p = true;
   if (!NILP (attrs[LFACE_INVERSE_INDEX]))
     face->tty_reverse_p = true;
   if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
     face->tty_strike_through_p = true;
 
+  /* Text underline.  */
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (NILP (underline)) {
+    face->tty_underline = FACE_NO_UNDERLINE;
+  } else if (EQ (underline, Qt)) {
+    face->tty_underline = FACE_UNDER_LINE;
+  } else if (STRINGP (underline)) {
+    face->tty_underline = FACE_UNDER_LINE;
+  } else if (CONSP (underline)) {
+    /* `(:style STYLE)'.
+       STYLE being one of `line', `double', `wave', `dotted' or `dashed'. */
+    face->tty_underline = FACE_UNDER_LINE;
+
+    while (CONSP (underline)) {
+      Lisp_Object keyword, value;
+
+      keyword = XCAR (underline);
+      underline = XCDR (underline);
+
+      if (!CONSP (underline))
+        break;
+      value = XCAR (underline);
+      underline = XCDR (underline);
+
+      if (EQ (keyword, QCstyle)) {
+        if (EQ (value, Qline))
+          face->tty_underline = FACE_UNDER_LINE;
+        else if (EQ (value, Qdouble))
+          face->tty_underline = FACE_DOUBLE_UNDER_LINE;
+        else if (EQ (value, Qwave))
+          face->tty_underline = FACE_UNDER_WAVE;
+        else if (EQ (value, Qdotted))
+          face->tty_underline = FACE_DOTTED_UNDER_LINE;
+        else if (EQ (value, Qdashed))
+          face->tty_underline = FACE_DASHED_UNDER_LINE;
+        else
+          face->tty_underline = FACE_UNDER_LINE;
+      }
+    }
+  }
+
   /* Map color names to color indices.  */
   map_tty_color (f, face, LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
   map_tty_color (f, face, LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
-- 
2.40.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mohkale <at> kisara.moe
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 0/3] Support styled underlines on tty Emacs
 frames
Date: Fri, 21 Apr 2023 18:52:43 +0300

> Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
> From: mohkale <at> kisara.moe
> Date: Fri, 21 Apr 2023 15:34:45 +0100
> 
> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> 
> Modern terminals (such as kitty) support setting the style of an
> underline with the escape sequence exposed in the Smulx termcap.
> This allows for (among others) wavy underlines on terminals.
> These terminals also support setting the color of these underlines
> using a separate escape sequence that to the best of my knowledge
> is not exposed as a termcap but has been adopted by other terminal
> supporting editors like neovim.

Thanks.

Please note up front that we must wait for the completion of your
legal paperwork before we can accept significant contributions (such
as this one) from you.

I will send a few comments to each of the patches separately.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mohkale <at> kisara.moe
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 1/3] Add face definitions for more underline
 styles
Date: Fri, 21 Apr 2023 18:58:39 +0300

> Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
> From: mohkale <at> kisara.moe
> Date: Fri, 21 Apr 2023 15:34:46 +0100
> 
> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> 
> --- a/src/dispextern.h
> +++ b/src/dispextern.h
> @@ -1653,9 +1653,13 @@ #define FONT_TOO_HIGH(ft)						\
>  
>  enum face_underline_type
>  {
> +  // Note: Order matches the order of the Smulx terminfo extension.

Please use the C style of comments, /* Like this. */, not the C++
style.

>    FACE_NO_UNDERLINE = 0,
>    FACE_UNDER_LINE,
> -  FACE_UNDER_WAVE
> +  FACE_DOUBLE_UNDER_LINE,
> +  FACE_UNDER_WAVE,
> +  FACE_DOTTED_UNDER_LINE,
> +  FACE_DASHED_UNDER_LINE,

Is it really necessary to change the numerical value of
FACE_UNDER_WAVE?  Can it be left at its original value?

> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -3255,7 +3255,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
>                  }
>  
>                else if (EQ (key, QCstyle)
> -                       && !(EQ (val, Qline) || EQ (val, Qwave)))
> +                       && !(EQ (val, Qline) ||
> +                            EQ (val, Qdouble) ||
> +                            EQ (val, Qwave) ||
> +                            EQ (val, Qdotted) ||
> +                            EQ (val, Qdashed)))

Our style is to put the operators ("||" in this case) at the beginning
of the line, not at its end.

> +  /* 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 */

Likewise here.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mohkale <at> kisara.moe
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Fri, 21 Apr 2023 19:07:33 +0300

> Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
> From: mohkale <at> kisara.moe
> Date: Fri, 21 Apr 2023 15:34:47 +0100
> 
> diff --git a/etc/NEWS b/etc/NEWS
> index 62d2fdcd3a4..9f34927dfad 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1916,6 +1916,22 @@ This command switches to the "*scratch*" buffer.  If "*scratch*" doesn't
>  exist, the command creates it first.  You can use this command if you
>  inadvertently delete the "*scratch*" buffer.
>  
> +---
> +** Support for 'styled-underline' face attributes on TTY frames
> +If your terminals termcap or terminfo database entry has the 'Su' or
> +'Smulx' capability defined, Emacs will now emit the prescribed escape
> +sequence necessary to render faces with styled underlines on TTY
> +frames.
> +
> +Styled underlines are any underlines containing a non-default
> +underline style.  The available underline styles for TTY frames are
> +'double', 'wave', 'dotted', and 'dashed'.
> +
> +The 'Smulx' capability should define the actual sequence needed to
> +render styled underlines. If ommitted, but the 'Su' flag is defined,
> +then a default sequence will be used. It's recommended to use 'Smulx'
> +instead of 'Su', with priority being given to 'Smulx'.

I think the last paragraph is unnecessary in NEWS, since there's
nothing users can do about the capabilities exposed by their terminfo
database.  In any case, please observe our convention of leaving two
spaces between sentences, not one.

> --- a/src/dispextern.h
> +++ b/src/dispextern.h
> @@ -1773,7 +1773,7 @@ #define FONT_TOO_HIGH(ft)						\
>       string meaning the default color of the TTY.  */
>    bool_bf tty_bold_p : 1;
>    bool_bf tty_italic_p : 1;
> -  bool_bf tty_underline_p : 1;
> +  ENUM_BF (face_underline_type) tty_underline : 3;
>    bool_bf tty_reverse_p : 1;
>    bool_bf tty_strike_through_p : 1;

Is there any reason for separate tty_* face attribute bits? why cannot
we use the same bits on both TTY and GUI frames?

> --- a/src/term.c
> +++ b/src/term.c
> @@ -1948,8 +1948,17 @@ turn_on_face (struct frame *f, int face_id)
>  	OUTPUT1 (tty, tty->TS_enter_dim_mode);
>      }
>  
> -  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
> -    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
> +  if (face->tty_underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE)) {
> +    if (face->tty_underline == FACE_UNDER_LINE ||
> +        !tty->TF_set_underline_style) {
> +      OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
> +    } else if (tty->TF_set_underline_style) {

Please use our style of braces and placing of operators in continued
lines.  The brace should be on their own lines, alone.

> +  // Styled underlines.
> +  //
> +  // Support for this is provided either by the escape sequence in
> +  // Smulx or the Su flag. The latter results in a common default
> +  // escape sequence and is not recommended.

Comment style again: please use the C style.

> +#ifdef TERMINFO
> +  tty->TF_set_underline_style = tigetstr("Smulx");
> +  if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
> +    tty->TF_set_underline_style = NULL;
> +#else
> +  tty->TF_set_underline_style = tgetstr("Smulx", address);
> +#endif
> +  if (!tty->TF_set_underline_style && tgetflag("Su"))
> +    tty->TF_set_underline_style = "\x1b[4:%p1%dm";

Any pointers to where this standard escape sequence is defined?  I'd
like to have that in a comment to this line.

> +      else if (EQ (CAR_SAFE (val), QCstyle))
> +    {
> +        if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline) ||
> +              EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble) ||
> +              EQ (CAR_SAFE (CDR_SAFE (val)), Qwave) ||
> +              EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted) ||
> +              EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed))) {

Continuation line style again.

> +            return false; /* Unsupported underline style */

Comments should be complete sentences, and should end in a period and
2 spaces.

> +  /* Text underline.  */
> +  underline = attrs[LFACE_UNDERLINE_INDEX];
> +  if (NILP (underline)) {
> +    face->tty_underline = FACE_NO_UNDERLINE;
> +  } else if (EQ (underline, Qt)) {
> +    face->tty_underline = FACE_UNDER_LINE;
> +  } else if (STRINGP (underline)) {
> +    face->tty_underline = FACE_UNDER_LINE;
> +  } else if (CONSP (underline)) {

Style of braces again.

> +    while (CONSP (underline)) {
> +      Lisp_Object keyword, value;

And here.

> +      if (EQ (keyword, QCstyle)) {

And here.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 1/3] Add face definitions for more underline
 styles
Date: Fri, 21 Apr 2023 17:08:39 +0100

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

> Is it really necessary to change the numerical value of
> FACE_UNDER_WAVE?  Can it be left at its original value?

If consistency with Smulx is desirable then I think it is. Otherwise
when we populate the escape sequence for the underline we'll need to map
from the face underline style to the sequence underline style.
Reordering like this makes them match so we don't have to do any extra
processing later, but if preferable I can retain the original order and
add a helper function to map to the sequence value.

-- 
Mohsin Kaleem

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 0/3] Support styled underlines on tty Emacs
 frames
Date: Fri, 21 Apr 2023 17:10:36 +0100

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

> Please note up front that we must wait for the completion of your
> legal paperwork before we can accept significant contributions (such
> as this one) from you.

Yep, I'm aware. I've already reached out to Craig at the FSF help desk.
I don't think there's anything more needed from me or my employer so I'm
just waiting on a confirmation response. Otherwise I'm OK keeping this
patch dormant until it's ready. I'd like to just get it in a state where
it's good to merge, then when the paperwork is signed we can merge.

> I will send a few comments to each of the patches separately.

Much appreciated, thanks Eli :-).

-- 
Mohsin Kaleem

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mohkale <at> kisara.moe
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 3/3] Add support for colored underlines on tty
 frames
Date: Fri, 21 Apr 2023 19:12:54 +0300

> Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
> From: mohkale <at> kisara.moe
> Date: Fri, 21 Apr 2023 15:34:48 +0100
> 
> diff --git a/etc/NEWS b/etc/NEWS
> index 9f34927dfad..46b2b0e25c5 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1917,20 +1917,23 @@ exist, the command creates it first.  You can use this command if you
>  inadvertently delete the "*scratch*" buffer.
>  
>  ---
> -** Support for 'styled-underline' face attributes on TTY frames
> +** Support for 'styled' and 'colored' underline face attributes on TTY frames
>  If your terminals termcap or terminfo database entry has the 'Su' or
>  'Smulx' capability defined, Emacs will now emit the prescribed escape
>  sequence necessary to render faces with styled underlines on TTY
>  frames.
>  
>  Styled underlines are any underlines containing a non-default
> -underline style.  The available underline styles for TTY frames are
> -'double', 'wave', 'dotted', and 'dashed'.
> +underline style or a color other than the foreground-color.
> +The available underline styles for TTY frames are 'double', 'wave',
> +'dotted', and 'dashed'.
>  
>  The 'Smulx' capability should define the actual sequence needed to
>  render styled underlines. If ommitted, but the 'Su' flag is defined,
>  then a default sequence will be used. It's recommended to use 'Smulx'
> -instead of 'Su', with priority being given to 'Smulx'.
> +instead of 'Su', with priority being given to 'Smulx'.  Support for
> +colored underlines is automatically enabled with a default escape
> +sequence when styled underline are supported.

Please don't break the changeset into several patches, not in this
case: this changeset introduces a feature that there's no reason to
break into several sub-features, so making a single patch will make it
easier to review the changes.

> +      ts = tty->TF_set_underline_color;
> +      if (ts && face->underline_color)
> +	{
> +          p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);

It looks like your edits use indentation with only spaces.  Our style
in C sources is to use TABs and spaces, so please set up your Emacs to
follow our style (it should happen automatically if you let Emacs read
and follow the .dir-locals.el file in the Emacs repository).

> +  if (tty->TF_set_underline_style)
> +    tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";

What is the source for this escape sequence? can you mention it in a
comment?

Thanks.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Fri, 21 Apr 2023 20:49:19 +0300

[Please use Reply All to keep the bug address on the CC list.]

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Date: Fri, 21 Apr 2023 18:04:12 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I think the last paragraph is unnecessary in NEWS, since there's
> > nothing users can do about the capabilities exposed by their terminfo
> > database.
> 
> I kinda disagree, I think mentioning this can help users debug why
> certain termcaps aren't being processed correctly or to direct requests
> to fix the processing directly to the terminal emulator instead of to
> Emacs first. I'll go ahead and remove it for now.

This is stuff for etc/PROBLEMS, not for NEWS.  And we only add it to
PROBLEMS if someone actually reports the relevant problems.

> > Please use our style of braces and placing of operators in continued
> > lines.  The brace should be on their own lines, alone.
> 
> For future reference is this style documented anywhere?

Yes, in standards.info.  This is the GNU recommended style.

> > Any pointers to where this standard escape sequence is defined?  I'd
> > like to have that in a comment to this line.
> 
> For the underline style I think kitty is a good reference since it was
> the first to support it. That documentation is here [1]. The sequence
> for color is also documented there... but I find the way its described
> at best confusing and at worst unhelpful. I've been using the neovim
> source code [2] as a reference for the implementation and color sequence
> alongside trial and error.
> 
> [1]: https://sw.kovidgoyal.net/kitty/underlines/
> [2]: https://github.com/neovim/neovim/commit/42f492ac99058bd1cd56c3c7871e7e464b2a5e24

Did Kitty invent that?  If not, there must be some reference that is
not the source code of a terminal emulator.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 3/3] Add support for colored underlines on
 tty frames
Date: Fri, 21 Apr 2023 20:56:18 +0300

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Date: Fri, 21 Apr 2023 18:10:39 +0100
> 
> > It looks like your edits use indentation with only spaces.  Our style
> > in C sources is to use TABs and spaces, so please set up your Emacs to
> > follow our style (it should happen automatically if you let Emacs read
> > and follow the .dir-locals.el file in the Emacs repository).
> 
> Looks like .dir-locals.el won't work for me since I use c-ts-mode and it
> hooks into c-mode only.

You are right, I've just fixed that.

> >> +  if (tty->TF_set_underline_style)
> >> +    tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
> >
> > What is the source for this escape sequence? can you mention it in a
> > comment?
> 
> The form here is nonstandard. It's adapted from the same escape sequence
> used by neovim [1]. The difference here is that this form accepts a
> single color parameter whereas that one accepts three different
> parameters for the color range. Since this form is how the existing
> foreground and background sequences are set AND the sequence doesn't
> come from a termcap I figured it was okay to adapt it to a form Emacs
> can already substitute.
> 
> [1]: https://github.com/neovim/neovim/blob/42f492ac99058bd1cd56c3c7871e7e464b2a5e24/src/nvim/tui/tui.c#L1932

This should all be in a comment there, thanks.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Fri, 21 Apr 2023 19:04:13 +0100

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

> [Please use Reply All to keep the bug address on the CC list.]

Thought I did, my bad. Sorry.

> This is stuff for etc/PROBLEMS, not for NEWS.  And we only add it to
> PROBLEMS if someone actually reports the relevant problems.

Good enough :-).

> Yes, in standards.info.  This is the GNU recommended style.

Cool, doesn't seem to be in the Emacs repo but I'm guessing it's this
[1]. I'll try to adapt to it.

[1]: https://www.gnu.org/prep/standards/standards.html#Formatting

> Did Kitty invent that?  If not, there must be some reference that is
> not the source code of a terminal emulator.

I think it did. What I've found online indicates kitty invented it, used
Su for the termcap, this lead to an issue on tmux which suggested
changing it to Smulx [2]. That's why the termcap for it is inconsistent.
I can't find any reference for support before kitty.

[2]: https://github.com/tmux/tmux/issues/1492

-- 
Mohsin Kaleem

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Fri, 21 Apr 2023 19:43:07 +0100

Mohsin Kaleem <mohkale <at> kisara.moe> writes:

> Cool, doesn't seem to be in the Emacs repo but I'm guessing it's this
> [1]. I'll try to adapt to it.

I still don't get how to apply this to this repo Eli. You mentioned using
tabs and spaces but every lines aside from a nested expression starts
with 2 spaces. Some times there's 3 levels deep of expressions all
indented with spaces and then one tab width just wide enough at 8
characters to appear like it's indented with spaces atop the previous
scope.

Can I just do leading indent with tabs matching current indentation
level, curly braces 2 space ahead of the parent indent, and spaces ahead
of tabs to align a parenthesized expression or multi line condition with
the previous line?

-- 
Mohsin Kaleem

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

From: mohkale <at> kisara.moe
To: Eli Zaretskii <eliz <at> gnu.org>,
	62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v2 0/1] Support styled underlines on tty Emacs frames
Date: Fri, 21 Apr 2023 20:24:32 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

Modern terminals (such as kitty) support setting the style of an
underline with the escape sequence exposed in the Smulx termcap.
This allows for (among others) wavy underlines on terminals.
These terminals also support setting the color of these underlines
using a separate escape sequence that to the best of my knowledge
is not exposed as a termcap but has been adopted by other terminal
supporting editors like neovim.

Version 2: This should've acted on all the comments on the last patch.
The only thing I'm not sure about is the formatting. I've tried my best
to get something consistent with what's already their or the style guide
reference I found online but it's probably broken in a few places.

Mohsin Kaleem (1):
  Add support for colored and styled underlines on tty frames

 etc/NEWS         |  12 +++++
 lisp/cus-face.el |   5 +-
 src/dispextern.h |  10 ++--
 src/term.c       |  54 +++++++++++++++++++--
 src/termchar.h   |   7 +++
 src/xfaces.c     | 121 ++++++++++++++++++++++++++++++++++++++++++-----
 6 files changed, 189 insertions(+), 20 deletions(-)

-- 
2.40.0

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

From: mohkale <at> kisara.moe
To: Eli Zaretskii <eliz <at> gnu.org>,
	62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v2 1/1] Add support for colored and styled underlines on tty
 frames
Date: Fri, 21 Apr 2023 20:24:33 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face, face_underline_type, syms_of_xfacse,
internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
Add definitions for new underline styles of Double, Dotted and Dashed.
Delete tty_underline_p from the face struct and use just underline going
forward.  Add a flag to check whether styled underlines are available.
* lisp/cus-face.el (custom-face-attributes): Add entries for Double,
Dotted and Dashed so they can be set through `customize'.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline style and color on terminal frames.
* src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
underline style escape sequence from the Smulx termcap (alternatively if
the Su flag is set use a default sequence).  Allow checking for support
of styled underlines in the current terminal frame.  Output the necessary
escape sequences to activate a styled underline on turn_on_face; this is
currently only used for the new special underline styles, a default
straight underline will still use the "us" termcap.  Output escape
sequence to set underline color when set in the face and supported by
the tty.  Save a default value for this sequence on init_tty when styled
underlines are supported.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face):
Assert whether styled underlines are supported by the current terminal
on display-supports-face-attributes-p checks.  Populate the correct
underline style and color in the face spec when realizing a face.
---
 etc/NEWS         |  12 +++++
 lisp/cus-face.el |   5 +-
 src/dispextern.h |  10 ++--
 src/term.c       |  54 +++++++++++++++++++--
 src/termchar.h   |   7 +++
 src/xfaces.c     | 121 ++++++++++++++++++++++++++++++++++++++++++-----
 6 files changed, 189 insertions(+), 20 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 62d2fdcd3a4..e0798687bf4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1916,6 +1916,18 @@ This command switches to the "*scratch*" buffer.  If "*scratch*" doesn't
 exist, the command creates it first.  You can use this command if you
 inadvertently delete the "*scratch*" buffer.
 
+---
+** Support for 'styled' and 'colored' underline face attributes on TTY frames
+If your terminals termcap or terminfo database entry has the 'Su' or
+'Smulx' capability defined, Emacs will now emit the prescribed escape
+sequence necessary to render faces with styled underlines on TTY
+frames.
+
+Styled underlines are any underlines containing a non-default
+underline style or a color other than the foreground-color.
+The available underline styles for TTY frames are 'double', 'wave',
+'dotted', and 'dashed'.
+
 ** Debugging
 
 ---
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index ec89b4f7ff6..2d6e6c7b73e 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -141,7 +141,10 @@ custom-face-attributes
 		   (const :format "" :value :style)
 		   (choice :tag "Style"
 			   (const :tag "Line" line)
-			   (const :tag "Wave" wave))
+			   (const :tag "Double" double)
+			   (const :tag "Wave" wave)
+			   (const :tag "Dotted" dotted)
+			   (const :tag "Dashed" dashed))
                    (const :format "" :value :position)
                    (choice :tag "Position"
                            (const :tag "At Default Position" nil)
diff --git a/src/dispextern.h b/src/dispextern.h
index 4dcab113ea2..753bee446f1 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1653,9 +1653,13 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+	/* Note: Order matches the order of the Smulx terminfo extension. */
   FACE_NO_UNDERLINE = 0,
   FACE_UNDER_LINE,
-  FACE_UNDER_WAVE
+  FACE_DOUBLE_UNDER_LINE,
+  FACE_UNDER_WAVE,
+  FACE_DOTTED_UNDER_LINE,
+  FACE_DASHED_UNDER_LINE,
 };
 
 /* Structure describing a realized face.
@@ -1737,7 +1741,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline : 2;
+  ENUM_BF (face_underline_type) underline : 3;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
@@ -1769,7 +1773,6 @@ #define FONT_TOO_HIGH(ft)						\
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
   bool_bf tty_italic_p : 1;
-  bool_bf tty_underline_p : 1;
   bool_bf tty_reverse_p : 1;
   bool_bf tty_strike_through_p : 1;
 
@@ -3361,6 +3364,7 @@ #define TTY_CAP_BOLD		0x04
 #define TTY_CAP_DIM		0x08
 #define TTY_CAP_ITALIC  	0x10
 #define TTY_CAP_STRIKE_THROUGH	0x20
+#define TTY_CAP_UNDERLINE_STYLED	0x32 & TTY_CAP_UNDERLINE
 
 
 /***********************************************************************
diff --git a/src/term.c b/src/term.c
index 53ba2a231e4..2c1d44bff7a 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1948,8 +1948,19 @@ turn_on_face (struct frame *f, int face_id)
 	OUTPUT1 (tty, tty->TS_enter_dim_mode);
     }
 
-  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
-    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+  if (face->underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
+    {
+	if (face->underline == FACE_UNDER_LINE
+	    || !tty->TF_set_underline_style)
+		OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+	else if (tty->TF_set_underline_style)
+	  {
+		char *p;
+		p = tparam(tty->TF_set_underline_style, NULL, 0, face->underline, 0, 0, 0);
+		OUTPUT (tty, p);
+		xfree (p);
+	  }
+    }
 
   if (face->tty_strike_through_p
       && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
@@ -1975,6 +1986,14 @@ turn_on_face (struct frame *f, int face_id)
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
+
+      ts = tty->TF_set_underline_color;
+      if (ts && face->underline_color)
+	{
+	  p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
+	  OUTPUT (tty, p);
+	  xfree (p);
+	}
     }
 }
 
@@ -1995,7 +2014,7 @@ turn_off_face (struct frame *f, int face_id)
       if (face->tty_bold_p
 	  || face->tty_italic_p
 	  || face->tty_reverse_p
-	  || face->tty_underline_p
+	  || face->underline
 	  || face->tty_strike_through_p)
 	{
 	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
@@ -2007,7 +2026,7 @@ turn_off_face (struct frame *f, int face_id)
     {
       /* If we don't have "me" we can only have those appearances
 	 that have exit sequences defined.  */
-      if (face->tty_underline_p)
+      if (face->underline)
 	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
     }
 
@@ -2036,6 +2055,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
   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,
+		     NC_UNDERLINE);
   TTY_CAPABLE_P_TRY (tty,
 		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
   TTY_CAPABLE_P_TRY (tty,
@@ -4250,6 +4272,30 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   tty->TF_underscore = tgetflag ("ul");
   tty->TF_teleray = tgetflag ("xt");
 
+  /* Styled underlines.  Support for this is provided either by the
+     escape sequence in Smulx or the Su flag.  The latter results in a
+     common default escape sequence and is not recommended.  */
+#ifdef TERMINFO
+	tty->TF_set_underline_style = tigetstr("Smulx");
+	if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
+		tty->TF_set_underline_style = NULL;
+#else
+	tty->TF_set_underline_style = tgetstr("Smulx", address);
+#endif
+	if (!tty->TF_set_underline_style && tgetflag("Su"))
+		/* Default to the kitty escape sequence.  See
+		   https://sw.kovidgoyal.net/kitty/underlines/ */
+		tty->TF_set_underline_style = "\x1b[4:%p1%dm";
+
+	if (tty->TF_set_underline_style)
+		/* This escape sequence for setting the underline color is
+		   consistent with the one described in kitty (see above) and
+		   adapted from the one used by neovim.  This sequence has
+		   been altered from the neovim sequence at
+		   https://github.com/neovim/neovim/blob/42f492ac99058bd1cd56c3c7871e7e464b2a5e24/src/nvim/tui/tui.c
+		   to require only a single parameter, the color index.  */
+		tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 5c47679a994..946ffb344d9 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -171,6 +171,13 @@ #define EMACS_TERMCHAR_H
                                    non-blank position.  Must clear before writing _.  */
   int TF_teleray;               /* termcap xt flag: many weird consequences.
                                    For t1061. */
+  const char *TF_set_underline_style; /* termcap Smulx entry: Switches the underline
+                                         style based on the parameter.  Param should
+                                         be one of: 0 (none), 1 (straight), 2 (double),
+                                         3 (wave), 4 (dotted), or 5 (dashed).  */
+  const char *TF_set_underline_color; /* Enabled when TF_set_underline_style is set:
+                                         Sets the color of the underline.  Accepts a
+                                         single parameter, the color index.  */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index 37b703984be..65118ff020e 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3255,7 +3255,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
                 }
 
               else if (EQ (key, QCstyle)
-                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+                       && !(EQ (val, Qline)
+                            || EQ (val, Qdouble)
+                            || EQ (val, Qwave)
+                            || EQ (val, Qdotted)
+                            || EQ (val, Qdashed)))
                 {
                   valid_p = false;
                   break;
@@ -5204,6 +5208,7 @@ gui_supports_face_attributes_p (struct frame *f,
                                 Lisp_Object attrs[LFACE_VECTOR_SIZE],
                                 struct face *def_face)
 {
+  Lisp_Object val;
   Lisp_Object *def_attrs = def_face->lface;
   Lisp_Object lattrs[LFACE_VECTOR_SIZE];
 
@@ -5298,6 +5303,20 @@ gui_supports_face_attributes_p (struct frame *f,
       return false;
     }
 
+  /* 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 */
+		  }
+	  }
+    }
+
   /* Everything checks out, this face is supported.  */
   return true;
 }
@@ -5390,15 +5409,26 @@ tty_supports_face_attributes_p (struct frame *f,
   val = attrs[LFACE_UNDERLINE_INDEX];
   if (!UNSPECIFIEDP (val))
     {
-      if (STRINGP (val))
-	return false;		/* ttys can't use colored underlines */
-      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
-	return false;		/* ttys can't use wave underlines */
-      else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
-	return false;		/* same as default */
-      else
-	test_caps |= TTY_CAP_UNDERLINE;
-    }
+	if (STRINGP (val))
+		test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	else if (EQ (CAR_SAFE (val), QCstyle))
+	  {
+	if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
+	      || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
+	      || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
+	      || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
+	      || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
+	  {
+		return false; /* Face uses an unsupported underline style.  */
+	  }
+
+	test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	  }
+	else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
+		return false;  /* same as default */
+	else
+		test_caps |= TTY_CAP_UNDERLINE;
+  }
 
   /* inverse video */
   val = attrs[LFACE_INVERSE_INDEX];
@@ -6319,6 +6349,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
                 face->underline = FACE_UNDER_LINE;
               else if (EQ (value, Qwave))
                 face->underline = FACE_UNDER_WAVE;
+              else
+                face->underline = FACE_UNDER_LINE;
             }
 	  else if (EQ (keyword, QCposition))
 	    {
@@ -6453,6 +6485,7 @@ realize_tty_face (struct face_cache *cache,
 {
   struct face *face;
   int weight, slant;
+  Lisp_Object underline;
   bool face_colors_defaulted = false;
   struct frame *f = cache->f;
 
@@ -6472,13 +6505,74 @@ realize_tty_face (struct face_cache *cache,
     face->tty_bold_p = true;
   if (slant != 100)
     face->tty_italic_p = true;
-  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
-    face->tty_underline_p = true;
   if (!NILP (attrs[LFACE_INVERSE_INDEX]))
     face->tty_reverse_p = true;
   if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
     face->tty_strike_through_p = true;
 
+  /* Text underline.  */
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (NILP (underline))
+    {
+	face->underline = FACE_NO_UNDERLINE;
+	face->underline_color = 0;
+    }
+  else if (EQ (underline, Qt))
+    {
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+    }
+  else if (STRINGP (underline))
+    {
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = load_color (f, face, underline, LFACE_UNDERLINE_INDEX);
+    }
+  else if (CONSP (underline))
+    {
+	/* `(:color COLOR :style STYLE)'.
+	   STYLE being one of `line', `double', `wave', `dotted' or `dashed'.  */
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+
+	while (CONSP (underline))
+	  {
+		Lisp_Object keyword, value;
+
+		keyword = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (!CONSP (underline))
+			break;
+		value = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (EQ (keyword, QCcolor))
+		  {
+			if (EQ (value, Qforeground_color))
+				face->underline_color = 0;
+			else if (STRINGP (value))
+				face->underline_color
+				    = load_color (f, face, value,
+				                  LFACE_UNDERLINE_INDEX);
+		  }
+		else if (EQ (keyword, QCstyle))
+		  {
+			if (EQ (value, Qline))
+				face->underline = FACE_UNDER_LINE;
+			else if (EQ (value, Qdouble))
+				face->underline = FACE_DOUBLE_UNDER_LINE;
+			else if (EQ (value, Qwave))
+				face->underline = FACE_UNDER_WAVE;
+			else if (EQ (value, Qdotted))
+				face->underline = FACE_DOTTED_UNDER_LINE;
+			else if (EQ (value, Qdashed))
+				face->underline = FACE_DASHED_UNDER_LINE;
+			else
+				face->underline = FACE_UNDER_LINE;
+		  }
+	  }
+    }
+
   /* Map color names to color indices.  */
   map_tty_color (f, face, LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
   map_tty_color (f, face, LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
@@ -7165,6 +7259,9 @@ syms_of_xfaces (void)
   DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
+  DEFSYM (Qdouble, "double");
+  DEFSYM (Qdotted, "dotted");
+  DEFSYM (Qdashed, "dashed");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qflat_button, "flat-button");
-- 
2.40.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Sat, 22 Apr 2023 09:47:12 +0300

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: 62994 <at> debbugs.gnu.org
> Date: Fri, 21 Apr 2023 19:04:13 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Yes, in standards.info.  This is the GNU recommended style.
> 
> Cool, doesn't seem to be in the Emacs repo but I'm guessing it's this
> [1].

Yes.

> I'll try to adapt to it.

It should be easier now, since .dir-locals.el now does that for
c-ts-mode as well, not just for c-mode.

> > Did Kitty invent that?  If not, there must be some reference that is
> > not the source code of a terminal emulator.
> 
> I think it did. What I've found online indicates kitty invented it, used
> Su for the termcap, this lead to an issue on tmux which suggested
> changing it to Smulx [2]. That's why the termcap for it is inconsistent.
> I can't find any reference for support before kitty.

Are there any terminal emulators besides Kitty which support these
attributes?  They should perhaps be mentioned in the NEWS entry.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Sat, 22 Apr 2023 10:00:46 +0300

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: 62994 <at> debbugs.gnu.org
> Date: Fri, 21 Apr 2023 19:43:07 +0100
> 
> Mohsin Kaleem <mohkale <at> kisara.moe> writes:
> 
> > Cool, doesn't seem to be in the Emacs repo but I'm guessing it's this
> > [1]. I'll try to adapt to it.
> 
> I still don't get how to apply this to this repo Eli. You mentioned using
> tabs and spaces but every lines aside from a nested expression starts
> with 2 spaces. Some times there's 3 levels deep of expressions all
> indented with spaces and then one tab width just wide enough at 8
> characters to appear like it's indented with spaces atop the previous
> scope.
> 
> Can I just do leading indent with tabs matching current indentation
> level, curly braces 2 space ahead of the parent indent, and spaces ahead
> of tabs to align a parenthesized expression or multi line condition with
> the previous line?

Why are you doing this manually?  Emacs does that for you, as soon as
you set indent-tabs-mode to a non-nil value.  .dir-locals.el in the
Emacs Git repository should do that automatically; it already does on
the release branch and soon will be doing that on the master branch as
well.  And to reformat existing code, make a region around it and
invoke "M-x tabify".

There should be no need to insert tabs and spaces manually.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Sat, 22 Apr 2023 10:32:23 +0100

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

> Why are you doing this manually?  Emacs does that for you, as soon as
> you set indent-tabs-mode to a non-nil value.  .dir-locals.el in the
> Emacs Git repository should do that automatically; it already does on
> the release branch and soon will be doing that on the master branch as
> well.  And to reformat existing code, make a region around it and
> invoke "M-x tabify".
>
> There should be no need to insert tabs and spaces manually.

Cool. If that works then I'll just reindent any affected hunks using M-x
tabify :-). As for why I didn't do it before, I suppose because I'm used
to Emacs never being able to indent as it should and then re-indenting on
top of my changes with formatters like clang-format. When I don't have
that the only approach I know of is manual.

-- 
Mohsin Kaleem

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH 2/3] Add support for styled underlines on tty
 frames
Date: Sat, 22 Apr 2023 10:57:46 +0100

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

> Are there any terminal emulators besides Kitty which support these
> attributes?  They should perhaps be mentioned in the NEWS entry.

I've tested the following. I'll add the relevant ones to the NEWS file:

- vte4 (through libvte). Supports both colored and styled underlines,
  but for some reason vte4 uses a TERM of xterm-256color for me so it
  hasn't got the termcaps to expose that it supports it. I guess it
  expects users to override it or is waiting for xterm to support it :/.
- xterm doesn't. I've never used xterm, it might be possible to
  configure it but I won't bother testing too thoroughly.
- St supports styled and colored underlines through the curly underlines
  patch.
- xterm.js apparently supports it [1] but installing hyper to check is
  taking forever so I can't confirm :/. Darn web terminals.

[1]: https://github.com/xtermjs/xterm.js/issues/1145

-- 
Mohsin Kaleem

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

From: mohkale <at> kisara.moe
To: Eli Zaretskii <eliz <at> gnu.org>,
	62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v3 1/1] Add support for colored and styled underlines on tty
 frames
Date: Sat, 22 Apr 2023 11:21:46 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face, face_underline_type, syms_of_xfacse,
internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
Add definitions for new underline styles of Double, Dotted and Dashed.
Delete tty_underline_p from the face struct and use just underline going
forward.  Add a flag to check whether styled underlines are available.
* lisp/cus-face.el (custom-face-attributes): Add entries for Double,
Dotted and Dashed so they can be set through `customize'.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline style and color on terminal frames.
* src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
underline style escape sequence from the Smulx termcap (alternatively if
the Su flag is set use a default sequence).  Allow checking for support
of styled underlines in the current terminal frame.  Output the necessary
escape sequences to activate a styled underline on turn_on_face; this is
currently only used for the new special underline styles, a default
straight underline will still use the "us" termcap.  Output escape
sequence to set underline color when set in the face and supported by
the tty.  Save a default value for this sequence on init_tty when styled
underlines are supported.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face):
Assert whether styled underlines are supported by the current terminal
on display-supports-face-attributes-p checks.  Populate the correct
underline style and color in the face spec when realizing a face.
---
 etc/NEWS         |  13 +++++
 lisp/cus-face.el |   5 +-
 src/dispextern.h |  10 ++--
 src/term.c       |  54 +++++++++++++++++++--
 src/termchar.h   |   7 +++
 src/xfaces.c     | 121 ++++++++++++++++++++++++++++++++++++++++++-----
 6 files changed, 190 insertions(+), 20 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 62d2fdcd3a4..b552bb2d75e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1916,6 +1916,19 @@ This command switches to the "*scratch*" buffer.  If "*scratch*" doesn't
 exist, the command creates it first.  You can use this command if you
 inadvertently delete the "*scratch*" buffer.
 
+---
+** Support for 'styled' and 'colored' underline face attributes on TTY frames
+If your terminals termcap or terminfo database entry has the 'Su' or
+'Smulx' capability defined, Emacs will now emit the prescribed escape
+sequence necessary to render faces with styled underlines on TTY
+frames.
+
+Styled underlines are any underlines containing a non-default
+underline style or a color other than the foreground-color.
+The available underline styles for TTY frames are 'double', 'wave',
+'dotted', and 'dashed'.  These are currently supported by Kitty,
+libvte, and st (through the undercurl patch) among other terminals.
+
 ** Debugging
 
 ---
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index ec89b4f7ff6..2d6e6c7b73e 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -141,7 +141,10 @@ custom-face-attributes
 		   (const :format "" :value :style)
 		   (choice :tag "Style"
 			   (const :tag "Line" line)
-			   (const :tag "Wave" wave))
+			   (const :tag "Double" double)
+			   (const :tag "Wave" wave)
+			   (const :tag "Dotted" dotted)
+			   (const :tag "Dashed" dashed))
                    (const :format "" :value :position)
                    (choice :tag "Position"
                            (const :tag "At Default Position" nil)
diff --git a/src/dispextern.h b/src/dispextern.h
index 4dcab113ea2..753bee446f1 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1653,9 +1653,13 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+	/* Note: Order matches the order of the Smulx terminfo extension. */
   FACE_NO_UNDERLINE = 0,
   FACE_UNDER_LINE,
-  FACE_UNDER_WAVE
+  FACE_DOUBLE_UNDER_LINE,
+  FACE_UNDER_WAVE,
+  FACE_DOTTED_UNDER_LINE,
+  FACE_DASHED_UNDER_LINE,
 };
 
 /* Structure describing a realized face.
@@ -1737,7 +1741,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline : 2;
+  ENUM_BF (face_underline_type) underline : 3;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
@@ -1769,7 +1773,6 @@ #define FONT_TOO_HIGH(ft)						\
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
   bool_bf tty_italic_p : 1;
-  bool_bf tty_underline_p : 1;
   bool_bf tty_reverse_p : 1;
   bool_bf tty_strike_through_p : 1;
 
@@ -3361,6 +3364,7 @@ #define TTY_CAP_BOLD		0x04
 #define TTY_CAP_DIM		0x08
 #define TTY_CAP_ITALIC  	0x10
 #define TTY_CAP_STRIKE_THROUGH	0x20
+#define TTY_CAP_UNDERLINE_STYLED	0x32 & TTY_CAP_UNDERLINE
 
 
 /***********************************************************************
diff --git a/src/term.c b/src/term.c
index 53ba2a231e4..b4fb607ee1f 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1948,8 +1948,19 @@ turn_on_face (struct frame *f, int face_id)
 	OUTPUT1 (tty, tty->TS_enter_dim_mode);
     }
 
-  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
-    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+  if (face->underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
+	{
+	if (face->underline == FACE_UNDER_LINE
+		|| !tty->TF_set_underline_style)
+		OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+	else if (tty->TF_set_underline_style)
+	  {
+		char *p;
+		p = tparam(tty->TF_set_underline_style, NULL, 0, face->underline, 0, 0, 0);
+		OUTPUT (tty, p);
+		xfree (p);
+	  }
+	}
 
   if (face->tty_strike_through_p
       && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
@@ -1975,6 +1986,14 @@ turn_on_face (struct frame *f, int face_id)
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
+
+	  ts = tty->TF_set_underline_color;
+	  if (ts && face->underline_color)
+	{
+	  p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
+	  OUTPUT (tty, p);
+	  xfree (p);
+	}
     }
 }
 
@@ -1995,7 +2014,7 @@ turn_off_face (struct frame *f, int face_id)
       if (face->tty_bold_p
 	  || face->tty_italic_p
 	  || face->tty_reverse_p
-	  || face->tty_underline_p
+	  || face->underline
 	  || face->tty_strike_through_p)
 	{
 	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
@@ -2007,7 +2026,7 @@ turn_off_face (struct frame *f, int face_id)
     {
       /* If we don't have "me" we can only have those appearances
 	 that have exit sequences defined.  */
-      if (face->tty_underline_p)
+      if (face->underline)
 	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
     }
 
@@ -2036,6 +2055,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
   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,
+			 NC_UNDERLINE);
   TTY_CAPABLE_P_TRY (tty,
 		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
   TTY_CAPABLE_P_TRY (tty,
@@ -4250,6 +4272,30 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   tty->TF_underscore = tgetflag ("ul");
   tty->TF_teleray = tgetflag ("xt");
 
+  /* Styled underlines.	 Support for this is provided either by the
+	 escape sequence in Smulx or the Su flag.  The latter results in a
+	 common default escape sequence and is not recommended.	 */
+#ifdef TERMINFO
+	tty->TF_set_underline_style = tigetstr("Smulx");
+	if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
+		tty->TF_set_underline_style = NULL;
+#else
+	tty->TF_set_underline_style = tgetstr("Smulx", address);
+#endif
+	if (!tty->TF_set_underline_style && tgetflag("Su"))
+		/* Default to the kitty escape sequence.  See
+		   https://sw.kovidgoyal.net/kitty/underlines/ */
+		tty->TF_set_underline_style = "\x1b[4:%p1%dm";
+
+	if (tty->TF_set_underline_style)
+		/* This escape sequence for setting the underline color is
+		   consistent with the one described in kitty (see above) and
+		   adapted from the one used by neovim.	 This sequence has
+		   been altered from the neovim sequence at
+		   https://github.com/neovim/neovim/blob/42f492ac99058bd1cd56c3c7871e7e464b2a5e24/src/nvim/tui/tui.c#L1932
+		   to require only a single parameter, the color index.	 */
+		tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 5c47679a994..a9c28fff5cf 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -171,6 +171,13 @@ #define EMACS_TERMCHAR_H
                                    non-blank position.  Must clear before writing _.  */
   int TF_teleray;               /* termcap xt flag: many weird consequences.
                                    For t1061. */
+  const char *TF_set_underline_style; /* termcap Smulx entry: Switches the underline
+										 style based on the parameter.	Param should
+										 be one of: 0 (none), 1 (straight), 2 (double),
+										 3 (wave), 4 (dotted), or 5 (dashed).  */
+  const char *TF_set_underline_color; /* Enabled when TF_set_underline_style is set:
+										 Sets the color of the underline.  Accepts a
+										 single parameter, the color index.	 */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index 37b703984be..f07bb6c8eca 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3255,7 +3255,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
                 }
 
               else if (EQ (key, QCstyle)
-                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+					   && !(EQ (val, Qline)
+							|| EQ (val, Qdouble)
+							|| EQ (val, Qwave)
+							|| EQ (val, Qdotted)
+							|| EQ (val, Qdashed)))
                 {
                   valid_p = false;
                   break;
@@ -5204,6 +5208,7 @@ gui_supports_face_attributes_p (struct frame *f,
                                 Lisp_Object attrs[LFACE_VECTOR_SIZE],
                                 struct face *def_face)
 {
+  Lisp_Object val;
   Lisp_Object *def_attrs = def_face->lface;
   Lisp_Object lattrs[LFACE_VECTOR_SIZE];
 
@@ -5298,6 +5303,20 @@ gui_supports_face_attributes_p (struct frame *f,
       return false;
     }
 
+  /* 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 */
+		  }
+	  }
+	}
+
   /* Everything checks out, this face is supported.  */
   return true;
 }
@@ -5390,15 +5409,26 @@ tty_supports_face_attributes_p (struct frame *f,
   val = attrs[LFACE_UNDERLINE_INDEX];
   if (!UNSPECIFIEDP (val))
     {
-      if (STRINGP (val))
-	return false;		/* ttys can't use colored underlines */
-      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
-	return false;		/* ttys can't use wave underlines */
-      else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
-	return false;		/* same as default */
-      else
-	test_caps |= TTY_CAP_UNDERLINE;
-    }
+	if (STRINGP (val))
+		test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	else if (EQ (CAR_SAFE (val), QCstyle))
+	  {
+	if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
+	  {
+		return false; /* Face uses an unsupported underline style.	*/
+	  }
+
+	test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	  }
+	else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
+		return false;  /* same as default */
+	else
+		test_caps |= TTY_CAP_UNDERLINE;
+  }
 
   /* inverse video */
   val = attrs[LFACE_INVERSE_INDEX];
@@ -6319,6 +6349,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
                 face->underline = FACE_UNDER_LINE;
               else if (EQ (value, Qwave))
                 face->underline = FACE_UNDER_WAVE;
+	      else
+		face->underline = FACE_UNDER_LINE;
             }
 	  else if (EQ (keyword, QCposition))
 	    {
@@ -6453,6 +6485,7 @@ realize_tty_face (struct face_cache *cache,
 {
   struct face *face;
   int weight, slant;
+  Lisp_Object underline;
   bool face_colors_defaulted = false;
   struct frame *f = cache->f;
 
@@ -6472,13 +6505,74 @@ realize_tty_face (struct face_cache *cache,
     face->tty_bold_p = true;
   if (slant != 100)
     face->tty_italic_p = true;
-  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
-    face->tty_underline_p = true;
   if (!NILP (attrs[LFACE_INVERSE_INDEX]))
     face->tty_reverse_p = true;
   if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
     face->tty_strike_through_p = true;
 
+  /* Text underline.  */
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (NILP (underline))
+    {
+	face->underline = FACE_NO_UNDERLINE;
+	face->underline_color = 0;
+    }
+  else if (EQ (underline, Qt))
+    {
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+    }
+  else if (STRINGP (underline))
+    {
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = load_color (f, face, underline, LFACE_UNDERLINE_INDEX);
+    }
+  else if (CONSP (underline))
+    {
+	/* `(:color COLOR :style STYLE)'.
+	   STYLE being one of `line', `double', `wave', `dotted' or `dashed'.  */
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+
+	while (CONSP (underline))
+	  {
+		Lisp_Object keyword, value;
+
+		keyword = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (!CONSP (underline))
+			break;
+		value = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (EQ (keyword, QCcolor))
+		  {
+			if (EQ (value, Qforeground_color))
+				face->underline_color = 0;
+			else if (STRINGP (value))
+				face->underline_color
+				    = load_color (f, face, value,
+						  LFACE_UNDERLINE_INDEX);
+		  }
+		else if (EQ (keyword, QCstyle))
+		  {
+			if (EQ (value, Qline))
+				face->underline = FACE_UNDER_LINE;
+			else if (EQ (value, Qdouble))
+				face->underline = FACE_DOUBLE_UNDER_LINE;
+			else if (EQ (value, Qwave))
+				face->underline = FACE_UNDER_WAVE;
+			else if (EQ (value, Qdotted))
+				face->underline = FACE_DOTTED_UNDER_LINE;
+			else if (EQ (value, Qdashed))
+				face->underline = FACE_DASHED_UNDER_LINE;
+			else
+				face->underline = FACE_UNDER_LINE;
+		  }
+	  }
+    }
+
   /* Map color names to color indices.  */
   map_tty_color (f, face, LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
   map_tty_color (f, face, LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
@@ -7165,6 +7259,9 @@ syms_of_xfaces (void)
   DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
+  DEFSYM (Qdouble, "double");
+  DEFSYM (Qdotted, "dotted");
+  DEFSYM (Qdashed, "dashed");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qflat_button, "flat-button");
-- 
2.40.0

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

From: mohkale <at> kisara.moe
To: Eli Zaretskii <eliz <at> gnu.org>,
	62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v3 0/1] Support styled underlines on tty Emacs frames
Date: Sat, 22 Apr 2023 11:21:45 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

Modern terminals (such as kitty) support setting the style of an
underline with the escape sequence exposed in the Smulx termcap.
This allows for (among others) wavy underlines on terminals.
These terminals also support setting the color of these underlines
using a separate escape sequence that to the best of my knowledge
is not exposed as a termcap but has been adopted by other terminal
supporting editors like neovim.

Version 3: All changed hunks reformatted with M-x tabify.
I think the only comment that hasn't been actioned on is whether to
retain the original order of underline styles in face_underline_type.

Mohsin Kaleem (1):
  Add support for colored and styled underlines on tty frames

 etc/NEWS         |  13 +++++
 lisp/cus-face.el |   5 +-
 src/dispextern.h |  10 ++--
 src/term.c       |  54 +++++++++++++++++++--
 src/termchar.h   |   7 +++
 src/xfaces.c     | 121 ++++++++++++++++++++++++++++++++++++++++++-----
 6 files changed, 190 insertions(+), 20 deletions(-)

-- 
2.40.0

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
-- 

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>, 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v3 0/1] Support styled underlines on tty Emacs frames
Date: Sun, 28 Jan 2024 12:30:25 +0100

mohkale <at> kisara.moe writes:

Hi there,

My copyright situation should be solved now. I was waiting on something
else from my work but would rather not put this off any longer. I've
figured out how to do the correct resolution for color. Only thing
that's confusing me a little is that in src/xfaces:6466 for
map_tty_color it looks like we change the color in the face spec itself
only on MSDOS frames. What are the ramifications of this? Is this logic
we want to keep going forward? For reference I don't this should be done
for the underline spec because unlike foreground and background it can
be a string or a plist containing the color value and we would not want
to update it in place.

-- 
Mohsin Kaleem

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v3 0/1] Support styled underlines on tty Emacs frames
Date: Sun, 28 Jan 2024 14:05:32 +0200

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Date: Sun, 28 Jan 2024 12:30:25 +0100
> 
> mohkale <at> kisara.moe writes:
> 
> My copyright situation should be solved now. I was waiting on something
> else from my work but would rather not put this off any longer.

That's good to know, thanks.

> I've
> figured out how to do the correct resolution for color. Only thing
> that's confusing me a little is that in src/xfaces:6466 for
> map_tty_color it looks like we change the color in the face spec itself
> only on MSDOS frames. What are the ramifications of this? Is this logic
> we want to keep going forward?

The MSDOS frames are different from other TTY frames in that we can
know the default colors used by the MSDOS terminals.  That's why MSDOS
needs a special code there.  And yes, we want to keep this logic, for
MSDOS only.

> For reference I don't this should be done for the underline spec
> because unlike foreground and background it can be a string or a
> plist containing the color value and we would not want to update it
> in place.

The MSDOS terminal doesn't support underline, so no worries here.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v3 0/1] Support styled underlines on tty Emacs frames
Date: Mon, 29 Jan 2024 14:37:03 +0200

[Please use Reply All to reply, to keep the bug tracker CC'ed.]

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Date: Mon, 29 Jan 2024 08:06:38 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > The MSDOS frames are different from other TTY frames in that we can
> > know the default colors used by the MSDOS terminals.  That's why MSDOS
> > needs a special code there.  And yes, we want to keep this logic, for
> > MSDOS only.
> 
> Interesting. Didn't know MSDOS terminals expose that. Neat. Although why
> do we update the face spec (face->lface) itself in this case instead of
> only the foreground/background values which we do later anyways.

AFAIR, it's because those default colors are then used to define the
'default' face and the frame's fore/back-ground pixel values, so we
need the actual colors.

> face->lface seems to be the actual spec set in the theme/customisation
> display (unless I'm misunderstanding). If we change it wouldn't that
> alter what's shown in customize for example.

How can that alter what is shown, when the colors we stick in the face
are the exact colors that the screen uses?  It's like you take a
"blue" color that is specified by the special name "default" and
replace it with an actual "blue" -- how can the result be different.

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org,
	Eli Zaretskii <eliz <at> gnu.org>
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v4 2/3] Add support for styled underlines on tty frames
Date: Sun, 11 Feb 2024 17:15:55 +0000

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face): Convert tty_underline_p from bool to a
face_underline_type enumeration. Add a flag to check whether styled
underlines are available.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline style on terminal frames.
* src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
underline style escape sequence from the Smulx termcap (alternatively if
the Su flag is set use a default sequence). Allow checking for support
of styled underlines in the current terminal frame. Output the necessary
escape sequences to activate a styled underline on turn_on_face; this is
currently only used for the new special underline styles, a default
straight underline will still use the "us" termcap.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face):
Assert whether styled underlines are supported by the current terminal
on display-supports-face-attributes-p checks. Populate the correct
underline style in the face spec when realizing a face.
---
 etc/NEWS         | 16 +++++++++++++
 src/dispextern.h |  3 ++-
 src/term.c       | 35 ++++++++++++++++++++++++----
 src/termchar.h   |  4 ++++
 src/xfaces.c     | 60 ++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 5ee1509859b..d868ee773fd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -420,6 +420,22 @@ These characters can now be input with 'C-x 8 a e' and 'C-x 8 A E',
 respectively, in addition to the existing translations 'C-x 8 / e' and
 'C-x 8 / E'.
 
+---
+** Support for 'styled-underline' face attributes on TTY frames
+If your terminals termcap or terminfo database entry has the 'Su' or
+'Smulx' capability defined, Emacs will now emit the prescribed escape
+sequence necessary to render faces with styled underlines on TTY
+frames.
+
+Styled underlines are any underlines containing a non-default
+underline style.  The available underline styles for TTY frames are
+'double', 'wave', 'dotted', and 'dashed'.
+
+The 'Smulx' capability should define the actual sequence needed to
+render styled underlines. If ommitted, but the 'Su' flag is defined,
+then a default sequence will be used. It's recommended to use 'Smulx'
+instead of 'Su', with priority being given to 'Smulx'.
+
 
 * Changes in Specialized Modes and Packages in Emacs 30.1
 
diff --git a/src/dispextern.h b/src/dispextern.h
index 470d2eea3ae..31e946c387a 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1812,7 +1812,7 @@ #define FONT_TOO_HIGH(ft)						\
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
   bool_bf tty_italic_p : 1;
-  bool_bf tty_underline_p : 1;
+  ENUM_BF (face_underline_type) tty_underline : 3;
   bool_bf tty_reverse_p : 1;
   bool_bf tty_strike_through_p : 1;
 
@@ -3425,6 +3425,7 @@ #define TTY_CAP_BOLD		0x04
 #define TTY_CAP_DIM		0x08
 #define TTY_CAP_ITALIC  	0x10
 #define TTY_CAP_STRIKE_THROUGH	0x20
+#define TTY_CAP_UNDERLINE_STYLED	0x32 & TTY_CAP_UNDERLINE
 
 
 /***********************************************************************
diff --git a/src/term.c b/src/term.c
index 3fa244be824..06dbb95171e 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2014,8 +2014,17 @@ turn_on_face (struct frame *f, int face_id)
 	OUTPUT1 (tty, tty->TS_enter_dim_mode);
     }
 
-  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
-    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+  if (face->tty_underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE)) {
+    if (face->tty_underline == FACE_UNDER_LINE ||
+        !tty->TF_set_underline_style) {
+      OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+    } else if (tty->TF_set_underline_style) {
+      char *p;
+      p = tparam(tty->TF_set_underline_style, NULL, 0, face->tty_underline, 0, 0, 0);
+      OUTPUT (tty, p);
+      xfree (p);
+    }
+  }
 
   if (face->tty_strike_through_p
       && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
@@ -2061,7 +2070,7 @@ turn_off_face (struct frame *f, int face_id)
       if (face->tty_bold_p
 	  || face->tty_italic_p
 	  || face->tty_reverse_p
-	  || face->tty_underline_p
+	  || face->tty_underline
 	  || face->tty_strike_through_p)
 	{
 	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
@@ -2073,7 +2082,7 @@ turn_off_face (struct frame *f, int face_id)
     {
       /* If we don't have "me" we can only have those appearances
 	 that have exit sequences defined.  */
-      if (face->tty_underline_p)
+      if (face->tty_underline)
 	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
     }
 
@@ -2104,6 +2113,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
   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,
+             NC_UNDERLINE);
   TTY_CAPABLE_P_TRY (tty,
 		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
   TTY_CAPABLE_P_TRY (tty,
@@ -4360,6 +4372,21 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   tty->TF_underscore = tgetflag ("ul");
   tty->TF_teleray = tgetflag ("xt");
 
+  // Styled underlines.
+  //
+  // Support for this is provided either by the escape sequence in
+  // Smulx or the Su flag. The latter results in a common default
+  // escape sequence and is not recommended.
+#ifdef TERMINFO
+  tty->TF_set_underline_style = tigetstr("Smulx");
+  if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
+    tty->TF_set_underline_style = NULL;
+#else
+  tty->TF_set_underline_style = tgetstr("Smulx", address);
+#endif
+  if (!tty->TF_set_underline_style && tgetflag("Su"))
+    tty->TF_set_underline_style = "\x1b[4:%p1%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 2d845107e11..7fc1f347230 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -171,6 +171,10 @@ #define EMACS_TERMCHAR_H
                                    non-blank position.  Must clear before writing _.  */
   int TF_teleray;               /* termcap xt flag: many weird consequences.
                                    For t1061. */
+  const char *TF_set_underline_style;   /* termcap Smulx entry: Switches the underline
+                                           style based on the parameter. Param should
+                                           be one of: 0 (none), 1 (straight), 2 (double),
+                                           3 (wave), 4 (dotted), or 5 (dashed). */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index a76a9365b8a..684e72bd897 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5470,8 +5470,18 @@ tty_supports_face_attributes_p (struct frame *f,
     {
       if (STRINGP (val))
 	return false;		/* ttys can't use colored underlines */
-      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
-	return false;		/* ttys can't use wave underlines */
+      else if (EQ (CAR_SAFE (val), QCstyle))
+    {
+        if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qwave) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted) ||
+              EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed))) {
+            return false; /* Unsupported underline style */
+        }
+
+	    test_caps |= TTY_CAP_UNDERLINE_STYLED;
+    }
       else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
 	return false;		/* same as default */
       else
@@ -6397,6 +6407,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
                 face->underline = FACE_UNDER_LINE;
               else if (EQ (value, Qwave))
                 face->underline = FACE_UNDER_WAVE;
+              else
+                face->underline = FACE_UNDER_LINE;
             }
 	  else if (EQ (keyword, QCposition))
 	    {
@@ -6531,6 +6543,7 @@ realize_tty_face (struct face_cache *cache,
 {
   struct face *face;
   int weight, slant;
+  Lisp_Object underline;
   bool face_colors_defaulted = false;
   struct frame *f = cache->f;
 
@@ -6550,13 +6563,52 @@ realize_tty_face (struct face_cache *cache,
     face->tty_bold_p = true;
   if (slant != 100)
     face->tty_italic_p = true;
-  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
-    face->tty_underline_p = true;
   if (!NILP (attrs[LFACE_INVERSE_INDEX]))
     face->tty_reverse_p = true;
   if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
     face->tty_strike_through_p = true;
 
+  /* Text underline.  */
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (NILP (underline)) {
+    face->tty_underline = FACE_NO_UNDERLINE;
+  } else if (EQ (underline, Qt)) {
+    face->tty_underline = FACE_UNDER_LINE;
+  } else if (STRINGP (underline)) {
+    face->tty_underline = FACE_UNDER_LINE;
+  } else if (CONSP (underline)) {
+    /* `(:style STYLE)'.
+       STYLE being one of `line', `double', `wave', `dotted' or `dashed'. */
+    face->tty_underline = FACE_UNDER_LINE;
+
+    while (CONSP (underline)) {
+      Lisp_Object keyword, value;
+
+      keyword = XCAR (underline);
+      underline = XCDR (underline);
+
+      if (!CONSP (underline))
+        break;
+      value = XCAR (underline);
+      underline = XCDR (underline);
+
+      if (EQ (keyword, QCstyle)) {
+        if (EQ (value, Qline))
+          face->tty_underline = FACE_UNDER_LINE;
+        else if (EQ (value, Qdouble))
+          face->tty_underline = FACE_DOUBLE_UNDER_LINE;
+        else if (EQ (value, Qwave))
+          face->tty_underline = FACE_UNDER_WAVE;
+        else if (EQ (value, Qdotted))
+          face->tty_underline = FACE_DOTTED_UNDER_LINE;
+        else if (EQ (value, Qdashed))
+          face->tty_underline = FACE_DASHED_UNDER_LINE;
+        else
+          face->tty_underline = FACE_UNDER_LINE;
+      }
+    }
+  }
+
   /* Map color names to color indices.  */
   map_tty_color (f, face, LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
   map_tty_color (f, face, LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
-- 
2.43.0

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org,
	Eli Zaretskii <eliz <at> gnu.org>
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v4 0/3] Support styled underlines on tty Emacs frames
Date: Sun, 11 Feb 2024 17:15:53 +0000

From: Mohsin Kaleem <mohkale <at> kisara.moe>

Modern terminals (such as kitty) support setting the style of an
underline with the escape sequence exposed in the Smulx termcap.
This allows for (among others) wavy underlines on terminals.
These terminals also support setting the color of these underlines
using a separate escape sequence that to the best of my knowledge
is not exposed as a termcap but has been adopted by other terminal
supporting editors like neovim.

Mohsin Kaleem (3):
  Add face definitions for more underline styles
  Add support for styled underlines on tty frames
  Add support for colored underlines on tty frames

 etc/NEWS         | 19 ++++++++++
 lisp/cus-face.el |  5 ++-
 src/dispextern.h | 11 ++++--
 src/term.c       | 46 +++++++++++++++++++++---
 src/termchar.h   |  7 ++++
 src/xfaces.c     | 93 ++++++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 167 insertions(+), 14 deletions(-)

-- 
2.43.0

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org,
	Eli Zaretskii <eliz <at> gnu.org>
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v4 1/3] Add face definitions for more underline styles
Date: Sun, 11 Feb 2024 17:15:54 +0000

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face_underline_type, syms_of_xfacse,
internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
Add definitions for new underline styles of Double, Dotted and Dashed.
* lisp/cus-face.el (custom-face-attributes): Add entries for Double,
Dotted and Dashed so they can be set through `customize'.
---
 lisp/cus-face.el |  5 ++++-
 src/dispextern.h |  8 ++++++--
 src/xfaces.c     | 21 ++++++++++++++++++++-
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index 47afa841f5e..12551e37785 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -141,7 +141,10 @@ custom-face-attributes
 		   (const :format "" :value :style)
 		   (choice :tag "Style"
 			   (const :tag "Line" line)
-			   (const :tag "Wave" wave))
+			   (const :tag "Double" double)
+			   (const :tag "Wave" wave)
+			   (const :tag "Dotted" dotted)
+			   (const :tag "Dashed" dashed))
                    (const :format "" :value :position)
                    (choice :tag "Position"
                            (const :tag "At Default Position" nil)
diff --git a/src/dispextern.h b/src/dispextern.h
index 5387cb45603..470d2eea3ae 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1690,9 +1690,13 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+  // Note: Order matches the order of the Smulx terminfo extension.
   FACE_NO_UNDERLINE = 0,
   FACE_UNDER_LINE,
-  FACE_UNDER_WAVE
+  FACE_DOUBLE_UNDER_LINE,
+  FACE_UNDER_WAVE,
+  FACE_DOTTED_UNDER_LINE,
+  FACE_DASHED_UNDER_LINE,
 };
 
 /* Structure describing a realized face.
@@ -1776,7 +1780,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline : 2;
+  ENUM_BF (face_underline_type) underline : 3;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
diff --git a/src/xfaces.c b/src/xfaces.c
index a558e7328c0..a76a9365b8a 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3311,7 +3311,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
                 }
 
               else if (EQ (key, QCstyle)
-                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+                       && !(EQ (val, Qline) ||
+                            EQ (val, Qdouble) ||
+                            EQ (val, Qwave) ||
+                            EQ (val, Qdotted) ||
+                            EQ (val, Qdashed)))
                 {
                   valid_p = false;
                   break;
@@ -5266,6 +5270,7 @@ gui_supports_face_attributes_p (struct frame *f,
                                 Lisp_Object attrs[LFACE_VECTOR_SIZE],
                                 struct face *def_face)
 {
+  Lisp_Object val;
   Lisp_Object *def_attrs = def_face->lface;
   Lisp_Object lattrs[LFACE_VECTOR_SIZE];
 
@@ -5360,6 +5365,17 @@ gui_supports_face_attributes_p (struct frame *f,
       return false;
     }
 
+  /* 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 */
+      }
+    }
+  }
+
   /* Everything checks out, this face is supported.  */
   return true;
 }
@@ -7229,6 +7245,9 @@ syms_of_xfaces (void)
   DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
+  DEFSYM (Qdouble, "double");
+  DEFSYM (Qdotted, "dotted");
+  DEFSYM (Qdashed, "dashed");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qflat_button, "flat-button");
-- 
2.43.0

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: 62994 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: [PATCH v4 0/3] Support styled underlines on tty Emacs frames
Date: Sun, 11 Feb 2024 17:23:58 +0000

mohkale <at> kisara.moe writes:

Hi, sorry, please disregard this email. I seem to have messed up and
published the wrong patchset and also to the wrong email chain.


-- 
Mohsin Kaleem

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org,
	Eli Zaretskii <eliz <at> gnu.org>
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v4 3/3] Add support for colored underlines on tty frames
Date: Sun, 11 Feb 2024 17:15:56 +0000

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/term.c (turn_on_face, init_tty): Output escape sequence to set
underline color when set in the face and supported by the tty. Save
a default value for this sequence on init_tty when styled underlines
are supported.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline color on terminal frames.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face):
Assert whether colored underlines are supported by the current terminal
on display-supports-face-attributes-p checks. Load and save the color
of the underline from the face spec when realizing a face.
---
 etc/NEWS       | 11 +++++++----
 src/term.c     | 11 +++++++++++
 src/termchar.h |  3 +++
 src/xfaces.c   | 16 +++++++++++++---
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d868ee773fd..73a15eb44a4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -421,20 +421,23 @@ respectively, in addition to the existing translations 'C-x 8 / e' and
 'C-x 8 / E'.
 
 ---
-** Support for 'styled-underline' face attributes on TTY frames
+** Support for 'styled' and 'colored' underline face attributes on TTY frames
 If your terminals termcap or terminfo database entry has the 'Su' or
 'Smulx' capability defined, Emacs will now emit the prescribed escape
 sequence necessary to render faces with styled underlines on TTY
 frames.
 
 Styled underlines are any underlines containing a non-default
-underline style.  The available underline styles for TTY frames are
-'double', 'wave', 'dotted', and 'dashed'.
+underline style or a color other than the foreground-color.
+The available underline styles for TTY frames are 'double', 'wave',
+'dotted', and 'dashed'.
 
 The 'Smulx' capability should define the actual sequence needed to
 render styled underlines. If ommitted, but the 'Su' flag is defined,
 then a default sequence will be used. It's recommended to use 'Smulx'
-instead of 'Su', with priority being given to 'Smulx'.
+instead of 'Su', with priority being given to 'Smulx'.  Support for
+colored underlines is automatically enabled with a default escape
+sequence when styled underline are supported.
 
 
 * Changes in Specialized Modes and Packages in Emacs 30.1
diff --git a/src/term.c b/src/term.c
index 06dbb95171e..1ab6936db76 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2050,6 +2050,14 @@ turn_on_face (struct frame *f, int face_id)
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
+
+      ts = tty->TF_set_underline_color;
+      if (ts && face->underline_color)
+	{
+          p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
+	  OUTPUT (tty, p);
+	  xfree (p);
+	}
     }
 }
 
@@ -4387,6 +4395,9 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   if (!tty->TF_set_underline_style && tgetflag("Su"))
     tty->TF_set_underline_style = "\x1b[4:%p1%dm";
 
+  if (tty->TF_set_underline_style)
+    tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 7fc1f347230..846cd3001cc 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -175,6 +175,9 @@ #define EMACS_TERMCHAR_H
                                            style based on the parameter. Param should
                                            be one of: 0 (none), 1 (straight), 2 (double),
                                            3 (wave), 4 (dotted), or 5 (dashed). */
+  const char *TF_set_underline_color;   /* Enabled when TF_set_underline_style is set:
+                                           Sets the color of the underline. Accepts a
+                                           single parameter, the color index. */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index 684e72bd897..4d860540cb9 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5469,7 +5469,7 @@ tty_supports_face_attributes_p (struct frame *f,
   if (!UNSPECIFIEDP (val))
     {
       if (STRINGP (val))
-	return false;		/* ttys can't use colored underlines */
+	test_caps |= TTY_CAP_UNDERLINE_STYLED;
       else if (EQ (CAR_SAFE (val), QCstyle))
     {
         if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline) ||
@@ -6572,14 +6572,18 @@ realize_tty_face (struct face_cache *cache,
   underline = attrs[LFACE_UNDERLINE_INDEX];
   if (NILP (underline)) {
     face->tty_underline = FACE_NO_UNDERLINE;
+    face->underline_color = 0;
   } else if (EQ (underline, Qt)) {
     face->tty_underline = FACE_UNDER_LINE;
+    face->underline_color = 0;
   } else if (STRINGP (underline)) {
     face->tty_underline = FACE_UNDER_LINE;
+	face->underline_color = load_color (f, face, underline, LFACE_UNDERLINE_INDEX);
   } else if (CONSP (underline)) {
-    /* `(:style STYLE)'.
+    /* `(:color COLOR :style STYLE)'.
        STYLE being one of `line', `double', `wave', `dotted' or `dashed'. */
     face->tty_underline = FACE_UNDER_LINE;
+    face->underline_color = 0;
 
     while (CONSP (underline)) {
       Lisp_Object keyword, value;
@@ -6592,7 +6596,13 @@ realize_tty_face (struct face_cache *cache,
       value = XCAR (underline);
       underline = XCDR (underline);
 
-      if (EQ (keyword, QCstyle)) {
+      if (EQ (keyword, QCcolor)) {
+        if (EQ (value, Qforeground_color))
+          face->underline_color = 0;
+        else if (STRINGP (value))
+          face->underline_color = load_color (f, face, value, LFACE_UNDERLINE_INDEX);
+      }
+      else if (EQ (keyword, QCstyle)) {
         if (EQ (value, Qline))
           face->tty_underline = FACE_UNDER_LINE;
         else if (EQ (value, Qdouble))
-- 
2.43.0

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org,
	Eli Zaretskii <eliz <at> gnu.org>
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v4] Add support for colored and styled underlines on tty frames
Date: Sun, 11 Feb 2024 17:40:35 +0000

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face, face_underline_type, syms_of_xfacse,
internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
Add definitions for new underline styles of Double, Dotted and Dashed.
Delete tty_underline_p from the face struct and use just underline going
forward.  Add a flag to check whether styled underlines are available.
* lisp/cus-face.el (custom-face-attributes): Add entries for Double,
Dotted and Dashed so they can be set through `customize'.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline style and color on terminal frames.
* src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
underline style escape sequence from the Smulx termcap (alternatively if
the Su flag is set use a default sequence).  Allow checking for support
of styled underlines in the current terminal frame.  Output the necessary
escape sequences to activate a styled underline on turn_on_face; this is
currently only used for the new special underline styles, a default
straight underline will still use the "us" termcap.  Output escape
sequence to set underline color when set in the face and supported by
the tty.  Save a default value for this sequence on init_tty when styled
underlines are supported.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face):
Assert whether styled underlines are supported by the current terminal
on display-supports-face-attributes-p checks.  Populate the correct
underline style and color in the face spec when realizing a face.
---
 etc/NEWS         |  13 ++++
 lisp/cus-face.el |   5 +-
 src/dispextern.h |  10 ++-
 src/term.c       |  54 +++++++++++++--
 src/termchar.h   |   7 ++
 src/xfaces.c     | 167 +++++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 235 insertions(+), 21 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 5ee1509859b..e6192e09f81 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -311,6 +311,19 @@ Previously, it was set to t but this broke remote file name detection.
 ** Multi-character key echo now ends with a suggestion to use Help.
 Customize 'echo-keystrokes-help' to nil to prevent that.
 
+---
+** Support for 'styled' and 'colored' underline face attributes on TTY frames
+If your terminals termcap or terminfo database entry has the 'Su' or
+'Smulx' capability defined, Emacs will now emit the prescribed escape
+sequence necessary to render faces with styled underlines on TTY
+frames.
+
+Styled underlines are any underlines containing a non-default
+underline style or a color other than the foreground-color.
+The available underline styles for TTY frames are 'double', 'wave',
+'dotted', and 'dashed'.  These are currently supported by Kitty,
+libvte, and st (through the undercurl patch) among other terminals.
+
 
 * Editing Changes in Emacs 30.1
 
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index 47afa841f5e..12551e37785 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -141,7 +141,10 @@ custom-face-attributes
 		   (const :format "" :value :style)
 		   (choice :tag "Style"
 			   (const :tag "Line" line)
-			   (const :tag "Wave" wave))
+			   (const :tag "Double" double)
+			   (const :tag "Wave" wave)
+			   (const :tag "Dotted" dotted)
+			   (const :tag "Dashed" dashed))
                    (const :format "" :value :position)
                    (choice :tag "Position"
                            (const :tag "At Default Position" nil)
diff --git a/src/dispextern.h b/src/dispextern.h
index 5387cb45603..574798fc547 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1690,9 +1690,13 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+	/* Note: Order matches the order of the Smulx terminfo extension. */
   FACE_NO_UNDERLINE = 0,
   FACE_UNDER_LINE,
-  FACE_UNDER_WAVE
+  FACE_DOUBLE_UNDER_LINE,
+  FACE_UNDER_WAVE,
+  FACE_DOTTED_UNDER_LINE,
+  FACE_DASHED_UNDER_LINE,
 };
 
 /* Structure describing a realized face.
@@ -1776,7 +1780,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline : 2;
+  ENUM_BF (face_underline_type) underline : 3;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
@@ -1808,7 +1812,6 @@ #define FONT_TOO_HIGH(ft)						\
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
   bool_bf tty_italic_p : 1;
-  bool_bf tty_underline_p : 1;
   bool_bf tty_reverse_p : 1;
   bool_bf tty_strike_through_p : 1;
 
@@ -3421,6 +3424,7 @@ #define TTY_CAP_BOLD		0x04
 #define TTY_CAP_DIM		0x08
 #define TTY_CAP_ITALIC  	0x10
 #define TTY_CAP_STRIKE_THROUGH	0x20
+#define TTY_CAP_UNDERLINE_STYLED	0x32 & TTY_CAP_UNDERLINE
 
 
 /***********************************************************************
diff --git a/src/term.c b/src/term.c
index 3fa244be824..d5a5442cb19 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2014,8 +2014,19 @@ turn_on_face (struct frame *f, int face_id)
 	OUTPUT1 (tty, tty->TS_enter_dim_mode);
     }
 
-  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
-    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+  if (face->underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
+	{
+	if (face->underline == FACE_UNDER_LINE
+		|| !tty->TF_set_underline_style)
+		OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+	else if (tty->TF_set_underline_style)
+	  {
+		char *p;
+		p = tparam(tty->TF_set_underline_style, NULL, 0, face->underline, 0, 0, 0);
+		OUTPUT (tty, p);
+		xfree (p);
+	  }
+	}
 
   if (face->tty_strike_through_p
       && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
@@ -2041,6 +2052,14 @@ turn_on_face (struct frame *f, int face_id)
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
+
+	  ts = tty->TF_set_underline_color;
+	  if (ts && face->underline_color)
+	{
+	  p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
+	  OUTPUT (tty, p);
+	  xfree (p);
+	}
     }
 }
 
@@ -2061,7 +2080,7 @@ turn_off_face (struct frame *f, int face_id)
       if (face->tty_bold_p
 	  || face->tty_italic_p
 	  || face->tty_reverse_p
-	  || face->tty_underline_p
+	  || face->underline
 	  || face->tty_strike_through_p)
 	{
 	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
@@ -2073,7 +2092,7 @@ turn_off_face (struct frame *f, int face_id)
     {
       /* If we don't have "me" we can only have those appearances
 	 that have exit sequences defined.  */
-      if (face->tty_underline_p)
+      if (face->underline)
 	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
     }
 
@@ -2104,6 +2123,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
   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,
+			 NC_UNDERLINE);
   TTY_CAPABLE_P_TRY (tty,
 		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
   TTY_CAPABLE_P_TRY (tty,
@@ -4360,6 +4382,30 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   tty->TF_underscore = tgetflag ("ul");
   tty->TF_teleray = tgetflag ("xt");
 
+  /* Styled underlines.	 Support for this is provided either by the
+	 escape sequence in Smulx or the Su flag.  The latter results in a
+	 common default escape sequence and is not recommended.	 */
+#ifdef TERMINFO
+	tty->TF_set_underline_style = tigetstr("Smulx");
+	if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
+		tty->TF_set_underline_style = NULL;
+#else
+	tty->TF_set_underline_style = tgetstr("Smulx", address);
+#endif
+	if (!tty->TF_set_underline_style && tgetflag("Su"))
+		/* Default to the kitty escape sequence.  See
+		   https://sw.kovidgoyal.net/kitty/underlines/ */
+		tty->TF_set_underline_style = "\x1b[4:%p1%dm";
+
+	if (tty->TF_set_underline_style)
+		/* This escape sequence for setting the underline color is
+		   consistent with the one described in kitty (see above) and
+		   adapted from the one used by neovim.	 This sequence has
+		   been altered from the neovim sequence at
+		   https://github.com/neovim/neovim/blob/42f492ac99058bd1cd56c3c7871e7e464b2a5e24/src/nvim/tui/tui.c#L1932
+		   to require only a single parameter, the color index.	 */
+		tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 2d845107e11..de9009d32f1 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -171,6 +171,13 @@ #define EMACS_TERMCHAR_H
                                    non-blank position.  Must clear before writing _.  */
   int TF_teleray;               /* termcap xt flag: many weird consequences.
                                    For t1061. */
+  const char *TF_set_underline_style; /* termcap Smulx entry: Switches the underline
+										 style based on the parameter.	Param should
+										 be one of: 0 (none), 1 (straight), 2 (double),
+										 3 (wave), 4 (dotted), or 5 (dashed).  */
+  const char *TF_set_underline_color; /* Enabled when TF_set_underline_style is set:
+										 Sets the color of the underline.  Accepts a
+										 single parameter, the color index.	 */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index a558e7328c0..75fae11dca3 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3311,7 +3311,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
                 }
 
               else if (EQ (key, QCstyle)
-                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+					   && !(EQ (val, Qline)
+							|| EQ (val, Qdouble)
+							|| EQ (val, Qwave)
+							|| EQ (val, Qdotted)
+							|| EQ (val, Qdashed)))
                 {
                   valid_p = false;
                   break;
@@ -5266,6 +5270,7 @@ gui_supports_face_attributes_p (struct frame *f,
                                 Lisp_Object attrs[LFACE_VECTOR_SIZE],
                                 struct face *def_face)
 {
+  Lisp_Object val;
   Lisp_Object *def_attrs = def_face->lface;
   Lisp_Object lattrs[LFACE_VECTOR_SIZE];
 
@@ -5360,6 +5365,20 @@ gui_supports_face_attributes_p (struct frame *f,
       return false;
     }
 
+  /* 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 */
+		  }
+	  }
+	}
+
   /* Everything checks out, this face is supported.  */
   return true;
 }
@@ -5452,15 +5471,26 @@ tty_supports_face_attributes_p (struct frame *f,
   val = attrs[LFACE_UNDERLINE_INDEX];
   if (!UNSPECIFIEDP (val))
     {
-      if (STRINGP (val))
-	return false;		/* ttys can't use colored underlines */
-      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
-	return false;		/* ttys can't use wave underlines */
-      else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
-	return false;		/* same as default */
-      else
-	test_caps |= TTY_CAP_UNDERLINE;
-    }
+	if (STRINGP (val))
+		test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	else if (EQ (CAR_SAFE (val), QCstyle))
+	  {
+	if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
+	  {
+		return false; /* Face uses an unsupported underline style.	*/
+	  }
+
+	test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	  }
+	else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
+		return false;  /* same as default */
+	else
+		test_caps |= TTY_CAP_UNDERLINE;
+  }
 
   /* inverse video */
   val = attrs[LFACE_INVERSE_INDEX];
@@ -6381,6 +6411,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
                 face->underline = FACE_UNDER_LINE;
               else if (EQ (value, Qwave))
                 face->underline = FACE_UNDER_WAVE;
+	      else
+		face->underline = FACE_UNDER_LINE;
             }
 	  else if (EQ (keyword, QCposition))
 	    {
@@ -6434,7 +6466,53 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
 /* Map a specified color of face FACE on frame F to a tty color index.
    IDX is either LFACE_FOREGROUND_INDEX or LFACE_BACKGROUND_INDEX, and
    specifies which color to map.  Set *DEFAULTED to true if mapping to the
-   default foreground/background colors.  */
+   default foreground/background colors. TODO(me): Update docstring. */
+
+static void
+my_map_tty_color (struct frame *f, struct face *face, Lisp_Object color,
+		  enum lface_attribute_index idx)
+{
+	Lisp_Object frame, def;
+	unsigned long default_pixel = FACE_TTY_DEFAULT_FG_COLOR;
+	unsigned long pixel = default_pixel;
+
+	eassert (idx == LFACE_FOREGROUND_INDEX
+		 || idx == LFACE_BACKGROUND_INDEX
+		 || idx == LFACE_UNDERLINE_INDEX);
+
+	XSETFRAME (frame, f);
+
+	if (STRINGP (color)
+	    && SCHARS (color)
+	    && CONSP (Vtty_defined_color_alist)
+	    && (def = assoc_no_quit (color, call1 (Qtty_color_alist, frame)),
+		CONSP (def)))
+	{
+		/* Associations in tty-defined-color-alist are of the form
+		   (NAME INDEX R G B).  We need the INDEX part.  */
+		pixel = XFIXNUM (XCAR (XCDR (def)));
+	}
+
+	if (pixel == default_pixel && STRINGP (color))
+	{
+		pixel = load_color (f, face, color, idx);
+	}
+
+	switch (idx)
+	{
+		case LFACE_UNDERLINE_INDEX:
+			face->underline_color = pixel;
+			break;
+		case LFACE_FOREGROUND_INDEX:
+			face->foreground = pixel;
+			break;
+		case LFACE_BACKGROUND_INDEX:
+			face->background = pixel;
+			break;
+		default:
+			emacs_abort ();
+	}
+}
 
 static void
 map_tty_color (struct frame *f, struct face *face,
@@ -6515,6 +6593,7 @@ realize_tty_face (struct face_cache *cache,
 {
   struct face *face;
   int weight, slant;
+  Lisp_Object underline;
   bool face_colors_defaulted = false;
   struct frame *f = cache->f;
 
@@ -6534,13 +6613,72 @@ realize_tty_face (struct face_cache *cache,
     face->tty_bold_p = true;
   if (slant != 100)
     face->tty_italic_p = true;
-  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
-    face->tty_underline_p = true;
   if (!NILP (attrs[LFACE_INVERSE_INDEX]))
     face->tty_reverse_p = true;
   if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
     face->tty_strike_through_p = true;
 
+  /* Text underline.  */
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (NILP (underline))
+    {
+	face->underline = FACE_NO_UNDERLINE;
+	face->underline_color = 0;
+    }
+  else if (EQ (underline, Qt))
+    {
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+    }
+  else if (STRINGP (underline))
+    {
+	face->underline = FACE_UNDER_LINE;
+	my_map_tty_color (f, face, underline, LFACE_UNDERLINE_INDEX);
+    }
+  else if (CONSP (underline))
+    {
+	/* `(:color COLOR :style STYLE)'.
+	   STYLE being one of `line', `double', `wave', `dotted' or `dashed'.  */
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+
+	while (CONSP (underline))
+	  {
+		Lisp_Object keyword, value;
+
+		keyword = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (!CONSP (underline))
+			break;
+		value = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (EQ (keyword, QCcolor))
+		  {
+			if (EQ (value, Qforeground_color))
+				face->underline_color = 0;
+			else if (STRINGP (value))
+				my_map_tty_color (f, face, value, LFACE_UNDERLINE_INDEX);
+		  }
+		else if (EQ (keyword, QCstyle))
+		  {
+			if (EQ (value, Qline))
+				face->underline = FACE_UNDER_LINE;
+			else if (EQ (value, Qdouble))
+				face->underline = FACE_DOUBLE_UNDER_LINE;
+			else if (EQ (value, Qwave))
+				face->underline = FACE_UNDER_WAVE;
+			else if (EQ (value, Qdotted))
+				face->underline = FACE_DOTTED_UNDER_LINE;
+			else if (EQ (value, Qdashed))
+				face->underline = FACE_DASHED_UNDER_LINE;
+			else
+				face->underline = FACE_UNDER_LINE;
+		  }
+	  }
+    }
+
   /* Map color names to color indices.  */
   map_tty_color (f, face, LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
   map_tty_color (f, face, LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
@@ -7229,6 +7367,9 @@ syms_of_xfaces (void)
   DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
+  DEFSYM (Qdouble, "double");
+  DEFSYM (Qdotted, "dotted");
+  DEFSYM (Qdashed, "dashed");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qflat_button, "flat-button");
-- 
2.43.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v4 0/3] Support styled underlines on tty Emacs frames
Date: Sun, 11 Feb 2024 19:46:58 +0200

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Date: Sun, 11 Feb 2024 17:23:58 +0000
> 
> mohkale <at> kisara.moe writes:
> 
> Hi, sorry, please disregard this email. I seem to have messed up and
> published the wrong patchset and also to the wrong email chain.

If you are going to post this patch again, please note that I prefer
to see a single large patch instead of a series of smaller ones,
especially if some files are touched by more than one patch in the
series.  Having a single patch makes reviewing the changeset easier.

TIA

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org,
	Eli Zaretskii <eliz <at> gnu.org>
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v5] Add support for colored and styled underlines on tty frames
Date: Sun, 11 Feb 2024 18:05:01 +0000

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face, face_underline_type, syms_of_xfacse,
internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
Add definitions for new underline styles of Double, Dotted and Dashed.
Delete tty_underline_p from the face struct and use just underline going
forward.  Add a flag to check whether styled underlines are available.
* lisp/cus-face.el (custom-face-attributes): Add entries for Double,
Dotted and Dashed so they can be set through `customize'.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline style and color on terminal frames.
* src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
underline style escape sequence from the Smulx termcap (alternatively if
the Su flag is set use a default sequence).  Allow checking for support
of styled underlines in the current terminal frame.  Output the necessary
escape sequences to activate a styled underline on turn_on_face; this is
currently only used for the new special underline styles, a default
straight underline will still use the "us" termcap.  Output escape
sequence to set underline color when set in the face and supported by
the tty.  Save a default value for this sequence on init_tty when styled
underlines are supported.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face,
map_tty_color, map_tty_color2): Assert whether styled underlines are
supported by the current terminal on
display-supports-face-attributes-p checks.  Populate the correct
underline style and color in the face spec when realizing a face.
Allow map_tty_color to map underline colors alongside foreground and
background.  The interface of map_tty_color was amended to allow
the caller to supply the underline color instead of accessing it
through the face attributes.  A new variant map_tty_color2 was added
for contexts where caller doesn't care about foreground/background
face defaulting.
---
 etc/NEWS         |  15 +++++
 lisp/cus-face.el |   5 +-
 src/dispextern.h |  10 ++-
 src/term.c       |  54 +++++++++++++--
 src/termchar.h   |   7 ++
 src/xfaces.c     | 171 +++++++++++++++++++++++++++++++++++++++--------
 6 files changed, 227 insertions(+), 35 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 5ee1509859b..d8cf8671d9d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -311,6 +311,21 @@ Previously, it was set to t but this broke remote file name detection.
 ** Multi-character key echo now ends with a suggestion to use Help.
 Customize 'echo-keystrokes-help' to nil to prevent that.
 
+** Terminal Emacs
+
++++
+*** Support for 'styled' and 'colored' underline face attributes on TTY frames
+If your terminals termcap or terminfo database entry has the 'Su' or
+'Smulx' capability defined, Emacs will now emit the prescribed escape
+sequence necessary to render faces with styled underlines on TTY
+frames.
+
+Styled underlines are any underlines containing a non-default
+underline style or a color other than the foreground-color.
+The available underline styles for TTY frames are 'double', 'wave',
+'dotted', and 'dashed'.  These are currently supported by Kitty,
+libvte, and st (through the undercurl patch) among other terminals.
+
 
 * Editing Changes in Emacs 30.1
 
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index 47afa841f5e..12551e37785 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -141,7 +141,10 @@ custom-face-attributes
 		   (const :format "" :value :style)
 		   (choice :tag "Style"
 			   (const :tag "Line" line)
-			   (const :tag "Wave" wave))
+			   (const :tag "Double" double)
+			   (const :tag "Wave" wave)
+			   (const :tag "Dotted" dotted)
+			   (const :tag "Dashed" dashed))
                    (const :format "" :value :position)
                    (choice :tag "Position"
                            (const :tag "At Default Position" nil)
diff --git a/src/dispextern.h b/src/dispextern.h
index 5387cb45603..574798fc547 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1690,9 +1690,13 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+	/* Note: Order matches the order of the Smulx terminfo extension. */
   FACE_NO_UNDERLINE = 0,
   FACE_UNDER_LINE,
-  FACE_UNDER_WAVE
+  FACE_DOUBLE_UNDER_LINE,
+  FACE_UNDER_WAVE,
+  FACE_DOTTED_UNDER_LINE,
+  FACE_DASHED_UNDER_LINE,
 };
 
 /* Structure describing a realized face.
@@ -1776,7 +1780,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline : 2;
+  ENUM_BF (face_underline_type) underline : 3;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
@@ -1808,7 +1812,6 @@ #define FONT_TOO_HIGH(ft)						\
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
   bool_bf tty_italic_p : 1;
-  bool_bf tty_underline_p : 1;
   bool_bf tty_reverse_p : 1;
   bool_bf tty_strike_through_p : 1;
 
@@ -3421,6 +3424,7 @@ #define TTY_CAP_BOLD		0x04
 #define TTY_CAP_DIM		0x08
 #define TTY_CAP_ITALIC  	0x10
 #define TTY_CAP_STRIKE_THROUGH	0x20
+#define TTY_CAP_UNDERLINE_STYLED	0x32 & TTY_CAP_UNDERLINE
 
 
 /***********************************************************************
diff --git a/src/term.c b/src/term.c
index 3fa244be824..d5a5442cb19 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2014,8 +2014,19 @@ turn_on_face (struct frame *f, int face_id)
 	OUTPUT1 (tty, tty->TS_enter_dim_mode);
     }
 
-  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
-    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+  if (face->underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
+	{
+	if (face->underline == FACE_UNDER_LINE
+		|| !tty->TF_set_underline_style)
+		OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+	else if (tty->TF_set_underline_style)
+	  {
+		char *p;
+		p = tparam(tty->TF_set_underline_style, NULL, 0, face->underline, 0, 0, 0);
+		OUTPUT (tty, p);
+		xfree (p);
+	  }
+	}
 
   if (face->tty_strike_through_p
       && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
@@ -2041,6 +2052,14 @@ turn_on_face (struct frame *f, int face_id)
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
+
+	  ts = tty->TF_set_underline_color;
+	  if (ts && face->underline_color)
+	{
+	  p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
+	  OUTPUT (tty, p);
+	  xfree (p);
+	}
     }
 }
 
@@ -2061,7 +2080,7 @@ turn_off_face (struct frame *f, int face_id)
       if (face->tty_bold_p
 	  || face->tty_italic_p
 	  || face->tty_reverse_p
-	  || face->tty_underline_p
+	  || face->underline
 	  || face->tty_strike_through_p)
 	{
 	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
@@ -2073,7 +2092,7 @@ turn_off_face (struct frame *f, int face_id)
     {
       /* If we don't have "me" we can only have those appearances
 	 that have exit sequences defined.  */
-      if (face->tty_underline_p)
+      if (face->underline)
 	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
     }
 
@@ -2104,6 +2123,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
   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,
+			 NC_UNDERLINE);
   TTY_CAPABLE_P_TRY (tty,
 		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
   TTY_CAPABLE_P_TRY (tty,
@@ -4360,6 +4382,30 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   tty->TF_underscore = tgetflag ("ul");
   tty->TF_teleray = tgetflag ("xt");
 
+  /* Styled underlines.	 Support for this is provided either by the
+	 escape sequence in Smulx or the Su flag.  The latter results in a
+	 common default escape sequence and is not recommended.	 */
+#ifdef TERMINFO
+	tty->TF_set_underline_style = tigetstr("Smulx");
+	if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
+		tty->TF_set_underline_style = NULL;
+#else
+	tty->TF_set_underline_style = tgetstr("Smulx", address);
+#endif
+	if (!tty->TF_set_underline_style && tgetflag("Su"))
+		/* Default to the kitty escape sequence.  See
+		   https://sw.kovidgoyal.net/kitty/underlines/ */
+		tty->TF_set_underline_style = "\x1b[4:%p1%dm";
+
+	if (tty->TF_set_underline_style)
+		/* This escape sequence for setting the underline color is
+		   consistent with the one described in kitty (see above) and
+		   adapted from the one used by neovim.	 This sequence has
+		   been altered from the neovim sequence at
+		   https://github.com/neovim/neovim/blob/42f492ac99058bd1cd56c3c7871e7e464b2a5e24/src/nvim/tui/tui.c#L1932
+		   to require only a single parameter, the color index.	 */
+		tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 2d845107e11..de9009d32f1 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -171,6 +171,13 @@ #define EMACS_TERMCHAR_H
                                    non-blank position.  Must clear before writing _.  */
   int TF_teleray;               /* termcap xt flag: many weird consequences.
                                    For t1061. */
+  const char *TF_set_underline_style; /* termcap Smulx entry: Switches the underline
+										 style based on the parameter.	Param should
+										 be one of: 0 (none), 1 (straight), 2 (double),
+										 3 (wave), 4 (dotted), or 5 (dashed).  */
+  const char *TF_set_underline_color; /* Enabled when TF_set_underline_style is set:
+										 Sets the color of the underline.  Accepts a
+										 single parameter, the color index.	 */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index a558e7328c0..a39e2bb6781 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3311,7 +3311,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
                 }
 
               else if (EQ (key, QCstyle)
-                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+					   && !(EQ (val, Qline)
+							|| EQ (val, Qdouble)
+							|| EQ (val, Qwave)
+							|| EQ (val, Qdotted)
+							|| EQ (val, Qdashed)))
                 {
                   valid_p = false;
                   break;
@@ -5266,6 +5270,7 @@ gui_supports_face_attributes_p (struct frame *f,
                                 Lisp_Object attrs[LFACE_VECTOR_SIZE],
                                 struct face *def_face)
 {
+  Lisp_Object val;
   Lisp_Object *def_attrs = def_face->lface;
   Lisp_Object lattrs[LFACE_VECTOR_SIZE];
 
@@ -5360,6 +5365,20 @@ gui_supports_face_attributes_p (struct frame *f,
       return false;
     }
 
+  /* 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 */
+		  }
+	  }
+	}
+
   /* Everything checks out, this face is supported.  */
   return true;
 }
@@ -5452,15 +5471,26 @@ tty_supports_face_attributes_p (struct frame *f,
   val = attrs[LFACE_UNDERLINE_INDEX];
   if (!UNSPECIFIEDP (val))
     {
-      if (STRINGP (val))
-	return false;		/* ttys can't use colored underlines */
-      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
-	return false;		/* ttys can't use wave underlines */
-      else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
-	return false;		/* same as default */
-      else
-	test_caps |= TTY_CAP_UNDERLINE;
-    }
+	if (STRINGP (val))
+		test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	else if (EQ (CAR_SAFE (val), QCstyle))
+	  {
+	if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
+		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
+	  {
+		return false; /* Face uses an unsupported underline style.	*/
+	  }
+
+	test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	  }
+	else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
+		return false;  /* same as default */
+	else
+		test_caps |= TTY_CAP_UNDERLINE;
+  }
 
   /* inverse video */
   val = attrs[LFACE_INVERSE_INDEX];
@@ -6381,6 +6411,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
                 face->underline = FACE_UNDER_LINE;
               else if (EQ (value, Qwave))
                 face->underline = FACE_UNDER_WAVE;
+	      else
+		face->underline = FACE_UNDER_LINE;
             }
 	  else if (EQ (keyword, QCposition))
 	    {
@@ -6431,17 +6463,18 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
 }
 
 
-/* Map a specified color of face FACE on frame F to a tty color index.
-   IDX is either LFACE_FOREGROUND_INDEX or LFACE_BACKGROUND_INDEX, and
-   specifies which color to map.  Set *DEFAULTED to true if mapping to the
-   default foreground/background colors.  */
+/* Map the specified color COLOR of face FACE on frame F to a tty
+   color index.  IDX is one of LFACE_FOREGROUND_INDEX,
+   LFACE_BACKGROUND_INDEX or LFACE_UNDERLINE_INDEX, and specifies
+   which color to map.  Set *DEFAULTED to true if mapping to the
+   default foreground/background colors. */
 
 static void
-map_tty_color (struct frame *f, struct face *face,
+map_tty_color (struct frame *f, struct face *face, Lisp_Object color,
 	       enum lface_attribute_index idx, bool *defaulted)
 {
-  Lisp_Object frame, color, def;
-  bool foreground_p = idx == LFACE_FOREGROUND_INDEX;
+  Lisp_Object frame, def;
+  bool foreground_p = idx != LFACE_BACKGROUND_INDEX;
   unsigned long default_pixel =
     foreground_p ? FACE_TTY_DEFAULT_FG_COLOR : FACE_TTY_DEFAULT_BG_COLOR;
   unsigned long pixel = default_pixel;
@@ -6450,10 +6483,11 @@ map_tty_color (struct frame *f, struct face *face,
     foreground_p ? FACE_TTY_DEFAULT_BG_COLOR : FACE_TTY_DEFAULT_FG_COLOR;
 #endif
 
-  eassert (idx == LFACE_FOREGROUND_INDEX || idx == LFACE_BACKGROUND_INDEX);
+  eassert (idx == LFACE_FOREGROUND_INDEX
+	   || idx == LFACE_BACKGROUND_INDEX
+	   || idx == LFACE_UNDERLINE_INDEX);
 
   XSETFRAME (frame, f);
-  color = face->lface[idx];
 
   if (STRINGP (color)
       && SCHARS (color)
@@ -6498,10 +6532,28 @@ map_tty_color (struct frame *f, struct face *face,
 #endif /* MSDOS */
     }
 
-  if (foreground_p)
-    face->foreground = pixel;
-  else
-    face->background = pixel;
+  switch (idx)
+	{
+	case LFACE_FOREGROUND_INDEX:
+	  face->foreground = pixel;
+	  break;
+	case LFACE_BACKGROUND_INDEX:
+	  face->background = pixel;
+	  break;
+	case LFACE_UNDERLINE_INDEX:
+	  face->underline_color = pixel;
+	  break;
+	default:
+	  emacs_abort ();
+	}
+}
+
+static void
+map_tty_color2 (struct frame *f, struct face *face, Lisp_Object color,
+		enum lface_attribute_index idx)
+{
+	bool face_colors_defaulted = false;
+	map_tty_color (f, face, color, idx, &face_colors_defaulted);
 }
 
 
@@ -6515,6 +6567,7 @@ realize_tty_face (struct face_cache *cache,
 {
   struct face *face;
   int weight, slant;
+  Lisp_Object underline;
   bool face_colors_defaulted = false;
   struct frame *f = cache->f;
 
@@ -6534,16 +6587,77 @@ realize_tty_face (struct face_cache *cache,
     face->tty_bold_p = true;
   if (slant != 100)
     face->tty_italic_p = true;
-  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
-    face->tty_underline_p = true;
   if (!NILP (attrs[LFACE_INVERSE_INDEX]))
     face->tty_reverse_p = true;
   if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
     face->tty_strike_through_p = true;
 
+  /* Text underline.  */
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (NILP (underline))
+    {
+	face->underline = FACE_NO_UNDERLINE;
+	face->underline_color = 0;
+    }
+  else if (EQ (underline, Qt))
+    {
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+    }
+  else if (STRINGP (underline))
+    {
+	face->underline = FACE_UNDER_LINE;
+	map_tty_color2 (f, face, underline, LFACE_UNDERLINE_INDEX);
+    }
+  else if (CONSP (underline))
+    {
+	/* `(:color COLOR :style STYLE)'.
+	   STYLE being one of `line', `double', `wave', `dotted' or `dashed'.  */
+	face->underline = FACE_UNDER_LINE;
+	face->underline_color = 0;
+
+	while (CONSP (underline))
+	  {
+		Lisp_Object keyword, value;
+
+		keyword = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (!CONSP (underline))
+			break;
+		value = XCAR (underline);
+		underline = XCDR (underline);
+
+		if (EQ (keyword, QCcolor))
+		  {
+			if (EQ (value, Qforeground_color))
+				face->underline_color = 0;
+			else if (STRINGP (value))
+				map_tty_color2 (f, face, value, LFACE_UNDERLINE_INDEX);
+		  }
+		else if (EQ (keyword, QCstyle))
+		  {
+			if (EQ (value, Qline))
+				face->underline = FACE_UNDER_LINE;
+			else if (EQ (value, Qdouble))
+				face->underline = FACE_DOUBLE_UNDER_LINE;
+			else if (EQ (value, Qwave))
+				face->underline = FACE_UNDER_WAVE;
+			else if (EQ (value, Qdotted))
+				face->underline = FACE_DOTTED_UNDER_LINE;
+			else if (EQ (value, Qdashed))
+				face->underline = FACE_DASHED_UNDER_LINE;
+			else
+				face->underline = FACE_UNDER_LINE;
+		  }
+	  }
+    }
+
   /* Map color names to color indices.  */
-  map_tty_color (f, face, LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
-  map_tty_color (f, face, LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
+  map_tty_color (f, face, face->lface[LFACE_FOREGROUND_INDEX],
+		 LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
+  map_tty_color (f, face, face->lface[LFACE_BACKGROUND_INDEX],
+		 LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
 
   /* Swap colors if face is inverse-video.  If the colors are taken
      from the frame colors, they are already inverted, since the
@@ -7229,6 +7343,9 @@ syms_of_xfaces (void)
   DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
+  DEFSYM (Qdouble, "double");
+  DEFSYM (Qdotted, "dotted");
+  DEFSYM (Qdashed, "dashed");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qflat_button, "flat-button");
-- 
2.43.0

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: 62994 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: [PATCH v5] Add support for colored and styled underlines on tty
 frames
Date: Sun, 11 Feb 2024 18:07:29 +0000

mohkale <at> kisara.moe writes:

Finally got there XD. This is the correct commit. Please disregard V4.

-- 
Mohsin Kaleem

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

From: Po Lu <luangruo <at> yahoo.com>
To: mohkale <at> kisara.moe
Cc: Eli Zaretskii <eliz <at> gnu.org>, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v3 1/1] Add support for colored and styled
 underlines on tty frames
Date: Mon, 12 Feb 2024 09:28:03 +0800

mohkale <at> kisara.moe writes:

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
>
> * src/dispextern.h (face, face_underline_type, syms_of_xfacse,
> internal-set-lisp-face-attribute, gui_supports_face_attributes_p):

Type `C-c C-w' within a *vc-log* buffer to generate commit messages, and
fill them with `M-q'.  Otherwise, you will wind up with typos and errors
such as this entry, where a newline is used to separate entries within a
single pair of parentheses.

Thanks.

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

From: Po Lu <luangruo <at> yahoo.com>
To: mohkale <at> kisara.moe
Cc: Eli Zaretskii <eliz <at> gnu.org>, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Mon, 12 Feb 2024 09:43:11 +0800

mohkale <at> kisara.moe writes:

> ++++

Where's the documentation for this change?

> +*** Support for 'styled' and 'colored' underline face attributes on TTY frames

We prefer to punctuate sentences within NEWS headings.

> +If your terminals termcap or terminfo database entry has the 'Su' or
> +'Smulx' capability defined, Emacs will now emit the prescribed escape
> +sequence necessary to render faces with styled underlines on TTY
> +frames.
> +
> +Styled underlines are any underlines containing a non-default
> +underline style or a color other than the foreground-color.
> +The available underline styles for TTY frames are 'double', 'wave',
> +'dotted', and 'dashed'.  These are currently supported by Kitty,
> +libvte, and st (through the undercurl patch) among other terminals.

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

>  
>  * Editing Changes in Emacs 30.1
>  
> diff --git a/lisp/cus-face.el b/lisp/cus-face.el
> index 47afa841f5e..12551e37785 100644
> --- a/lisp/cus-face.el
> +++ b/lisp/cus-face.el
> @@ -141,7 +141,10 @@ custom-face-attributes
>  		   (const :format "" :value :style)
>  		   (choice :tag "Style"
>  			   (const :tag "Line" line)
> -			   (const :tag "Wave" wave))
> +			   (const :tag "Double" double)
> +			   (const :tag "Wave" wave)
> +			   (const :tag "Dotted" dotted)
> +			   (const :tag "Dashed" dashed))
>                     (const :format "" :value :position)
>                     (choice :tag "Position"
>                             (const :tag "At Default Position" nil)
> diff --git a/src/dispextern.h b/src/dispextern.h
> index 5387cb45603..574798fc547 100644
> --- a/src/dispextern.h
> +++ b/src/dispextern.h
> @@ -1690,9 +1690,13 @@ #define FONT_TOO_HIGH(ft)						\
>  
>  enum face_underline_type
>  {
> +	/* Note: Order matches the order of the Smulx terminfo extension. */
>    FACE_NO_UNDERLINE = 0,
>    FACE_UNDER_LINE,
> -  FACE_UNDER_WAVE
> +  FACE_DOUBLE_UNDER_LINE,
> +  FACE_UNDER_WAVE,
> +  FACE_DOTTED_UNDER_LINE,
> +  FACE_DASHED_UNDER_LINE,
>  };
>  
>  /* Structure describing a realized face.
> @@ -1776,7 +1780,7 @@ #define FONT_TOO_HIGH(ft)						\
>    ENUM_BF (face_box_type) box : 2;
>  
>    /* Style of underlining. */
> -  ENUM_BF (face_underline_type) underline : 2;
> +  ENUM_BF (face_underline_type) underline : 3;
>  
>    /* If `box' above specifies a 3D type, true means use box_color for
>       drawing shadows.  */
> @@ -1808,7 +1812,6 @@ #define FONT_TOO_HIGH(ft)						\
>       string meaning the default color of the TTY.  */
>    bool_bf tty_bold_p : 1;
>    bool_bf tty_italic_p : 1;
> -  bool_bf tty_underline_p : 1;
>    bool_bf tty_reverse_p : 1;
>    bool_bf tty_strike_through_p : 1;
>  
> @@ -3421,6 +3424,7 @@ #define TTY_CAP_BOLD		0x04
>  #define TTY_CAP_DIM		0x08
>  #define TTY_CAP_ITALIC  	0x10
>  #define TTY_CAP_STRIKE_THROUGH	0x20
> +#define TTY_CAP_UNDERLINE_STYLED	0x32 & TTY_CAP_UNDERLINE

#define TTY_CAP_UNDERLINE_STYLED (0x32 & TTY_CAP_UNDERLINE)

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

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.

>    if (face->tty_strike_through_p
>        && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
> @@ -2041,6 +2052,14 @@ turn_on_face (struct frame *f, int face_id)
>  	  OUTPUT (tty, p);
>  	  xfree (p);
>  	}
> +
> +	  ts = tty->TF_set_underline_color;
> +	  if (ts && face->underline_color)
> +	{
> +	  p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
> +	  OUTPUT (tty, p);
> +	  xfree (p);
> +	}
>      }
>  }

Likewise.

> @@ -2061,7 +2080,7 @@ turn_off_face (struct frame *f, int face_id)
>        if (face->tty_bold_p
>  	  || face->tty_italic_p
>  	  || face->tty_reverse_p
> -	  || face->tty_underline_p
> +	  || face->underline
>  	  || face->tty_strike_through_p)
>  	{
>  	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
> @@ -2073,7 +2092,7 @@ turn_off_face (struct frame *f, int face_id)
>      {
>        /* If we don't have "me" we can only have those appearances
>  	 that have exit sequences defined.  */
> -      if (face->tty_underline_p)
> +      if (face->underline)
>  	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
>      }
>  
> @@ -2104,6 +2123,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
>    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,
> +			 NC_UNDERLINE);
>    TTY_CAPABLE_P_TRY (tty,
>  		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
>    TTY_CAPABLE_P_TRY (tty,
> @@ -4360,6 +4382,30 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
>    tty->TF_underscore = tgetflag ("ul");
>    tty->TF_teleray = tgetflag ("xt");
>  
> +  /* Styled underlines.	 Support for this is provided either by the
> +	 escape sequence in Smulx or the Su flag.  The latter results in a
> +	 common default escape sequence and is not recommended.	 */

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.

> +#ifdef TERMINFO
> +	tty->TF_set_underline_style = tigetstr("Smulx");
> +	if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
> +		tty->TF_set_underline_style = NULL;
> +#else
> +	tty->TF_set_underline_style = tgetstr("Smulx", address);
> +#endif
> +	if (!tty->TF_set_underline_style && tgetflag("Su"))
> +		/* Default to the kitty escape sequence.  See
> +		   https://sw.kovidgoyal.net/kitty/underlines/ */
> +		tty->TF_set_underline_style = "\x1b[4:%p1%dm";
> +
> +	if (tty->TF_set_underline_style)
> +		/* This escape sequence for setting the underline color is
> +		   consistent with the one described in kitty (see above) and
> +		   adapted from the one used by neovim.	 This sequence has
> +		   been altered from the neovim sequence at
> +		   https://github.com/neovim/neovim/blob/42f492ac99058bd1cd56c3c7871e7e464b2a5e24/src/nvim/tui/tui.c#L1932
> +		   to require only a single parameter, the color index.	 */
> +		tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";

More indentation problems.  Please don't link to Github, and explain
instead the reasoning behind the sequence itself.

>  #else /* DOS_NT */
>  #ifdef WINDOWSNT
>    {
> diff --git a/src/termchar.h b/src/termchar.h
> index 2d845107e11..de9009d32f1 100644
> --- a/src/termchar.h
> +++ b/src/termchar.h
> @@ -171,6 +171,13 @@ #define EMACS_TERMCHAR_H
>                                     non-blank position.  Must clear before writing _.  */
>    int TF_teleray;               /* termcap xt flag: many weird consequences.
>                                     For t1061. */
> +  const char *TF_set_underline_style; /* termcap Smulx entry: Switches the underline
> +										 style based on the parameter.	Param should
> +										 be one of: 0 (none), 1 (straight), 2 (double),
> +										 3 (wave), 4 (dotted), or 5 (dashed).  */
> +  const char *TF_set_underline_color; /* Enabled when TF_set_underline_style is set:
> +										 Sets the color of the underline.  Accepts a
> +										 single parameter, the color index.	 */

Egregious indentation error.

>    int RPov;                     /* # chars to start a TS_repeat */
>  
> diff --git a/src/xfaces.c b/src/xfaces.c
> index a558e7328c0..a39e2bb6781 100644
> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -3311,7 +3311,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
>                  }
>  
>                else if (EQ (key, QCstyle)
> -                       && !(EQ (val, Qline) || EQ (val, Qwave)))
> +					   && !(EQ (val, Qline)
> +							|| EQ (val, Qdouble)
> +							|| EQ (val, Qwave)
> +							|| EQ (val, Qdotted)
> +							|| EQ (val, Qdashed)))
>                  {
>                    valid_p = false;
>                    break;
> @@ -5266,6 +5270,7 @@ gui_supports_face_attributes_p (struct frame *f,
>                                  Lisp_Object attrs[LFACE_VECTOR_SIZE],
>                                  struct face *def_face)
>  {
> +  Lisp_Object val;
>    Lisp_Object *def_attrs = def_face->lface;
>    Lisp_Object lattrs[LFACE_VECTOR_SIZE];
>  
> @@ -5360,6 +5365,20 @@ gui_supports_face_attributes_p (struct frame *f,
>        return false;
>      }
>  
> +  /* 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 */
> +		  }
> +	  }
> +	}
> +
>    /* Everything checks out, this face is supported.  */
>    return true;
>  }
> @@ -5452,15 +5471,26 @@ tty_supports_face_attributes_p (struct frame *f,
>    val = attrs[LFACE_UNDERLINE_INDEX];
>    if (!UNSPECIFIEDP (val))
>      {
> -      if (STRINGP (val))
> -	return false;		/* ttys can't use colored underlines */
> -      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
> -	return false;		/* ttys can't use wave underlines */
> -      else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
> -	return false;		/* same as default */
> -      else
> -	test_caps |= TTY_CAP_UNDERLINE;
> -    }
> +	if (STRINGP (val))
> +		test_caps |= TTY_CAP_UNDERLINE_STYLED;
> +	else if (EQ (CAR_SAFE (val), QCstyle))
> +	  {
> +	if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
> +	  {
> +		return false; /* Face uses an unsupported underline style.	*/
> +	  }
> +
> +	test_caps |= TTY_CAP_UNDERLINE_STYLED;
> +	  }
> +	else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
> +		return false;  /* same as default */
> +	else
> +		test_caps |= TTY_CAP_UNDERLINE;
> +  }

Ditto.  Please also avoid inserting redundant braces in if statements.

>  static void
> -map_tty_color (struct frame *f, struct face *face,
> +map_tty_color (struct frame *f, struct face *face, Lisp_Object color,
>  	       enum lface_attribute_index idx, bool *defaulted)
>  {
> -  Lisp_Object frame, color, def;
> -  bool foreground_p = idx == LFACE_FOREGROUND_INDEX;
> +  Lisp_Object frame, def;
> +  bool foreground_p = idx != LFACE_BACKGROUND_INDEX;
>    unsigned long default_pixel =
>      foreground_p ? FACE_TTY_DEFAULT_FG_COLOR : FACE_TTY_DEFAULT_BG_COLOR;
>    unsigned long pixel = default_pixel;
> @@ -6450,10 +6483,11 @@ map_tty_color (struct frame *f, struct face *face,
>      foreground_p ? FACE_TTY_DEFAULT_BG_COLOR : FACE_TTY_DEFAULT_FG_COLOR;
>  #endif
>  
> -  eassert (idx == LFACE_FOREGROUND_INDEX || idx == LFACE_BACKGROUND_INDEX);
> +  eassert (idx == LFACE_FOREGROUND_INDEX
> +	   || idx == LFACE_BACKGROUND_INDEX
> +	   || idx == LFACE_UNDERLINE_INDEX);
>  
>    XSETFRAME (frame, f);
> -  color = face->lface[idx];
>  
>    if (STRINGP (color)
>        && SCHARS (color)
> @@ -6498,10 +6532,28 @@ map_tty_color (struct frame *f, struct face *face,
>  #endif /* MSDOS */
>      }
>  
> -  if (foreground_p)
> -    face->foreground = pixel;
> -  else
> -    face->background = pixel;
> +  switch (idx)
> +	{
> +	case LFACE_FOREGROUND_INDEX:
> +	  face->foreground = pixel;
> +	  break;
> +	case LFACE_BACKGROUND_INDEX:
> +	  face->background = pixel;
> +	  break;
> +	case LFACE_UNDERLINE_INDEX:
> +	  face->underline_color = pixel;
> +	  break;
> +	default:
> +	  emacs_abort ();
> +	}
> +}
> +
> +static void
> +map_tty_color2 (struct frame *f, struct face *face, Lisp_Object color,
> +		enum lface_attribute_index idx)
> +{
> +	bool face_colors_defaulted = false;
> +	map_tty_color (f, face, color, idx, &face_colors_defaulted);
>  }

Ditto.

> @@ -6515,6 +6567,7 @@ realize_tty_face (struct face_cache *cache,
>  {
>    struct face *face;
>    int weight, slant;
> +  Lisp_Object underline;
>    bool face_colors_defaulted = false;
>    struct frame *f = cache->f;
>  
> @@ -6534,16 +6587,77 @@ realize_tty_face (struct face_cache *cache,
>      face->tty_bold_p = true;
>    if (slant != 100)
>      face->tty_italic_p = true;
> -  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
> -    face->tty_underline_p = true;
>    if (!NILP (attrs[LFACE_INVERSE_INDEX]))
>      face->tty_reverse_p = true;
>    if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
>      face->tty_strike_through_p = true;
>  
> +  /* Text underline.  */
> +  underline = attrs[LFACE_UNDERLINE_INDEX];
> +  if (NILP (underline))
> +    {
> +	face->underline = FACE_NO_UNDERLINE;
> +	face->underline_color = 0;
> +    }
> +  else if (EQ (underline, Qt))
> +    {
> +	face->underline = FACE_UNDER_LINE;
> +	face->underline_color = 0;
> +    }
> +  else if (STRINGP (underline))
> +    {
> +	face->underline = FACE_UNDER_LINE;
> +	map_tty_color2 (f, face, underline, LFACE_UNDERLINE_INDEX);
> +    }
> +  else if (CONSP (underline))
> +    {
> +	/* `(:color COLOR :style STYLE)'.
> +	   STYLE being one of `line', `double', `wave', `dotted' or `dashed'.  */
> +	face->underline = FACE_UNDER_LINE;
> +	face->underline_color = 0;
> +
> +	while (CONSP (underline))
> +	  {
> +		Lisp_Object keyword, value;
> +
> +		keyword = XCAR (underline);
> +		underline = XCDR (underline);
> +
> +		if (!CONSP (underline))
> +			break;
> +		value = XCAR (underline);
> +		underline = XCDR (underline);
> +
> +		if (EQ (keyword, QCcolor))
> +		  {
> +			if (EQ (value, Qforeground_color))
> +				face->underline_color = 0;
> +			else if (STRINGP (value))
> +				map_tty_color2 (f, face, value, LFACE_UNDERLINE_INDEX);
> +		  }
> +		else if (EQ (keyword, QCstyle))
> +		  {
> +			if (EQ (value, Qline))
> +				face->underline = FACE_UNDER_LINE;
> +			else if (EQ (value, Qdouble))
> +				face->underline = FACE_DOUBLE_UNDER_LINE;
> +			else if (EQ (value, Qwave))
> +				face->underline = FACE_UNDER_WAVE;
> +			else if (EQ (value, Qdotted))
> +				face->underline = FACE_DOTTED_UNDER_LINE;
> +			else if (EQ (value, Qdashed))
> +				face->underline = FACE_DASHED_UNDER_LINE;
> +			else
> +				face->underline = FACE_UNDER_LINE;
> +		  }
> +	  }
> +    }

And ditto.  Thanks.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mohkale <at> kisara.moe
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v5] Add support for colored and styled underlines on tty
 frames
Date: Mon, 12 Feb 2024 14:48:49 +0200

> From: mohkale <at> kisara.moe
> Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
> Date: Sun, 11 Feb 2024 18:05:01 +0000
> 
> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> 
> * src/dispextern.h (face, face_underline_type, syms_of_xfacse,
> internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
> Add definitions for new underline styles of Double, Dotted and Dashed.
> Delete tty_underline_p from the face struct and use just underline going
> forward.  Add a flag to check whether styled underlines are available.
> * lisp/cus-face.el (custom-face-attributes): Add entries for Double,
> Dotted and Dashed so they can be set through `customize'.
> * src/termchar.c (tty_display_info): Add an entry for the escape
> sequence to set the underline style and color on terminal frames.
> * src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
> underline style escape sequence from the Smulx termcap (alternatively if
> the Su flag is set use a default sequence).  Allow checking for support
> of styled underlines in the current terminal frame.  Output the necessary
> escape sequences to activate a styled underline on turn_on_face; this is
> currently only used for the new special underline styles, a default
> straight underline will still use the "us" termcap.  Output escape
> sequence to set underline color when set in the face and supported by
> the tty.  Save a default value for this sequence on init_tty when styled
> underlines are supported.
> * src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face,
> map_tty_color, map_tty_color2): Assert whether styled underlines are
> supported by the current terminal on
> display-supports-face-attributes-p checks.  Populate the correct
> underline style and color in the face spec when realizing a face.
> Allow map_tty_color to map underline colors alongside foreground and
> background.  The interface of map_tty_color was amended to allow
> the caller to supply the underline color instead of accessing it
> through the face attributes.  A new variant map_tty_color2 was added
> for contexts where caller doesn't care about foreground/background
> face defaulting.
> ---
>  etc/NEWS         |  15 +++++
>  lisp/cus-face.el |   5 +-
>  src/dispextern.h |  10 ++-
>  src/term.c       |  54 +++++++++++++--
>  src/termchar.h   |   7 ++
>  src/xfaces.c     | 171 +++++++++++++++++++++++++++++++++++++++--------
>  6 files changed, 227 insertions(+), 35 deletions(-)

Thanks.  I think in addition to NEWS, we'd need to update the ELisp
Reference manual, because the new underline styles are not currently
mentioned there.

> --- a/src/term.c
> +++ b/src/term.c
> @@ -2014,8 +2014,19 @@ turn_on_face (struct frame *f, int face_id)
>  	OUTPUT1 (tty, tty->TS_enter_dim_mode);
>      }
>  
> -  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
> -    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
> +  if (face->underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
> +	{
> +	if (face->underline == FACE_UNDER_LINE
> +		|| !tty->TF_set_underline_style)
> +		OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
> +	else if (tty->TF_set_underline_style)
> +	  {
> +		char *p;

Here and elsewhere in the patch, you use indentation style slightly
different from ours, so please reindent to follow our style (which
uses TABs and SPACEs, not just TABs).

> +		p = tparam(tty->TF_set_underline_style, NULL, 0, face->underline, 0, 0, 0);
                         ^^
Our style is to leave a single SPACE between the name of a function
and the opening parenthesis.  Several places in the patch don't leave
that SPACE.

> +  /* Styled underlines.	 Support for this is provided either by the
                          ^^^^^^^
Please don't use TABs inside comments, except as indentation.

> +	if (!tty->TF_set_underline_style && tgetflag("Su"))
> +		/* Default to the kitty escape sequence.  See
> +		   https://sw.kovidgoyal.net/kitty/underlines/ */
                                                             ^^
This should be a period.  Also, our style is to leave two SPACEs after
the final sentence of a comment, before the "*/" comment delimiter.

> +			return false; /* Unsupported underline style */
  			                                            ^
Period and one more SPACE are missing there.

> +	if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
> +	  {
> +		return false; /* Face uses an unsupported underline style.	*/
> +	  }

Our style is not to use braces for single-statement blocks.

> +/* Map the specified color COLOR of face FACE on frame F to a tty
> +   color index.  IDX is one of LFACE_FOREGROUND_INDEX,
> +   LFACE_BACKGROUND_INDEX or LFACE_UNDERLINE_INDEX, and specifies
> +   which color to map.  Set *DEFAULTED to true if mapping to the
> +   default foreground/background colors. */
                                          ^^
One more SPACE there.

> -  if (foreground_p)
> -    face->foreground = pixel;
> -  else
> -    face->background = pixel;
> +  switch (idx)
> +	{
> +	case LFACE_FOREGROUND_INDEX:
> +	  face->foreground = pixel;
> +	  break;
> +	case LFACE_BACKGROUND_INDEX:
> +	  face->background = pixel;
> +	  break;
> +	case LFACE_UNDERLINE_INDEX:
> +	  face->underline_color = pixel;
> +	  break;
> +	default:
> +	  emacs_abort ();

The original code didn't call emacs_abort, but instead simply used
PIXEL as the background color.  Why would we do something different
now?

> +static void
> +map_tty_color2 (struct frame *f, struct face *face, Lisp_Object color,
> +		enum lface_attribute_index idx)
> +{
> +	bool face_colors_defaulted = false;
> +	map_tty_color (f, face, color, idx, &face_colors_defaulted);
>  }

Is this function really justified? why not call map_tty_color?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Mon, 12 Feb 2024 15:50:40 +0200

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: 62994 <at> debbugs.gnu.org,  Eli Zaretskii <eliz <at> gnu.org>
> Date: Mon, 12 Feb 2024 09:43:11 +0800
> 
> > +If your terminals termcap or terminfo database entry has the 'Su' or
> > +'Smulx' capability defined, Emacs will now emit the prescribed escape
> > +sequence necessary to render faces with styled underlines on TTY
> > +frames.
> > +
> > +Styled underlines are any underlines containing a non-default
> > +underline style or a color other than the foreground-color.
> > +The available underline styles for TTY frames are 'double', 'wave',
> > +'dotted', and 'dashed'.  These are currently supported by Kitty,
> > +libvte, and st (through the undercurl patch) among other terminals.
> 
> What about GUI frames?  I don't want to see a display feature installed
> before it is also implemented for the likes of X.

Unless you are saying that implementing this on X would be impossible
or very hard, I don't think we need to delay installing this feature
until it is also available on X.  Of course, if Mohsin can also
provide an X-based implementation, that would be super.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Mon, 12 Feb 2024 22:49:28 +0800

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

> Of course, if Mohsin can also provide an X-based implementation, that
> would be super.

This was my question, yes.  Thanks.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Wed, 14 Feb 2024 11:40:34 -0800

On 2/11/2024 10:05 AM, mohkale <at> kisara.moe wrote:
> @@ -7229,6 +7343,9 @@ syms_of_xfaces (void)
>     DEFSYM (QCposition, ":position");
>     DEFSYM (Qline, "line");
>     DEFSYM (Qwave, "wave");
> +  DEFSYM (Qdouble, "double");
> +  DEFSYM (Qdotted, "dotted");
> +  DEFSYM (Qdashed, "dashed");
>     DEFSYM (Qreleased_button, "released-button");
>     DEFSYM (Qpressed_button, "pressed-button");
>     DEFSYM (Qflat_button, "flat-button");

This is very minor, but for consistency with the existing styles ("line" 
and "wave"), maybe we should use a noun form for the new styles. For 
example "dots", "dashes", and maybe "double-line"? That would hopefully 
be easier for programmers to remember, and leaves the door open for some 
other kind of doubled underline in the future too (e.g. a double wave).

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 17:15:25 +0000

Po Lu <luangruo <at> yahoo.com> writes:

> mohkale <at> kisara.moe 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

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Po Lu <luangruo <at> yahoo.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 17:20:34 +0000

Po Lu <luangruo <at> yahoo.com> writes:

>> Of course, if Mohsin can also provide an X-based implementation, that
>> would be super.
>
> This was my question, yes.  Thanks.

Ah, my bad, I responded to this in the root email. I don't really have
much interest in adding this to GUI frames. If preferred I can remove
the extra underline support from this patch set (my primary goal was
really just colored underlines).

-- 
Mohsin Kaleem

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 19:45:01 +0200

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: 62994 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
> Date: Sun, 10 Mar 2024 17:15:25 +0000
> 
> Po Lu <luangruo <at> yahoo.com> writes:
> 
> > mohkale <at> kisara.moe 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.

The "+++" thingy means the manuals were updated for this change, which
is why Po Lu asked where are those updates.

> > 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?

Not absolutely necessary, but very much preferred.  It is unheard of
in Emacs to have some face attribute that is supported on TTY frames
but not on GUI frames.

> > 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.

No, you should mark the region and then press C-M-\.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v5] Add support for colored and styled underlines on tty
 frames
Date: Sun, 10 Mar 2024 18:08:19 +0000

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

> Thanks.  I think in addition to NEWS, we'd need to update the ELisp
> Reference manual, because the new underline styles are not currently
> mentioned there.

Added. Should I mention display support between GUI and TTY frames or is
it sufficient just to mention these are the options?

> Here and elsewhere in the patch, you use indentation style slightly
> different from ours, so please reindent to follow our style (which
> uses TABs and SPACEs, not just TABs).

I did just run M-x tabify over all the lines I changed but looks like
that was a mistake. I've tried just manually re-adjusting all the
changesets in the next patchset but still not fully clear if it adheres
to the conventions wanted here 😅.

>> +		p = tparam(tty->TF_set_underline_style, NULL, 0, face->underline, 0, 0, 0);
>                          ^^
> Our style is to leave a single SPACE between the name of a function
> and the opening parenthesis.  Several places in the patch don't leave
> that SPACE.

Did a quick grep for `[^ ](` and fixed all offending lines.

>
>> +  /* Styled underlines.	 Support for this is provided either by the
>                           ^^^^^^^
> Please don't use TABs inside comments, except as indentation.

Seems to have been a byproduct of `M-x tabify`. Apologies.

>> +			return false; /* Unsupported underline style */
>   			                                            ^
> Period and one more SPACE are missing there.

Added.

>
>> +	if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
>> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble)
>> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
>> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdotted)
>> +		  || EQ (CAR_SAFE (CDR_SAFE (val)), Qdashed)))
>> +	  {
>> +		return false; /* Face uses an unsupported underline style.	*/
>> +	  }
>
> Our style is not to use braces for single-statement blocks.

Removed.

>
>> +/* Map the specified color COLOR of face FACE on frame F to a tty
>> +   color index.  IDX is one of LFACE_FOREGROUND_INDEX,
>> +   LFACE_BACKGROUND_INDEX or LFACE_UNDERLINE_INDEX, and specifies
>> +   which color to map.  Set *DEFAULTED to true if mapping to the
>> +   default foreground/background colors. */
>                                           ^^
> One more SPACE there.

Added.

>
>> -  if (foreground_p)
>> -    face->foreground = pixel;
>> -  else
>> -    face->background = pixel;
>> +  switch (idx)
>> +	{
>> +	case LFACE_FOREGROUND_INDEX:
>> +	  face->foreground = pixel;
>> +	  break;
>> +	case LFACE_BACKGROUND_INDEX:
>> +	  face->background = pixel;
>> +	  break;
>> +	case LFACE_UNDERLINE_INDEX:
>> +	  face->underline_color = pixel;
>> +	  break;
>> +	default:
>> +	  emacs_abort ();
>
> The original code didn't call emacs_abort, but instead simply used
> PIXEL as the background color.  Why would we do something different
> now?

Earlier in the function we call eassert on the index parameter. I
might've misunderstood but I thought that check would terminate the
function at that point so this line where we handle a index value
outside the supported range should never be called. If preferred I can
fallthrough into the background statement to stay consistent?

>
>> +static void
>> +map_tty_color2 (struct frame *f, struct face *face, Lisp_Object color,
>> +		enum lface_attribute_index idx)
>> +{
>> +	bool face_colors_defaulted = false;
>> +	map_tty_color (f, face, color, idx, &face_colors_defaulted);
>>  }
>
> Is this function really justified? why not call map_tty_color?

The extra parameter face_colors_defaulted doesn't really make sense for
anything but foreground/background color calls. Using the existing one
would make the callsite for the underline color set more noisy with an
extra output param that we then just ignore. I thought this was a
slightly nicer solution from the caller side. If preferred I can remove
and go the alternate route?

-- 
Mohsin Kaleem

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Jim Porter <jporterbugs <at> gmail.com>, 62994 <at> debbugs.gnu.org, Eli Zaretskii
 <eliz <at> gnu.org>
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 18:10:13 +0000

Jim Porter <jporterbugs <at> gmail.com> writes:

> On 2/11/2024 10:05 AM, mohkale <at> kisara.moe wrote:
>> @@ -7229,6 +7343,9 @@ syms_of_xfaces (void)
>>     DEFSYM (QCposition, ":position");
>>     DEFSYM (Qline, "line");
>>     DEFSYM (Qwave, "wave");
>> +  DEFSYM (Qdouble, "double");
>> +  DEFSYM (Qdotted, "dotted");
>> +  DEFSYM (Qdashed, "dashed");
>>     DEFSYM (Qreleased_button, "released-button");
>>     DEFSYM (Qpressed_button, "pressed-button");
>>     DEFSYM (Qflat_button, "flat-button");
>
> This is very minor, but for consistency with the existing styles ("line" 
> and "wave"), maybe we should use a noun form for the new styles. For 
> example "dots", "dashes", and maybe "double-line"? That would hopefully 
> be easier for programmers to remember, and leaves the door open for some 
> other kind of doubled underline in the future too (e.g. a double wave).

The current names were just what kitty decided to call the related
underline styles [1] but no reason to stick to that. I'll update as
suggested. Just to clarify wave will stay wave (instead of wavy) but the
remaining will be converted to nouns.

[1]: https://sw.kovidgoyal.net/kitty/underlines/

-- 
Mohsin Kaleem

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 18:22:39 +0000

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

> The "+++" thingy means the manuals were updated for this change, which
> is why Po Lu asked where are those updates.

I see, does --- have a similar alternate meaning or can I just replace
the pluses with minuses?

>> Is this absolutely necessary?
>
> Not absolutely necessary, but very much preferred.  It is unheard of
> in Emacs to have some face attribute that is supported on TTY frames
> but not on GUI frames.

I see. If a blocker I'm happy to remove the extra underline styles? I'm
not sure how long it would take me to add GUI support and as I don't use
the GUI I'd be unlikely to prioritise it any time soon.

> No, you should mark the region and then press C-M-\.

I see. So I should run this over all the lines I've changed to normalize
them?

-- 
Mohsin Kaleem

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Mohsin Kaleem <mohkale <at> kisara.moe>, Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 11:51:51 -0700

On 3/10/2024 11:22 AM, Mohsin Kaleem wrote:
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>> The "+++" thingy means the manuals were updated for this change, which
>> is why Po Lu asked where are those updates.
> 
> I see, does --- have a similar alternate meaning or can I just replace
> the pluses with minuses?

"---" means there's no need to update the manuals for that change in NEWS.

>> Not absolutely necessary, but very much preferred.  It is unheard of
>> in Emacs to have some face attribute that is supported on TTY frames
>> but not on GUI frames.
> 
> I see. If a blocker I'm happy to remove the extra underline styles? I'm
> not sure how long it would take me to add GUI support and as I don't use
> the GUI I'd be unlikely to prioritise it any time soon.

If someone points me in the right direction, I could take a look (I'd 
particularly like to use the dotted underlines in GUI frames). It'll 
probably be a couple weeks at least until I could work on it though.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 21:27:10 +0200

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
> Date: Sun, 10 Mar 2024 18:22:39 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > The "+++" thingy means the manuals were updated for this change, which
> > is why Po Lu asked where are those updates.
> 
> I see, does --- have a similar alternate meaning or can I just replace
> the pluses with minuses?

"---" means the change is so minor it will not require any manual
changes.  When in doubt leave the entry with neither "+++" nor "---".

> >> Is this absolutely necessary?
> >
> > Not absolutely necessary, but very much preferred.  It is unheard of
> > in Emacs to have some face attribute that is supported on TTY frames
> > but not on GUI frames.
> 
> I see. If a blocker I'm happy to remove the extra underline styles? I'm
> not sure how long it would take me to add GUI support and as I don't use
> the GUI I'd be unlikely to prioritise it any time soon.

I'm not sure it should be a blocker.

> > No, you should mark the region and then press C-M-\.
> 
> I see. So I should run this over all the lines I've changed to normalize
> them?

Yes.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 21:28:56 +0200

> Date: Sun, 10 Mar 2024 11:51:51 -0700
> Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 3/10/2024 11:22 AM, Mohsin Kaleem wrote:
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> > 
> >> The "+++" thingy means the manuals were updated for this change, which
> >> is why Po Lu asked where are those updates.
> > 
> > I see, does --- have a similar alternate meaning or can I just replace
> > the pluses with minuses?
> 
> "---" means there's no need to update the manuals for that change in NEWS.
> 
> >> Not absolutely necessary, but very much preferred.  It is unheard of
> >> in Emacs to have some face attribute that is supported on TTY frames
> >> but not on GUI frames.
> > 
> > I see. If a blocker I'm happy to remove the extra underline styles? I'm
> > not sure how long it would take me to add GUI support and as I don't use
> > the GUI I'd be unlikely to prioritise it any time soon.
> 
> If someone points me in the right direction, I could take a look (I'd 
> particularly like to use the dotted underlines in GUI frames). It'll 
> probably be a couple weeks at least until I could work on it though.

Thanks.  I'm not sure I understand where you want to be directed.  Is
it to the code which implements underwave on GUI frames? something
else?

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Sun, 10 Mar 2024 12:47:44 -0700

On 3/10/2024 12:28 PM, Eli Zaretskii wrote:
>> Date: Sun, 10 Mar 2024 11:51:51 -0700
>> Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
>> From: Jim Porter <jporterbugs <at> gmail.com>
>>
>> If someone points me in the right direction, I could take a look (I'd
>> particularly like to use the dotted underlines in GUI frames). It'll
>> probably be a couple weeks at least until I could work on it though.
> 
> Thanks.  I'm not sure I understand where you want to be directed.  Is
> it to the code which implements underwave on GUI frames? something
> else?

Yeah, exactly. I could probably find it myself with some searching, but 
if someone already knows where to start, that would save me some time.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Jim Porter <jporterbugs <at> gmail.com>, 62994 <at> debbugs.gnu.org,
 mohkale <at> kisara.moe
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Mon, 11 Mar 2024 10:07:47 +0800

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

> Thanks.  I'm not sure I understand where you want to be directed.  Is
> it to the code which implements underwave on GUI frames? something
> else?

It's x_draw_underwave and its siblings in each of the *term.[cm] files,
though only X support is absolutely essential.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v5] Add support for colored and styled
 underlines on tty frames
Date: Thu, 14 Mar 2024 12:20:17 +0200

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: 62994 <at> debbugs.gnu.org
> Date: Sun, 10 Mar 2024 18:08:19 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Thanks.  I think in addition to NEWS, we'd need to update the ELisp
> > Reference manual, because the new underline styles are not currently
> > mentioned there.
> 
> Added. Should I mention display support between GUI and TTY frames or is
> it sufficient just to mention these are the options?

The documentation should say which displays support this.

> >> -  if (foreground_p)
> >> -    face->foreground = pixel;
> >> -  else
> >> -    face->background = pixel;
> >> +  switch (idx)
> >> +	{
> >> +	case LFACE_FOREGROUND_INDEX:
> >> +	  face->foreground = pixel;
> >> +	  break;
> >> +	case LFACE_BACKGROUND_INDEX:
> >> +	  face->background = pixel;
> >> +	  break;
> >> +	case LFACE_UNDERLINE_INDEX:
> >> +	  face->underline_color = pixel;
> >> +	  break;
> >> +	default:
> >> +	  emacs_abort ();
> >
> > The original code didn't call emacs_abort, but instead simply used
> > PIXEL as the background color.  Why would we do something different
> > now?
> 
> Earlier in the function we call eassert on the index parameter. I
> might've misunderstood but I thought that check would terminate the
> function at that point so this line where we handle a index value
> outside the supported range should never be called.

eassert are only active in debug builds with --enable-checking; they
compile to nothing in production builds.

> If preferred I can
> fallthrough into the background statement to stay consistent?

Yes, please.

> >> +static void
> >> +map_tty_color2 (struct frame *f, struct face *face, Lisp_Object color,
> >> +		enum lface_attribute_index idx)
> >> +{
> >> +	bool face_colors_defaulted = false;
> >> +	map_tty_color (f, face, color, idx, &face_colors_defaulted);
> >>  }
> >
> > Is this function really justified? why not call map_tty_color?
> 
> The extra parameter face_colors_defaulted doesn't really make sense for
> anything but foreground/background color calls. Using the existing one
> would make the callsite for the underline color set more noisy with an
> extra output param that we then just ignore. I thought this was a
> slightly nicer solution from the caller side. If preferred I can remove
> and go the alternate route?

How many callers does this function have?  If just one or two, I don't
think a separate function is worth our while.

Thanks.

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

From: mohkale <at> kisara.moe
To: 62994 <at> debbugs.gnu.org
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
Subject: [PATCH v6] Add support for colored and styled underlines on tty frames
Date: Sun, 14 Apr 2024 14:56:32 +0100

From: Mohsin Kaleem <mohkale <at> kisara.moe>

* src/dispextern.h (face, face_underline_type, syms_of_xfacse,
internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
Add definitions for new underline styles of Double-line, Dots and
Dashes.  Rename FACE_UNDER_LINE and FACE_UNDER_WAVE to make definitions
consistent.  Delete tty_underline_p from the face struct and use just
underline going forward.  Add a flag to check whether styled underlines
are available.
* lisp/cus-face.el (custom-face-attributes): Add entries for Double-line,
Dots and Dashes so they can be set through `customize'.
* src/termchar.c (tty_display_info): Add an entry for the escape
sequence to set the underline style and color on terminal frames.
* src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
underline style escape sequence from the Smulx termcap (alternatively if
the Su flag is set use a default sequence).  Allow checking for support
of styled underlines in the current terminal frame.  Output the necessary
escape sequences to activate a styled underline on turn_on_face; this is
currently only used for the new special underline styles, a default
straight underline will still use the "us" termcap.  Output escape
sequence to set underline color when set in the face and supported by
the tty.  Save a default value for this sequence on init_tty when styled
underlines are supported.
* src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face,
map_tty_color): Assert whether styled underlines are supported by the
current terminal on display-supports-face-attributes-p checks.  Populate
the correct underline style and color in the face spec when realizing a
face.  Allow map_tty_color to map underline colors alongside foreground
and background.  The interface of map_tty_color was amended to allow the
caller to supply the underline color instead of accessing it through the
face attributes.
* src/xterm.c (x_draw_glyph_string): Updated to use renamed
FACE_UNDERLINE_SINGLE and FACE_UNDERLINE_WAVE face_underline_type
enumerations.
---
 doc/lispref/display.texi |  15 ++--
 etc/NEWS                 |  16 ++++
 lisp/cus-face.el         |   5 +-
 src/dispextern.h         |  12 ++-
 src/term.c               |  50 +++++++++++-
 src/termchar.h           |   7 ++
 src/xfaces.c             | 161 ++++++++++++++++++++++++++++++++-------
 src/xterm.c              |   6 +-
 8 files changed, 226 insertions(+), 46 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index fba15578f4f..8425aa23422 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2685,12 +2685,15 @@ Face Attributes
 @var{color} is either a string, or the symbol @code{foreground-color},
 meaning the foreground color of the face.  Omitting the attribute
 @code{:color} means to use the foreground color of the face.
-@var{style} should be a symbol @code{line} or @code{wave}, meaning to
-use a straight or wavy line.  Omitting the attribute @code{:style}
-means to use a straight line.  @var{position}, if non-@code{nil}, means to
-display the underline at the descent of the text, instead of at the
-baseline level.  If it is a number, then it specifies the amount of
-pixels above the descent to display the underline.
+@var{style} is a symbol which sets the line-style to of the underline.
+It should be one of @code{line}, @code{double-line}, @code{wave},
+@code{dots}, or @code{dashes}.  GUI frames only support @code{line} and
+@code{wave}.  Terminal frames can support all aforementioned underline
+styles.  Omitting the attribute @code{:style} means to use a straight
+line.  @var{position}, if non-@code{nil}, means to display the underline
+at the descent of the text, instead of at the baseline level.  If it is
+a number, then it specifies the amount of pixels above the descent to
+display the underline.
 @end table
 
 @cindex overlined text
diff --git a/etc/NEWS b/etc/NEWS
index bc8be557711..7926f34f85a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -424,6 +424,22 @@ Use 'TAB' in the minibuffer to show or hide the password.  Likewise,
 there is an icon on the mode-line, which toggles the visibility of the
 password when clicking with 'mouse-1'.
 
+** Terminal Emacs
+
+---
+*** Support for 'styled' and 'colored' underline face attributes on TTY frames.
+If your terminals termcap or terminfo database entry has the 'Su' or
+'Smulx' capability defined, Emacs will now emit the prescribed escape
+sequence necessary to render faces with styled underlines on TTY
+frames.
+
+Styled underlines are any underlines containing a non-default
+underline style or a color other than the foreground-color.
+The available underline styles for TTY frames are 'single',
+'double-line', 'wave', 'dots, and 'dashes'.  These are currently
+supported by Kitty, libvte, and st (through the undercurl patch) among
+other terminals.
+
 
 * Editing Changes in Emacs 30.1
 
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index 47afa841f5e..d0a1a66e29f 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -141,7 +141,10 @@ custom-face-attributes
 		   (const :format "" :value :style)
 		   (choice :tag "Style"
 			   (const :tag "Line" line)
-			   (const :tag "Wave" wave))
+			   (const :tag "Double line" double-line)
+			   (const :tag "Wave" wave)
+			   (const :tag "Dots" dots)
+			   (const :tag "Dashes" dashes))
                    (const :format "" :value :position)
                    (choice :tag "Position"
                            (const :tag "At Default Position" nil)
diff --git a/src/dispextern.h b/src/dispextern.h
index f29377f3596..fc5e53bb055 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1690,9 +1690,13 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+	/* Note: Order matches the order of the Smulx terminfo extension. */
   FACE_NO_UNDERLINE = 0,
-  FACE_UNDER_LINE,
-  FACE_UNDER_WAVE
+  FACE_UNDERLINE_SINGLE,
+  FACE_UNDERLINE_DOUBLE_LINE,
+  FACE_UNDERLINE_WAVE,
+  FACE_UNDERLINE_DOTS,
+  FACE_UNDERLINE_DASHES,
 };
 
 /* Structure describing a realized face.
@@ -1776,7 +1780,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline : 2;
+  ENUM_BF (face_underline_type) underline : 3;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
@@ -1808,7 +1812,6 @@ #define FONT_TOO_HIGH(ft)						\
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
   bool_bf tty_italic_p : 1;
-  bool_bf tty_underline_p : 1;
   bool_bf tty_reverse_p : 1;
   bool_bf tty_strike_through_p : 1;
 
@@ -3426,6 +3429,7 @@ #define TTY_CAP_BOLD		0x04
 #define TTY_CAP_DIM		0x08
 #define TTY_CAP_ITALIC  	0x10
 #define TTY_CAP_STRIKE_THROUGH	0x20
+#define TTY_CAP_UNDERLINE_STYLED	(0x32 & TTY_CAP_UNDERLINE)
 
 
 /***********************************************************************
diff --git a/src/term.c b/src/term.c
index 3fa244be824..37174027e04 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2014,8 +2014,19 @@ turn_on_face (struct frame *f, int face_id)
 	OUTPUT1 (tty, tty->TS_enter_dim_mode);
     }
 
-  if (face->tty_underline_p && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
-    OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+  if (face->underline && MAY_USE_WITH_COLORS_P (tty, NC_UNDERLINE))
+    {
+      if (face->underline == FACE_UNDERLINE_SINGLE
+	  || !tty->TF_set_underline_style)
+	OUTPUT1_IF (tty, tty->TS_enter_underline_mode);
+      else if (tty->TF_set_underline_style)
+	{
+	  char *p;
+	  p = tparam (tty->TF_set_underline_style, NULL, 0, face->underline, 0, 0, 0);
+	  OUTPUT (tty, p);
+	  xfree (p);
+	}
+    }
 
   if (face->tty_strike_through_p
       && MAY_USE_WITH_COLORS_P (tty, NC_STRIKE_THROUGH))
@@ -2041,6 +2052,14 @@ turn_on_face (struct frame *f, int face_id)
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
+
+	ts = tty->TF_set_underline_color;
+	if (ts && face->underline_color)
+	  {
+		p = tparam (ts, NULL, 0, face->underline_color, 0, 0, 0);
+		OUTPUT (tty, p);
+		xfree (p);
+	  }
     }
 }
 
@@ -2061,7 +2080,7 @@ turn_off_face (struct frame *f, int face_id)
       if (face->tty_bold_p
 	  || face->tty_italic_p
 	  || face->tty_reverse_p
-	  || face->tty_underline_p
+	  || face->underline
 	  || face->tty_strike_through_p)
 	{
 	  OUTPUT1_IF (tty, tty->TS_exit_attribute_mode);
@@ -2073,7 +2092,7 @@ turn_off_face (struct frame *f, int face_id)
     {
       /* If we don't have "me" we can only have those appearances
 	 that have exit sequences defined.  */
-      if (face->tty_underline_p)
+      if (face->underline)
 	OUTPUT_IF (tty, tty->TS_exit_underline_mode);
     }
 
@@ -2104,6 +2123,9 @@ #define TTY_CAPABLE_P_TRY(tty, cap, TS, NC_bit)				\
   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,
+		     NC_UNDERLINE);
   TTY_CAPABLE_P_TRY (tty,
 		     TTY_CAP_BOLD,	  tty->TS_enter_bold_mode, NC_BOLD);
   TTY_CAPABLE_P_TRY (tty,
@@ -4360,6 +4382,26 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
   tty->TF_underscore = tgetflag ("ul");
   tty->TF_teleray = tgetflag ("xt");
 
+  /* Styled underlines.  Support for this is provided either by the
+     escape sequence in Smulx or the Su flag.  The latter results in a
+     common default escape sequence and is not recommended.  */
+#ifdef TERMINFO
+  tty->TF_set_underline_style = tigetstr ("Smulx");
+  if (tty->TF_set_underline_style == (char *) (intptr_t) -1)
+    tty->TF_set_underline_style = NULL;
+#else
+  tty->TF_set_underline_style = tgetstr ("Smulx", address);
+#endif
+  if (!tty->TF_set_underline_style && tgetflag ("Su"))
+    /* Default to the kitty escape sequence.  See
+       https://sw.kovidgoyal.net/kitty/underlines/ */
+    tty->TF_set_underline_style = "\x1b[4:%p1%dm";
+
+  if (tty->TF_set_underline_style)
+    /* Standard escape sequence to set the underline color.
+       Requires a single parameter, the color index.  */
+    tty->TF_set_underline_color = "\x1b[58:2::%p1%{65536}%/%d:%p1%{256}%/%{255}%&%d:%p1%{255}%&%dm";
+
 #else /* DOS_NT */
 #ifdef WINDOWSNT
   {
diff --git a/src/termchar.h b/src/termchar.h
index 2d845107e11..a1df5a19518 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -171,6 +171,13 @@ #define EMACS_TERMCHAR_H
                                    non-blank position.  Must clear before writing _.  */
   int TF_teleray;               /* termcap xt flag: many weird consequences.
                                    For t1061. */
+  const char *TF_set_underline_style; /* termcap Smulx entry: Switches the underline
+					 style based on the parameter.  Param should
+					 be one of: 0 (none), 1 (straight), 2 (double-line),
+					 3 (wave), 4 (dots), or 5 (dashes).  */
+  const char *TF_set_underline_color; /* Enabled when TF_set_underline_style is set:
+                                         Sets the color of the underline.  Accepts a
+                                         single parameter, the color index.  */
 
   int RPov;                     /* # chars to start a TS_repeat */
 
diff --git a/src/xfaces.c b/src/xfaces.c
index d4583e1a78f..6480bfe1fff 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3311,7 +3311,11 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
                 }
 
               else if (EQ (key, QCstyle)
-                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+                       && !(EQ (val, Qline)
+                            || EQ (val, Qdouble_line)
+                            || EQ (val, Qwave)
+                            || EQ (val, Qdots)
+                            || EQ (val, Qdashes)))
                 {
                   valid_p = false;
                   break;
@@ -5275,6 +5279,7 @@ gui_supports_face_attributes_p (struct frame *f,
                                 Lisp_Object attrs[LFACE_VECTOR_SIZE],
                                 struct face *def_face)
 {
+  Lisp_Object val;
   Lisp_Object *def_attrs = def_face->lface;
   Lisp_Object lattrs[LFACE_VECTOR_SIZE];
 
@@ -5369,6 +5374,14 @@ gui_supports_face_attributes_p (struct frame *f,
       return false;
     }
 
+  /* 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.  */
+
   /* Everything checks out, this face is supported.  */
   return true;
 }
@@ -5462,11 +5475,20 @@ tty_supports_face_attributes_p (struct frame *f,
   if (!UNSPECIFIEDP (val))
     {
       if (STRINGP (val))
-	return false;		/* ttys can't use colored underlines */
-      else if (EQ (CAR_SAFE (val), QCstyle) && EQ (CAR_SAFE (CDR_SAFE (val)), Qwave))
-	return false;		/* ttys can't use wave underlines */
+	test_caps |= TTY_CAP_UNDERLINE_STYLED;
+      else if (EQ (CAR_SAFE (val), QCstyle))
+	{
+	  if (!(EQ (CAR_SAFE (CDR_SAFE (val)), Qline)
+		|| EQ (CAR_SAFE (CDR_SAFE (val)), Qdouble_line)
+		|| EQ (CAR_SAFE (CDR_SAFE (val)), Qwave)
+		|| EQ (CAR_SAFE (CDR_SAFE (val)), Qdots)
+		|| EQ (CAR_SAFE (CDR_SAFE (val)), Qdashes)))
+	    return false; /* Face uses an unsupported underline style.  */
+
+	  test_caps |= TTY_CAP_UNDERLINE_STYLED;
+	}
       else if (face_attr_equal_p (val, def_attrs[LFACE_UNDERLINE_INDEX]))
-	return false;		/* same as default */
+	return false;  /* same as default */
       else
 	test_caps |= TTY_CAP_UNDERLINE;
     }
@@ -6321,7 +6343,7 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
   if (EQ (underline, Qt))
     {
       /* Use default color (same as foreground color).  */
-      face->underline = FACE_UNDER_LINE;
+      face->underline = FACE_UNDERLINE_SINGLE;
       face->underline_defaulted_p = true;
       face->underline_color = 0;
       face->underline_at_descent_line_p = false;
@@ -6330,7 +6352,7 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
   else if (STRINGP (underline))
     {
       /* Use specified color.  */
-      face->underline = FACE_UNDER_LINE;
+      face->underline = FACE_UNDERLINE_SINGLE;
       face->underline_defaulted_p = false;
       face->underline_color
 	= load_color (f, face, underline,
@@ -6350,7 +6372,7 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
     {
       /* `(:color COLOR :style STYLE)'.
          STYLE being one of `line' or `wave'. */
-      face->underline = FACE_UNDER_LINE;
+      face->underline = FACE_UNDERLINE_SINGLE;
       face->underline_color = 0;
       face->underline_defaulted_p = true;
       face->underline_at_descent_line_p = false;
@@ -6387,9 +6409,11 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
           else if (EQ (keyword, QCstyle))
             {
               if (EQ (value, Qline))
-                face->underline = FACE_UNDER_LINE;
+                face->underline = FACE_UNDERLINE_SINGLE;
               else if (EQ (value, Qwave))
-                face->underline = FACE_UNDER_WAVE;
+                face->underline = FACE_UNDERLINE_WAVE;
+              else
+                face->underline = FACE_UNDERLINE_SINGLE;
             }
 	  else if (EQ (keyword, QCposition))
 	    {
@@ -6440,17 +6464,18 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
 }
 
 
-/* Map a specified color of face FACE on frame F to a tty color index.
-   IDX is either LFACE_FOREGROUND_INDEX or LFACE_BACKGROUND_INDEX, and
-   specifies which color to map.  Set *DEFAULTED to true if mapping to the
+/* Map the specified color COLOR of face FACE on frame F to a tty
+   color index.  IDX is one of LFACE_FOREGROUND_INDEX,
+   LFACE_BACKGROUND_INDEX or LFACE_UNDERLINE_INDEX, and specifies
+   which color to map.  Set *DEFAULTED to true if mapping to the
    default foreground/background colors.  */
 
 static void
-map_tty_color (struct frame *f, struct face *face,
-	       enum lface_attribute_index idx, bool *defaulted)
+map_tty_color (struct frame *f, struct face *face, Lisp_Object color,
+               enum lface_attribute_index idx, bool *defaulted)
 {
-  Lisp_Object frame, color, def;
-  bool foreground_p = idx == LFACE_FOREGROUND_INDEX;
+  Lisp_Object frame, def;
+  bool foreground_p = idx != LFACE_BACKGROUND_INDEX;
   unsigned long default_pixel =
     foreground_p ? FACE_TTY_DEFAULT_FG_COLOR : FACE_TTY_DEFAULT_BG_COLOR;
   unsigned long pixel = default_pixel;
@@ -6459,10 +6484,11 @@ map_tty_color (struct frame *f, struct face *face,
     foreground_p ? FACE_TTY_DEFAULT_BG_COLOR : FACE_TTY_DEFAULT_FG_COLOR;
 #endif
 
-  eassert (idx == LFACE_FOREGROUND_INDEX || idx == LFACE_BACKGROUND_INDEX);
+  eassert (idx == LFACE_FOREGROUND_INDEX
+           || idx == LFACE_BACKGROUND_INDEX
+           || idx == LFACE_UNDERLINE_INDEX);
 
   XSETFRAME (frame, f);
-  color = face->lface[idx];
 
   if (STRINGP (color)
       && SCHARS (color)
@@ -6507,13 +6533,21 @@ map_tty_color (struct frame *f, struct face *face,
 #endif /* MSDOS */
     }
 
-  if (foreground_p)
-    face->foreground = pixel;
-  else
-    face->background = pixel;
+  switch (idx)
+    {
+    case LFACE_FOREGROUND_INDEX:
+      face->foreground = pixel;
+      break;
+    case LFACE_UNDERLINE_INDEX:
+      face->underline_color = pixel;
+      break;
+    case LFACE_BACKGROUND_INDEX:
+    default:
+      face->background = pixel;
+      break;
+    }
 }
 
-
 /* Realize the fully-specified face with attributes ATTRS in face
    cache CACHE for ASCII characters.  Do it for TTY frame CACHE->f.
    Value is a pointer to the newly created realized face.  */
@@ -6524,6 +6558,7 @@ realize_tty_face (struct face_cache *cache,
 {
   struct face *face;
   int weight, slant;
+  Lisp_Object underline;
   bool face_colors_defaulted = false;
   struct frame *f = cache->f;
 
@@ -6543,16 +6578,83 @@ realize_tty_face (struct face_cache *cache,
     face->tty_bold_p = true;
   if (slant != 100)
     face->tty_italic_p = true;
-  if (!NILP (attrs[LFACE_UNDERLINE_INDEX]))
-    face->tty_underline_p = true;
   if (!NILP (attrs[LFACE_INVERSE_INDEX]))
     face->tty_reverse_p = true;
   if (!NILP (attrs[LFACE_STRIKE_THROUGH_INDEX]))
     face->tty_strike_through_p = true;
 
+  /* Text underline.  */
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (NILP (underline))
+    {
+      face->underline = FACE_NO_UNDERLINE;
+      face->underline_color = 0;
+    }
+  else if (EQ (underline, Qt))
+    {
+      face->underline = FACE_UNDERLINE_SINGLE;
+      face->underline_color = 0;
+    }
+  else if (STRINGP (underline))
+    {
+      face->underline = FACE_UNDERLINE_SINGLE;
+      bool underline_color_defaulted;
+      map_tty_color (f, face, underline, LFACE_UNDERLINE_INDEX,
+		     &underline_color_defaulted);
+    }
+  else if (CONSP (underline))
+    {
+      /* `(:color COLOR :style STYLE)'.
+	 STYLE being one of `line', `double-line', `wave', `dots' or `dashes'.  */
+      face->underline = FACE_UNDERLINE_SINGLE;
+      face->underline_color = 0;
+
+      while (CONSP (underline))
+	{
+	  Lisp_Object keyword, value;
+
+	  keyword = XCAR (underline);
+	  underline = XCDR (underline);
+
+	  if (!CONSP (underline))
+	    break;
+	  value = XCAR (underline);
+	  underline = XCDR (underline);
+
+	  if (EQ (keyword, QCcolor))
+	    {
+	      if (EQ (value, Qforeground_color))
+		face->underline_color = 0;
+	      else if (STRINGP (value))
+		{
+		  bool underline_color_defaulted;
+		  map_tty_color (f, face, value, LFACE_UNDERLINE_INDEX,
+				 &underline_color_defaulted);
+		}
+	    }
+	  else if (EQ (keyword, QCstyle))
+	    {
+	      if (EQ (value, Qline))
+		face->underline = FACE_UNDERLINE_SINGLE;
+	      else if (EQ (value, Qdouble_line))
+		face->underline = FACE_UNDERLINE_DOUBLE_LINE;
+	      else if (EQ (value, Qwave))
+		face->underline = FACE_UNDERLINE_WAVE;
+	      else if (EQ (value, Qdots))
+		face->underline = FACE_UNDERLINE_DOTS;
+	      else if (EQ (value, Qdashes))
+		face->underline = FACE_UNDERLINE_DASHES;
+	      else
+		face->underline = FACE_UNDERLINE_SINGLE;
+	    }
+	}
+    }
+
   /* Map color names to color indices.  */
-  map_tty_color (f, face, LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
-  map_tty_color (f, face, LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
+  map_tty_color (f, face, face->lface[LFACE_FOREGROUND_INDEX],
+                 LFACE_FOREGROUND_INDEX, &face_colors_defaulted);
+  map_tty_color (f, face, face->lface[LFACE_BACKGROUND_INDEX],
+                 LFACE_BACKGROUND_INDEX, &face_colors_defaulted);
 
   /* Swap colors if face is inverse-video.  If the colors are taken
      from the frame colors, they are already inverted, since the
@@ -7238,6 +7340,9 @@ syms_of_xfaces (void)
   DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
+  DEFSYM (Qdouble_line, "double-line");
+  DEFSYM (Qdots, "dots");
+  DEFSYM (Qdashes, "dashes");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qflat_button, "flat-button");
diff --git a/src/xterm.c b/src/xterm.c
index e08ffd15b18..360541ac0b9 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -10957,7 +10957,7 @@ x_draw_glyph_string (struct glyph_string *s)
       /* Draw underline.  */
       if (s->face->underline)
         {
-          if (s->face->underline == FACE_UNDER_WAVE)
+          if (s->face->underline == FACE_UNDERLINE_WAVE)
             {
               if (s->face->underline_defaulted_p)
                 x_draw_underwave (s, decoration_width);
@@ -10971,13 +10971,13 @@ x_draw_glyph_string (struct glyph_string *s)
                   XSetForeground (display, s->gc, xgcv.foreground);
                 }
             }
-          else if (s->face->underline == FACE_UNDER_LINE)
+          else if (s->face->underline == FACE_UNDERLINE_SINGLE)
             {
               unsigned long thickness, position;
               int y;
 
               if (s->prev
-		  && s->prev->face->underline == FACE_UNDER_LINE
+		  && s->prev->face->underline == FACE_UNDERLINE_SINGLE
 		  && (s->prev->face->underline_at_descent_line_p
 		      == s->face->underline_at_descent_line_p)
 		  && (s->prev->face->underline_pixels_above_descent_line
-- 
2.44.0

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v6] Add support for colored and styled underlines on tty
 frames
Date: Sun, 14 Apr 2024 15:13:07 +0100

mohkale <at> kisara.moe writes:

Next patch attempt. I think I've acted on all the comments from last
time. Also renamed some of the existing enum face_underline_type entries
to make them consistent with the names of the new underline styles.

-- 
Mohsin Kaleem

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mohkale <at> kisara.moe
Cc: mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v6] Add support for colored and styled
 underlines on tty frames
Date: Sat, 20 Apr 2024 11:16:44 +0300

> Cc: Mohsin Kaleem <mohkale <at> kisara.moe>
> From: mohkale <at> kisara.moe
> Date: Sun, 14 Apr 2024 14:56:32 +0100

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).

> * src/dispextern.h (face, face_underline_type, syms_of_xfacse,
> internal-set-lisp-face-attribute, gui_supports_face_attributes_p):
> Add definitions for new underline styles of Double-line, Dots and
> Dashes.  Rename FACE_UNDER_LINE and FACE_UNDER_WAVE to make definitions
> consistent.  Delete tty_underline_p from the face struct and use just
> underline going forward.  Add a flag to check whether styled underlines
> are available.
> * lisp/cus-face.el (custom-face-attributes): Add entries for Double-line,
> Dots and Dashes so they can be set through `customize'.
> * src/termchar.c (tty_display_info): Add an entry for the escape
> sequence to set the underline style and color on terminal frames.
> * src/term.c (init_tty, tty_capable_p, turn_on_face): Read and save the
> underline style escape sequence from the Smulx termcap (alternatively if
> the Su flag is set use a default sequence).  Allow checking for support
> of styled underlines in the current terminal frame.  Output the necessary
> escape sequences to activate a styled underline on turn_on_face; this is
> currently only used for the new special underline styles, a default
> straight underline will still use the "us" termcap.  Output escape
> sequence to set underline color when set in the face and supported by
> the tty.  Save a default value for this sequence on init_tty when styled
> underlines are supported.
> * src/xfaces.c (tty_supports_face_attributes_p, realize_tty_face,
> map_tty_color): Assert whether styled underlines are supported by the
> current terminal on display-supports-face-attributes-p checks.  Populate
> the correct underline style and color in the face spec when realizing a
> face.  Allow map_tty_color to map underline colors alongside foreground
> and background.  The interface of map_tty_color was amended to allow the
> caller to supply the underline color instead of accessing it through the
> face attributes.
> * 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.

> +  if (!tty->TF_set_underline_style && tgetflag ("Su"))
> +    /* Default to the kitty escape sequence.  See
> +       https://sw.kovidgoyal.net/kitty/underlines/ */

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.

> +  /* 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?

> -	return false;		/* same as default */
> +	return false;  /* same as default */

Why this whitespace change?

Thanks.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: 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

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

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

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: 62994 <at> debbugs.gnu.org
> 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.

I don't know what exactly does "git am" do with Subject, but the fact
is the pre-commit hook aborted the commit.

How about if you produce a patch with "git format-patch", and then
attach the result to the mail (as an attachment)?  It could be that
the problem here is with some problematic format of the Subject line,
but there's no reason to have the patch heading be the Subject of the
email.

> 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.

That's not what I meant.  In order to have the proper line length, you
need first to add a TAB before each line, then turn on
change-log-mode, then fill the lines with M-q, then remove the leading
TABs.

The reason for this is that the Git logs are used to produce ChangeLog
files when we prepare a release, and ChangeLog files have a TAB before
each line of the log entries.

> 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.

Did you forget to attach the updated patch?

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

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

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

> How about if you produce a patch with "git format-patch", and then
> attach the result to the mail (as an attachment)?  It could be that
> the problem here is with some problematic format of the Subject line,
> but there's no reason to have the patch heading be the Subject of the
> email.

I'll give that a go.

> That's not what I meant.  In order to have the proper line length, you
> need first to add a TAB before each line, then turn on
> change-log-mode, then fill the lines with M-q, then remove the leading
> TABs.
>
> The reason for this is that the Git logs are used to produce ChangeLog
> files when we prepare a release, and ChangeLog files have a TAB before
> each line of the log entries.

I see, gave that a go and it seems to have reorganised things somewhat
so hopefully should meet conventions now. Apologies for the back and
forth on this, I'm accustomed to lints and convention adherence like
this being automated.

> Did you forget to attach the updated patch?

Was waiting on a reply to this message just to make sure I acted on all
the comments from the last time round. Sending now...

-- 
Mohsin Kaleem

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: 62994 <at> debbugs.gnu.org
Subject: bug#62994: [PATCH v7] Add support for colored and styled underlines
 on tty frames
Date: Sun, 21 Apr 2024 17:11:56 +0100

[Message part 1 (text/plain, inline)]

Manually attaching git exported patch.

[v7-0001-Add-support-for-colored-and-styled-underlines-on-.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
Mohsin Kaleem

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

From: Po Lu <luangruo <at> yahoo.com>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v7] Add support for colored and styled
 underlines on tty frames
Date: Mon, 22 Apr 2024 08:44:15 +0800

Mohsin Kaleem <mohkale <at> kisara.moe> writes:

> CLOSES bug#62994

This is not the proper manner of specifying bug numbers relating to a
change.  Place it immediately after the most relevant (in your judgment)
item in the log entry, e.g.:

  * src/foo.c (pertinent_function): Adjust for something.
  (bug#62994)

> * src/xterm.c (x_draw_glyph_string): Updated to use renamed
> FACE_UNDERLINE_SINGLE and FACE_UNDERLINE_WAVE face_underline_type
> enumerations.

This change should be accompanied by like updates to *_draw_glyph_string
and related functions in the remaining terminal backends.

Last but not least, are there terminal emulators besides Kitty that
support these underline styles?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>, Jim Porter <jporterbugs <at> gmail.com>
Cc: mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v7] Add support for colored and styled
 underlines on tty frames
Date: Mon, 22 Apr 2024 09:01:28 +0300

> Cc: 62994 <at> debbugs.gnu.org
> Date: Mon, 22 Apr 2024 08:44:15 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> > * src/xterm.c (x_draw_glyph_string): Updated to use renamed
> > FACE_UNDERLINE_SINGLE and FACE_UNDERLINE_WAVE face_underline_type
> > enumerations.
> 
> This change should be accompanied by like updates to *_draw_glyph_string
> and related functions in the remaining terminal backends.

I believe Jim Porter wanted to work on that.  Jim, have you made any
progress in this matter?

In any case, given that implementing this for GUI displays should not
be a problem, I don't see a reason to block this changeset until the
GUI parts are done.  Hopefully, Jim or someone else will get it ready
soon.

Thanks.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Po Lu <luangruo <at> yahoo.com>
Cc: mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v7] Add support for colored and styled
 underlines on tty frames
Date: Mon, 22 Apr 2024 10:35:57 -0700

On 4/21/2024 11:01 PM, Eli Zaretskii wrote:
>> Cc: 62994 <at> debbugs.gnu.org
>> Date: Mon, 22 Apr 2024 08:44:15 +0800
>> From:  Po Lu via "Bug reports for GNU Emacs,
>>   the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> This change should be accompanied by like updates to *_draw_glyph_string
>> and related functions in the remaining terminal backends.
> 
> I believe Jim Porter wanted to work on that.  Jim, have you made any
> progress in this matter?

I spent some time looking at it, and I have what I think would be a 
reasonable path forward. Essentially, make a more-generalized form of 
'x_draw_underwave' that can draw a tiled 1-bit image for the underline, 
and then pass the image data in as an argument. That would keep the code 
duplication to a minimum.

I haven't gotten a working implementation of this idea yet though, since 
I've been very busy with non-Emacs things. If someone else is itching to 
work on this, go for it. Otherwise, I should have a lot more free time 
by May.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v7] Add support for colored and styled
 underlines on tty frames
Date: Mon, 22 Apr 2024 18:52:27 +0100

Po Lu <luangruo <at> yahoo.com> writes:

> This is not the proper manner of specifying bug numbers relating to a
> change.  Place it immediately after the most relevant (in your judgment)
> item in the log entry, e.g.:
>
>   * src/foo.c (pertinent_function): Adjust for something.
>   (bug#62994)

I've moved it to after another section. Will resubmit patch.

>> * src/xterm.c (x_draw_glyph_string): Updated to use renamed
>> FACE_UNDERLINE_SINGLE and FACE_UNDERLINE_WAVE face_underline_type
>> enumerations.
>
> This change should be accompanied by like updates to *_draw_glyph_string
> and related functions in the remaining terminal backends.

Based on Elis comments I don't think is a hard requirement. I'll defer
this to Jim or anyone else who considers it a priority if that's ok :-).

>
> Last but not least, are there terminal emulators besides Kitty that
> support these underline styles?

I haven't tested exhaustively, although I listed 2 others in the NEWS
file. That being st (through a custom patch) and libvte. Personally I
use st so that was my primary motivation in adopting this.

-- 
Mohsin Kaleem

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: 62994 <at> debbugs.gnu.org
Cc: Po Lu <luangruo <at> yahoo.com>, Jim Porter <jporterbugs <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: [PATCH v8] Add support for colored and styled underlines on tty frames
Date: Mon, 22 Apr 2024 18:53:52 +0100

[Message part 1 (text/plain, inline)]

help-debbugs <at> gnu.org (GNU bug Tracking System) writes:

[v8-0001-Add-support-for-colored-and-styled-underlines-on-.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
Mohsin Kaleem

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Jim Porter <jporterbugs <at> gmail.com>, 62994 <at> debbugs.gnu.org,
 mohkale <at> kisara.moe
Subject: Re: bug#62994: [PATCH v7] Add support for colored and styled
 underlines on tty frames
Date: Tue, 23 Apr 2024 08:53:20 +0800

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

>> Cc: 62994 <at> debbugs.gnu.org
>> Date: Mon, 22 Apr 2024 08:44:15 +0800
>> From:  Po Lu via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>> 
>> > * src/xterm.c (x_draw_glyph_string): Updated to use renamed
>> > FACE_UNDERLINE_SINGLE and FACE_UNDERLINE_WAVE face_underline_type
>> > enumerations.
>> 
>> This change should be accompanied by like updates to
>> *_draw_glyph_string
>> and related functions in the remaining terminal backends.
>
> I believe Jim Porter wanted to work on that.  Jim, have you made any
> progress in this matter?
>
> In any case, given that implementing this for GUI displays should not
> be a problem, I don't see a reason to block this changeset until the
> GUI parts are done.  Hopefully, Jim or someone else will get it ready
> soon.

I wasn't asking that the new underline styles be ported to other
backends, but that the change to account for renamed enumerator values
be ported to the remainder of the *term.[cm] files.  Without which they
will fail to compile, which goes without saying.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>, mohkale <at> kisara.moe
Cc: jporterbugs <at> gmail.com, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v7] Add support for colored and styled
 underlines on tty frames
Date: Tue, 23 Apr 2024 09:02:18 +0300

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: Jim Porter <jporterbugs <at> gmail.com>,  mohkale <at> kisara.moe,
>   62994 <at> debbugs.gnu.org
> Date: Tue, 23 Apr 2024 08:53:20 +0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> Cc: 62994 <at> debbugs.gnu.org
> >> Date: Mon, 22 Apr 2024 08:44:15 +0800
> >> From:  Po Lu via "Bug reports for GNU Emacs,
> >>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> >> 
> >> > * src/xterm.c (x_draw_glyph_string): Updated to use renamed
> >> > FACE_UNDERLINE_SINGLE and FACE_UNDERLINE_WAVE face_underline_type
> >> > enumerations.
> >> 
> >> This change should be accompanied by like updates to
> >> *_draw_glyph_string
> >> and related functions in the remaining terminal backends.
> >
> > I believe Jim Porter wanted to work on that.  Jim, have you made any
> > progress in this matter?
> >
> > In any case, given that implementing this for GUI displays should not
> > be a problem, I don't see a reason to block this changeset until the
> > GUI parts are done.  Hopefully, Jim or someone else will get it ready
> > soon.
> 
> I wasn't asking that the new underline styles be ported to other
> backends, but that the change to account for renamed enumerator values
> be ported to the remainder of the *term.[cm] files.  Without which they
> will fail to compile, which goes without saying.

I will not install a changeset that breaks the build, rest assured.  I
always build Emacs after applying patches from others, so I'd know if
the build is broken.  I presume Mohsin builds Emacs with his changes,
so unless he only builds Emacs --without-x, I presume the changes do
build?  Or what am I missing?

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org, jporterbugs <at> gmail.com
Subject: Re: bug#62994: [PATCH v7] Add support for colored and styled
 underlines on tty frames
Date: Tue, 23 Apr 2024 15:32:40 +0800

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

> I will not install a changeset that breaks the build, rest assured.  I
> always build Emacs after applying patches from others, so I'd know if
> the build is broken.  I presume Mohsin builds Emacs with his changes,
> so unless he only builds Emacs --without-x, I presume the changes do
> build?  Or what am I missing?

It will build on X, but on no other GUI configuration, since he did not
update corresponding references to the renamed enumerators in the
remaining *term.c files.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: luangruo <at> yahoo.com, jporterbugs <at> gmail.com, 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v8] Add support for colored and styled underlines on tty
 frames
Date: Sat, 27 Apr 2024 12:11:39 +0300

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: Po Lu <luangruo <at> yahoo.com>, Jim Porter <jporterbugs <at> gmail.com>, Eli
>  Zaretskii <eliz <at> gnu.org>
> Date: Mon, 22 Apr 2024 18:53:52 +0100
> 
> >From b0b6d024fb66dad9027129e4dabe70d765d36b59 Mon Sep 17 00:00:00 2001
> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Date: Thu, 20 Apr 2023 22:30:12 +0100
> Subject: Add support for colored and styled underlines on tty frames

Thanks, installed on the master branch.

Jim, are you working on the GUI support for this?

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v8] Add support for colored and styled
 underlines on tty frames
Date: Sun, 28 Apr 2024 18:04:11 -0700

On 4/27/2024 2:11 AM, Eli Zaretskii wrote:
> Thanks, installed on the master branch.
> 
> Jim, are you working on the GUI support for this?

I'm back and available to work on Emacs as of today, but in the 
meantime, Po Lu has implemented the GUI side of this (see 
e844b81c56d74aa2b2efa0ce98ed3de71647e656 among others). I could take a 
look at an MS-Windows port, but I don't have Emacs set up to build on 
MS-Windows at the moment.

Having support for this on X and PGTK (not to mention Android and Haiku) 
is probably enough though; MS-Windows and Nextstep are just "nice to 
have" in my opinion.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Mohsin Kaleem <mohkale <at> kisara.moe>, Eli Zaretskii <eliz <at> gnu.org>,
 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v8] Add support for colored and styled
 underlines on tty frames
Date: Mon, 29 Apr 2024 12:52:52 +0800

Jim Porter <jporterbugs <at> gmail.com> writes:

> Having support for this on X and PGTK (not to mention Android and
> Haiku) is probably enough though; MS-Windows and Nextstep are just
> "nice to have" in my opinion.

I get the impression that inconsistent feature support between the
various GUI backends is a significant contributor to the bad publicity
we receive in some quarters as to our attitude towards the, if you will,
neglected, operating system ports.  Strictly speaking, all systems
besides X are organized on a "nice to have" basis, but for publicity's
sake they ought to receive these basic display features as well.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, mohkale <at> kisara.moe, 62994 <at> debbugs.gnu.org
Subject: Re: bug#62994: [PATCH v8] Add support for colored and styled
 underlines on tty frames
Date: Mon, 29 Apr 2024 10:10:15 +0300

> Date: Sun, 28 Apr 2024 18:04:11 -0700
> Cc: luangruo <at> yahoo.com, 62994 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 4/27/2024 2:11 AM, Eli Zaretskii wrote:
> > Thanks, installed on the master branch.
> > 
> > Jim, are you working on the GUI support for this?
> 
> I'm back and available to work on Emacs as of today, but in the 
> meantime, Po Lu has implemented the GUI side of this (see 
> e844b81c56d74aa2b2efa0ce98ed3de71647e656 among others). I could take a 
> look at an MS-Windows port, but I don't have Emacs set up to build on 
> MS-Windows at the moment.
> 
> Having support for this on X and PGTK (not to mention Android and Haiku) 
> is probably enough though; MS-Windows and Nextstep are just "nice to 
> have" in my opinion.

I don't mind leaving this stuff as it is now, until someone volunteers
to make the changes for the w32 build.

Thanks.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: jporterbugs <at> gmail.com, 62994 <at> debbugs.gnu.org, mohkale <at> kisara.moe
Subject: Re: bug#62994: [PATCH v8] Add support for colored and styled
 underlines on tty frames
Date: Mon, 29 Apr 2024 10:25:41 +0300

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  Mohsin Kaleem <mohkale <at> kisara.moe>,
>   62994 <at> debbugs.gnu.org
> Date: Mon, 29 Apr 2024 12:52:52 +0800
> 
> Jim Porter <jporterbugs <at> gmail.com> writes:
> 
> > Having support for this on X and PGTK (not to mention Android and
> > Haiku) is probably enough though; MS-Windows and Nextstep are just
> > "nice to have" in my opinion.
> 
> I get the impression that inconsistent feature support between the
> various GUI backends is a significant contributor to the bad publicity
> we receive in some quarters as to our attitude towards the, if you will,
> neglected, operating system ports.  Strictly speaking, all systems
> besides X are organized on a "nice to have" basis, but for publicity's
> sake they ought to receive these basic display features as well.

One counter-example is the non-support for stipple in the w32 port.

Patches to add that are welcome, of course.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jporterbugs <at> gmail.com, 62994 <at> debbugs.gnu.org, mohkale <at> kisara.moe
Subject: Re: bug#62994: [PATCH v8] Add support for colored and styled
 underlines on tty frames
Date: Mon, 29 Apr 2024 19:36:57 +0800

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

> One counter-example is the non-support for stipple in the w32 port.

That's also a source of dissatisfaction with the said port.  There is
this package that decorates buffers with extravagant stipples to
indicate indentation levels, which has grown popular with users, and of
which no discussion is complete without a passing remark bemoaning this
deficiency.

> Patches to add that are welcome, of course.

Styled underlines don't take priority over this, naturally.  As always,
the prerogative belongs to whoever writes the first line of code.

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

From: Mohsin Kaleem <mohkale <at> kisara.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, jporterbugs <at> gmail.com, 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v8] Add support for colored and styled underlines on tty
 frames
Date: Tue, 30 Apr 2024 18:53:17 +0100

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

> Thanks, installed on the master branch.
>
> Jim, are you working on the GUI support for this?

Ah, thanks Eli. And also for correcting those references in the related
UI files. I was just about to update them and send a new patch before
noticing you'd already corrected it. Thanks :-).

-- 
Mohsin Kaleem

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mohsin Kaleem <mohkale <at> kisara.moe>
Cc: luangruo <at> yahoo.com, jporterbugs <at> gmail.com, 62994 <at> debbugs.gnu.org
Subject: Re: [PATCH v8] Add support for colored and styled underlines on tty
 frames
Date: Tue, 30 Apr 2024 21:20:37 +0300

> From: Mohsin Kaleem <mohkale <at> kisara.moe>
> Cc: 62994 <at> debbugs.gnu.org, luangruo <at> yahoo.com, jporterbugs <at> gmail.com
> Date: Tue, 30 Apr 2024 18:53:17 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Thanks, installed on the master branch.
> >
> > Jim, are you working on the GUI support for this?
> 
> Ah, thanks Eli. And also for correcting those references in the related
> UI files. I was just about to update them and send a new patch before
> noticing you'd already corrected it. Thanks :-).

I had to do it, because otherwise I couldn't build Emacs, and I always
build after applying a patch to make sure the build is not broken.

Thanks for coming up with this feature.  Emacs 30 will be better now
than it could have been otherwise.

Previous Next

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