[PATCH 0/2] Use check-mark for current row of CLI table

[PATCH 0/2] Use check-mark for current row of CLI table

[Tom Tromey]

This series changes CLI table output to use a check mark or other
string instead of the current "*" when emoji output is enabled.  The
default in this case is now a Unicode check mark symbol.

Regression tested on x86-64 Fedora 41.

Signed-off-by: Tom Tromey <tromey@adacore.com>
---
Tom Tromey (2):
      Introduce ui_out::field_check_mark
      Allow check-mark to be changed for CLI

 gdb/NEWS                         |  6 ++++++
 gdb/ada-tasks.c                  |  5 +----
 gdb/cli-out.c                    |  8 ++++++++
 gdb/cli-out.h                    |  2 ++
 gdb/cli/cli-style.c              | 32 ++++++++++++++++++++++++++++++++
 gdb/cli/cli-style.h              |  3 +++
 gdb/doc/gdb.texinfo              | 10 ++++++++++
 gdb/inferior.c                   |  6 +-----
 gdb/linux-fork.c                 |  5 +----
 gdb/progspace.c                  |  6 +-----
 gdb/target-connection.c          |  6 ++----
 gdb/testsuite/gdb.base/style.exp |  4 ++++
 gdb/thread.c                     |  6 +-----
 gdb/ui-out.c                     |  9 +++++++++
 gdb/ui-out.h                     | 13 +++++++++++++
 15 files changed, 94 insertions(+), 27 deletions(-)
---
base-commit: 3fabf85b5cb0271fcd70047489a6513362cb4ea9
change-id: 20250509-emoji-check-mark-36415e813a85

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/2] Introduce ui_out::field_check_mark

[Tom Tromey]

This adds a new ui_out method, field_check_mark.  This method is to be
used to indicate the "current" or "selected" row in a table.  This
patch updates all the relevant tables to call this method.  No change
should be expected.
---
 gdb/ada-tasks.c         |  5 +----
 gdb/inferior.c          |  6 +-----
 gdb/linux-fork.c        |  5 +----
 gdb/progspace.c         |  6 +-----
 gdb/target-connection.c |  6 ++----
 gdb/thread.c            |  6 +-----
 gdb/ui-out.c            |  9 +++++++++
 gdb/ui-out.h            | 13 +++++++++++++
 8 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 259512377f085423d4feda1a8a67e409109179a2..2f152ec3581f1076110df2ca149a2add5a825e9e 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -1163,10 +1163,7 @@ print_ada_task_info (struct ui_out *uiout,
 
       /* Print a star if this task is the current task (or the task
 	 currently selected).  */
-      if (task_info->ptid == inferior_ptid)
-	uiout->field_string ("current", "*");
-      else
-	uiout->field_skip ("current");
+      uiout->field_check_mark ("current", task_info->ptid == inferior_ptid);
 
       /* Print the task number.  */
       uiout->field_signed ("id", taskno);
diff --git a/gdb/inferior.c b/gdb/inferior.c
index e8a28cd2a0e1a4f76946d4fc9c1abdfab08b5d3e..221dbce73d6721f5ed883745c0474e78d94c405d 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -598,11 +598,7 @@ print_inferior (struct ui_out *uiout, const char *requested_inferiors)
 
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
-      if (inf == current_inf)
-	uiout->field_string ("current", "*");
-      else
-	uiout->field_skip ("current");
-
+      uiout->field_check_mark ("current", inf == current_inf);
       uiout->field_signed ("number", inf->num);
 
       /* Because target_pid_to_str uses the current inferior,
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 338ba032a0e75862d449b9c02c3c61cfc5492dbe..dc50391b22a0ba2d5728d7ae5a4505a73d864fd9 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -849,10 +849,7 @@ print_checkpoints (struct ui_out *uiout, inferior *req_inf, fork_info *req_fi)
 
 	  ui_out_emit_tuple tuple_emitter (uiout, nullptr);
 
-	  if (is_current && cur_inf == inf)
-	    uiout->field_string ("current", "*");
-	  else
-	    uiout->field_skip ("current");
+	  uiout->field_check_mark ("current", is_current && cur_inf == inf);
 
 	  if (print_inf)
 	    uiout->field_fmt ("id", "%d.%d", inf->num, fi.num);
diff --git a/gdb/progspace.c b/gdb/progspace.c
index eda63790aec9169724119c766b7e4c72da3cb550..7f1b0652516f53bd0a69e88fb409887d8639e4c5 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -305,11 +305,7 @@ print_program_space (struct ui_out *uiout, int requested)
 
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
-      if (pspace == current_program_space)
-	uiout->field_string ("current", "*");
-      else
-	uiout->field_skip ("current");
-
+      uiout->field_check_mark ("current", pspace == current_program_space);
       uiout->field_signed ("id", pspace->num);
 
       if (pspace->exec_filename () != nullptr)
diff --git a/gdb/target-connection.c b/gdb/target-connection.c
index 60d7732b56e465ac6fda1164846302e1b9167456..69e5765959ef6bf0bfbc2fb482f5a26fa1a03c79 100644
--- a/gdb/target-connection.c
+++ b/gdb/target-connection.c
@@ -126,10 +126,8 @@ print_connection (struct ui_out *uiout, const char *requested_connections)
 
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
-      if (current_inferior ()->process_target () == t)
-	uiout->field_string ("current", "*");
-      else
-	uiout->field_skip ("current");
+      uiout->field_check_mark ("current",
+			       current_inferior ()->process_target () == t);
 
       uiout->field_signed ("number", t->connection_number);
 
diff --git a/gdb/thread.c b/gdb/thread.c
index b659463ef0246992a5290c1eb3b1175094a2e826..2ce94a01efbdfdfb2587d90a160f716171fd63c6 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1122,11 +1122,7 @@ do_print_thread (ui_out *uiout, const char *requested_threads,
 
   if (!uiout->is_mi_like_p ())
     {
-      if (tp == current_thread)
-	uiout->field_string ("current", "*");
-      else
-	uiout->field_skip ("current");
-
+      uiout->field_check_mark ("current", tp == current_thread);
       uiout->field_string ("id-in-tg", print_thread_id (tp));
     }
 
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 106a896c07fdadeae6c902a5bd541f81900c6453..a45a7af20d69170fb3ecac62f3f0345395bd88c2 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -557,6 +557,15 @@ ui_out::field_fmt (const char *fldname, const ui_file_style &style,
   va_end (args);
 }
 
+void
+ui_out::field_check_mark (const char *fldname, bool value)
+{
+  if (value)
+    field_string (fldname, get_check_mark ());
+  else
+    field_skip (fldname);
+}
+
 void
 ui_out::call_do_message (const ui_file_style &style, const char *format,
 			 ...)
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 1796e9c9eb3439422a767603d2799f4a7c167cf2..11d12024c2c15dd203a1df9d62eec72039a94c5e 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -206,6 +206,11 @@ class ui_out
 		  const char *format, ...)
     ATTRIBUTE_PRINTF (4, 5);
 
+  /* This is used when a table has a "current" field or the like.  If
+     VALUE is true, a check mark of some kind is emitted for the
+     field.  If VALUE is false, the field is skipped instead.  */
+  void field_check_mark (const char *fldname, bool value);
+
   void spaces (int numspaces) { do_spaces (numspaces); }
   void text (const char *string) { do_text (string); }
   void text (const std::string &string) { text (string.c_str ()); }
@@ -374,6 +379,14 @@ class ui_out
 				   double, double) = 0;
   virtual void do_progress_end () = 0;
 
+  /* Return the appropriate check-mark string.  */
+  virtual const char *get_check_mark ()
+  {
+    /* The default is "*" because that's what was done
+       historically.  */
+    return "*";
+  }
+
   /* Set as not MI-like by default.  It is overridden in subclasses if
      necessary.  */
 

-- 
2.49.0

Re: [PATCH 1/2] Introduce ui_out::field_check_mark

[Kevin Buettner]

On Fri, 09 May 2025 14:18:50 -0600
Tom Tromey <tromey@adacore.com> wrote:

> This adds a new ui_out method, field_check_mark.  This method is to be
> used to indicate the "current" or "selected" row in a table.  This
> patch updates all the relevant tables to call this method.  No change
> should be expected.
> ---
>  gdb/ada-tasks.c         |  5 +----
>  gdb/inferior.c          |  6 +-----
>  gdb/linux-fork.c        |  5 +----
>  gdb/progspace.c         |  6 +-----
>  gdb/target-connection.c |  6 ++----
>  gdb/thread.c            |  6 +-----
>  gdb/ui-out.c            |  9 +++++++++
>  gdb/ui-out.h            | 13 +++++++++++++
>  8 files changed, 29 insertions(+), 27 deletions(-)

LGTM.

Approved-by: Kevin Buettner <kevinb@redhat.com>

[PATCH 2/2] Allow check-mark to be changed for CLI

[Tom Tromey]

In keeping with the emojification of gdb, this patch changes the
default "current" marker to be a check-mark.  It adds a knob to allow
the character to be changed, and, as always, reverts to the old output
when emojis are disabled.
---
 gdb/NEWS                         |  6 ++++++
 gdb/cli-out.c                    |  8 ++++++++
 gdb/cli-out.h                    |  2 ++
 gdb/cli/cli-style.c              | 32 ++++++++++++++++++++++++++++++++
 gdb/cli/cli-style.h              |  3 +++
 gdb/doc/gdb.texinfo              | 10 ++++++++++
 gdb/testsuite/gdb.base/style.exp |  4 ++++
 7 files changed, 65 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index a82b7e3342c57b9b066e9012c5a961ad6ba839f0..07e72833a269ffe0ab034d4c4ea8b185ad217983 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -78,6 +78,12 @@ set style error-prefix STRING
   with emoji display, and so the prefixes are only displayed if emoji
   styling is enabled.
 
+set style check-mark STRING
+  Set the string that is used to indicate the "current" row when a
+  table is printed by the CLI.  This functionality is intended for use
+  with emoji display, and so the prefixes are only displayed if emoji
+  styling is enabled.
+
 info linker-namespaces
 info linker-namespaces [[N]]
   Print information about the given linker namespace (identified as N),
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 27a82f607eb3d6abcd9867880e99f824a0392312..c619bb4243dbfe4bdc0fb442e7496aa066b52575 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -421,6 +421,14 @@ cli_ui_out::do_progress_end ()
   m_progress_info.pop_back ();
 }
 
+/* See ui-out.h.  */
+
+const char *
+cli_ui_out::get_check_mark ()
+{
+  return ::get_check_mark ();
+}
+
 /* local functions */
 
 void
diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index c761dacc9a83cf5ffadfca2ee4751a37c2159402..342f2d0a15fb1d3823214d2f946f4ac10ca9346d 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -81,6 +81,8 @@ class cli_ui_out : public ui_out
 				   double, double) override;
   virtual void do_progress_end () override;
 
+  const char *get_check_mark () override;
+
   bool suppress_output ()
   { return m_suppress_output; }
 
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index e6441277d05f5215ecf3f04a8c82be65d9fd031d..ea56bbccf255b1a11c1ef4b3dfb7e06c5dc9656e 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -494,6 +494,29 @@ print_error_prefix (ui_file *file)
     gdb_puts (error_prefix.c_str (), file);
 }
 
+/* Emoji check-mark.  */
+static std::string check_mark = "✓";
+
+/* Implement 'show style check-mark'.  */
+
+static void
+show_check_mark (struct ui_file *file, int from_tty,
+		 struct cmd_list_element *c, const char *value)
+{
+  gdb_printf (file, _("Check mark is \"%s\".\n"),
+	      check_mark.c_str ());
+}
+
+/* See cli-style.h.  */
+
+const char *
+get_check_mark ()
+{
+  if (emojis_ok ())
+    return check_mark.c_str ();
+  return "*";
+}
+
 void _initialize_cli_style ();
 void
 _initialize_cli_style ()
@@ -737,4 +760,13 @@ The error prefix text is displayed before any error, when\n\
 emoji output is enabled."),
 			  nullptr, show_error_prefix,
 			  &style_set_list, &style_show_list);
+  add_setshow_string_cmd ("check-mark", no_class,
+			  &check_mark,
+			  _("Set the check mark text."),
+			  _("Show the check mark text."),
+			  _("\
+The check mark text is displayed in a table to indicate which\n\
+row is currently selected."),
+			  nullptr, show_check_mark,
+			  &style_set_list, &style_show_list);
 }
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index b1a950a539cb8eaf5381f3ea546ee9fa3e87500b..7b88424d972303b592149499985e7d868494be03 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -204,4 +204,7 @@ extern void print_warning_prefix (ui_file *file);
 /* Print the error prefix, if desired.  */
 extern void print_error_prefix (ui_file *file);
 
+/* Return the current check mark string.  */
+extern const char *get_check_mark ();
+
 #endif /* GDB_CLI_CLI_STYLE_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5e5e888cf38cea33a9d7b4e77039207374ec2d5e..539b3cba92d6893dcd843f684b00c91037324c0b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -28014,6 +28014,16 @@ emoji display, and so the prefixes are only displayed if emoji styling
 is enabled.  The defaults are the warning sign emoji for warnings, and
 and the cross mark emoji for errors.
 
+@item set style check-mark @var{string}
+@itemx show style check-mark
+These commands control the string that the CLI uses to indicate which
+line in a table indicates the current state of @value{GDBN}.  For
+example, this would be used to indicate the current thread in the
+output of @samp{info threads}.  This functionality is intended for use
+with emoji display, and so the chosen string only displayed if emoji
+styling is enabled.  The default is a check box character; with the
+non-emoji fallback being an asterisk.
+
 @item set style tui-current-position @samp{on|off}
 Enable or disable styling of the source and assembly code highlighted
 by the TUI's current position indicator.  The default is @samp{off}.
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index c10be3bc12aa10f5a13856cb14f7fad86f32ff90..9f2a288e953f0711d698673e723d6192c3862d79 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -341,6 +341,10 @@ proc run_style_tests { } {
 	gdb_test "maint translate-address" \
 	    "abcd:requires argument.*" \
 	    "error prefix"
+
+	gdb_test_no_output "set style check-mark @"
+	gdb_test "info inferior" "\r\n@ 1 .*"
+	gdb_test "show style check-mark" "Check mark is \"@\"\\."
     }
 }
 

-- 
2.49.0

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Eli Zaretskii]

> From: Tom Tromey <tromey@adacore.com>
> Date: Fri, 09 May 2025 14:18:51 -0600
> Cc: Tom Tromey <tromey@adacore.com>
> 
> In keeping with the emojification of gdb, this patch changes the
> default "current" marker to be a check-mark.  It adds a knob to allow
> the character to be changed, and, as always, reverts to the old output
> when emojis are disabled.
> ---
>  gdb/NEWS                         |  6 ++++++
>  gdb/cli-out.c                    |  8 ++++++++
>  gdb/cli-out.h                    |  2 ++
>  gdb/cli/cli-style.c              | 32 ++++++++++++++++++++++++++++++++
>  gdb/cli/cli-style.h              |  3 +++
>  gdb/doc/gdb.texinfo              | 10 ++++++++++
>  gdb/testsuite/gdb.base/style.exp |  4 ++++
>  7 files changed, 65 insertions(+)

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index a82b7e3342c57b9b066e9012c5a961ad6ba839f0..07e72833a269ffe0ab034d4c4ea8b185ad217983 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -78,6 +78,12 @@ set style error-prefix STRING
>    with emoji display, and so the prefixes are only displayed if emoji
>    styling is enabled.
>  
> +set style check-mark STRING
> +  Set the string that is used to indicate the "current" row when a
> +  table is printed by the CLI.  This functionality is intended for use
> +  with emoji display, and so the prefixes are only displayed if emoji
> +  styling is enabled.

This part is okay.

> +  add_setshow_string_cmd ("check-mark", no_class,
> +			  &check_mark,
> +			  _("Set the check mark text."),
> +			  _("Show the check mark text."),

I suggest

  Set the check-mark string.\n\
  Check-mark is the string used to indicate the current line in a table.

> +@item set style check-mark @var{string}
> +@itemx show style check-mark
> +These commands control the string that the CLI uses to indicate which
> +line in a table indicates the current state of @value{GDBN}.  For
> +example, this would be used to indicate the current thread in the
> +output of @samp{info threads}.  This functionality is intended for use
> +with emoji display, and so the chosen string only displayed if emoji
                                              ^^^
"is" is missing there.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Eli Zaretskii]

> From: Tom Tromey <tromey@adacore.com>
> Date: Fri, 09 May 2025 14:18:51 -0600
> Cc: Tom Tromey <tromey@adacore.com>
> 
> +/* Emoji check-mark.  */
> +static std::string check_mark = "✓";

Btw, if this is related to emoji display, shouldn't the default be
U+2713 followed by U+FE0F VARIATION SELECTOR-16, to show the "emoji
presentation" of the CHECK MARK character, if the terminal supports
that?  Or do too few terminals have such support?

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tom Tromey <tromey@adacore.com>
>> Date: Fri, 09 May 2025 14:18:51 -0600
>> Cc: Tom Tromey <tromey@adacore.com>
>> 
>> +/* Emoji check-mark.  */
>> +static std::string check_mark = "✓";

Eli> Btw, if this is related to emoji display, shouldn't the default be
Eli> U+2713 followed by U+FE0F VARIATION SELECTOR-16, to show the "emoji
Eli> presentation" of the CHECK MARK character, if the terminal supports
Eli> that?  Or do too few terminals have such support?

I tried this but this combination doesn't seem to change the display.

There are some "true" emoji check marks (like a heavy one with a
surrounding box or something) but I thought they looked strange in this
context.

Tom

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Eli Zaretskii]

> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>,  gdb-patches@sourceware.org
> Date: Fri, 16 May 2025 08:18:54 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Tom Tromey <tromey@adacore.com>
> >> Date: Fri, 09 May 2025 14:18:51 -0600
> >> Cc: Tom Tromey <tromey@adacore.com>
> >> 
> >> +/* Emoji check-mark.  */
> >> +static std::string check_mark = "✓";
> 
> Eli> Btw, if this is related to emoji display, shouldn't the default be
> Eli> U+2713 followed by U+FE0F VARIATION SELECTOR-16, to show the "emoji
> Eli> presentation" of the CHECK MARK character, if the terminal supports
> Eli> that?  Or do too few terminals have such support?
> 
> I tried this but this combination doesn't seem to change the display.

I think this might depend on the terminal.  E.g., the display in Emacs
does change, at least here and with fonts I'm using for Emoji.

Anyway, just a minor comment, I don't think it's a serious issue.

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> >> From: Tom Tromey <tromey@adacore.com>
>> >> Date: Fri, 09 May 2025 14:18:51 -0600
>> >> Cc: Tom Tromey <tromey@adacore.com>
>> >> 
>> >> +/* Emoji check-mark.  */
>> >> +static std::string check_mark = "✓";
>> 
>> Eli> Btw, if this is related to emoji display, shouldn't the default be
>> Eli> U+2713 followed by U+FE0F VARIATION SELECTOR-16, to show the "emoji
>> Eli> presentation" of the CHECK MARK character, if the terminal supports
>> Eli> that?  Or do too few terminals have such support?
>> 
>> I tried this but this combination doesn't seem to change the display.
>
> I think this might depend on the terminal.  E.g., the display in Emacs
> does change, at least here and with fonts I'm using for Emoji.

In this specific example, could it just be that U+2713 CHECK MARK has no
emoji presentation, or am I reading emoji-data.txt wrong?

From my emacs.git copy:

    # ================================================

    # All omitted code points have Emoji=No

    […]
    2705          ; Emoji                # E0.6   [1] (✅)       check mark button
    2708..270C    ; Emoji                # E0.6   [5] (✈️..✌️)    airplane..victory hand
    270D          ; Emoji                # E0.7   [1] (✍️)       writing hand
    270F          ; Emoji                # E0.6   [1] (✏️)       pencil
    2712          ; Emoji                # E0.6   [1] (✒️)       black nib
    2714          ; Emoji                # E0.6   [1] (✔️)       check mark
    […]

    # ================================================

    # All omitted code points have Emoji_Presentation=No

    […]
    2705          ; Emoji_Presentation   # E0.6   [1] (✅)       check mark button
    270A..270B    ; Emoji_Presentation   # E0.6   [2] (✊..✋)    raised fist..raised hand
    2728          ; Emoji_Presentation   # E0.6   [1] (✨)       sparkles

Notably absent: 2713?  So 2714 does vary with (✔️) & without (✔) VS-16,
but not 2713?

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Eli Zaretskii]

> From: Kévin Le Gouguec <legouguec@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>,  gdb-patches@sourceware.org
> Date: Fri, 23 May 2025 17:00:31 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> >> From: Tom Tromey <tromey@adacore.com>
> >> >> Date: Fri, 09 May 2025 14:18:51 -0600
> >> >> Cc: Tom Tromey <tromey@adacore.com>
> >> >> 
> >> >> +/* Emoji check-mark.  */
> >> >> +static std::string check_mark = "✓";
> >> 
> >> Eli> Btw, if this is related to emoji display, shouldn't the default be
> >> Eli> U+2713 followed by U+FE0F VARIATION SELECTOR-16, to show the "emoji
> >> Eli> presentation" of the CHECK MARK character, if the terminal supports
> >> Eli> that?  Or do too few terminals have such support?
> >> 
> >> I tried this but this combination doesn't seem to change the display.
> >
> > I think this might depend on the terminal.  E.g., the display in Emacs
> > does change, at least here and with fonts I'm using for Emoji.
> 
> In this specific example, could it just be that U+2713 CHECK MARK has no
> emoji presentation, or am I reading emoji-data.txt wrong?
> 
> >From my emacs.git copy:
> 
>     # ================================================
> 
>     # All omitted code points have Emoji=No
> 
>     […]
>     2705          ; Emoji                # E0.6   [1] (✅)       check mark button
>     2708..270C    ; Emoji                # E0.6   [5] (✈️..✌️)    airplane..victory hand
>     270D          ; Emoji                # E0.7   [1] (✍️)       writing hand
>     270F          ; Emoji                # E0.6   [1] (✏️)       pencil
>     2712          ; Emoji                # E0.6   [1] (✒️)       black nib
>     2714          ; Emoji                # E0.6   [1] (✔️)       check mark
>     […]
> 
>     # ================================================
> 
>     # All omitted code points have Emoji_Presentation=No
> 
>     […]
>     2705          ; Emoji_Presentation   # E0.6   [1] (✅)       check mark button
>     270A..270B    ; Emoji_Presentation   # E0.6   [2] (✊..✋)    raised fist..raised hand
>     2728          ; Emoji_Presentation   # E0.6   [1] (✨)       sparkles
> 
> Notably absent: 2713?  So 2714 does vary with (✔️) & without (✔) VS-16,
> but not 2713?

All I know is that the display in Emacs does change, as I said.

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Tom Tromey]

>>>>> "Kévin" == Kévin Le Gouguec <legouguec@adacore.com> writes:

Kévin> Notably absent: 2713?  So 2714 does vary with (✔️) & without (✔) VS-16,
Kévin> but not 2713?

I ended up going with 2714 plus the emoji presentation character.

Tom

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Andrew Burgess]

Tom Tromey <tromey@adacore.com> writes:

> In keeping with the emojification of gdb, this patch changes the
> default "current" marker to be a check-mark.  It adds a knob to allow
> the character to be changed, and, as always, reverts to the old output
> when emojis are disabled.

I haven't tested this patch just yet (short of time), but I wonder if I
set the check-mark character to a multi-character string, does that
corrupt the output?  I'm guessing so as I don't think table columns
auto-adjust.

So my real question: at a minimum, should the docs mention that the
STRING should only be a single character.  And better yet, can we check
that the string is only a single character in width somewhere?

Thanks,
Andrew


> ---
>  gdb/NEWS                         |  6 ++++++
>  gdb/cli-out.c                    |  8 ++++++++
>  gdb/cli-out.h                    |  2 ++
>  gdb/cli/cli-style.c              | 32 ++++++++++++++++++++++++++++++++
>  gdb/cli/cli-style.h              |  3 +++
>  gdb/doc/gdb.texinfo              | 10 ++++++++++
>  gdb/testsuite/gdb.base/style.exp |  4 ++++
>  7 files changed, 65 insertions(+)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index a82b7e3342c57b9b066e9012c5a961ad6ba839f0..07e72833a269ffe0ab034d4c4ea8b185ad217983 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -78,6 +78,12 @@ set style error-prefix STRING
>    with emoji display, and so the prefixes are only displayed if emoji
>    styling is enabled.
>  
> +set style check-mark STRING
> +  Set the string that is used to indicate the "current" row when a
> +  table is printed by the CLI.  This functionality is intended for use
> +  with emoji display, and so the prefixes are only displayed if emoji
> +  styling is enabled.
> +
>  info linker-namespaces
>  info linker-namespaces [[N]]
>    Print information about the given linker namespace (identified as N),
> diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> index 27a82f607eb3d6abcd9867880e99f824a0392312..c619bb4243dbfe4bdc0fb442e7496aa066b52575 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -421,6 +421,14 @@ cli_ui_out::do_progress_end ()
>    m_progress_info.pop_back ();
>  }
>  
> +/* See ui-out.h.  */
> +
> +const char *
> +cli_ui_out::get_check_mark ()
> +{
> +  return ::get_check_mark ();
> +}
> +
>  /* local functions */
>  
>  void
> diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> index c761dacc9a83cf5ffadfca2ee4751a37c2159402..342f2d0a15fb1d3823214d2f946f4ac10ca9346d 100644
> --- a/gdb/cli-out.h
> +++ b/gdb/cli-out.h
> @@ -81,6 +81,8 @@ class cli_ui_out : public ui_out
>  				   double, double) override;
>    virtual void do_progress_end () override;
>  
> +  const char *get_check_mark () override;
> +
>    bool suppress_output ()
>    { return m_suppress_output; }
>  
> diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
> index e6441277d05f5215ecf3f04a8c82be65d9fd031d..ea56bbccf255b1a11c1ef4b3dfb7e06c5dc9656e 100644
> --- a/gdb/cli/cli-style.c
> +++ b/gdb/cli/cli-style.c
> @@ -494,6 +494,29 @@ print_error_prefix (ui_file *file)
>      gdb_puts (error_prefix.c_str (), file);
>  }
>  
> +/* Emoji check-mark.  */
> +static std::string check_mark = "✓";
> +
> +/* Implement 'show style check-mark'.  */
> +
> +static void
> +show_check_mark (struct ui_file *file, int from_tty,
> +		 struct cmd_list_element *c, const char *value)
> +{
> +  gdb_printf (file, _("Check mark is \"%s\".\n"),
> +	      check_mark.c_str ());
> +}
> +
> +/* See cli-style.h.  */
> +
> +const char *
> +get_check_mark ()
> +{
> +  if (emojis_ok ())
> +    return check_mark.c_str ();
> +  return "*";
> +}
> +
>  void _initialize_cli_style ();
>  void
>  _initialize_cli_style ()
> @@ -737,4 +760,13 @@ The error prefix text is displayed before any error, when\n\
>  emoji output is enabled."),
>  			  nullptr, show_error_prefix,
>  			  &style_set_list, &style_show_list);
> +  add_setshow_string_cmd ("check-mark", no_class,
> +			  &check_mark,
> +			  _("Set the check mark text."),
> +			  _("Show the check mark text."),
> +			  _("\
> +The check mark text is displayed in a table to indicate which\n\
> +row is currently selected."),
> +			  nullptr, show_check_mark,
> +			  &style_set_list, &style_show_list);
>  }
> diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
> index b1a950a539cb8eaf5381f3ea546ee9fa3e87500b..7b88424d972303b592149499985e7d868494be03 100644
> --- a/gdb/cli/cli-style.h
> +++ b/gdb/cli/cli-style.h
> @@ -204,4 +204,7 @@ extern void print_warning_prefix (ui_file *file);
>  /* Print the error prefix, if desired.  */
>  extern void print_error_prefix (ui_file *file);
>  
> +/* Return the current check mark string.  */
> +extern const char *get_check_mark ();
> +
>  #endif /* GDB_CLI_CLI_STYLE_H */
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 5e5e888cf38cea33a9d7b4e77039207374ec2d5e..539b3cba92d6893dcd843f684b00c91037324c0b 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -28014,6 +28014,16 @@ emoji display, and so the prefixes are only displayed if emoji styling
>  is enabled.  The defaults are the warning sign emoji for warnings, and
>  and the cross mark emoji for errors.
>  
> +@item set style check-mark @var{string}
> +@itemx show style check-mark
> +These commands control the string that the CLI uses to indicate which
> +line in a table indicates the current state of @value{GDBN}.  For
> +example, this would be used to indicate the current thread in the
> +output of @samp{info threads}.  This functionality is intended for use
> +with emoji display, and so the chosen string only displayed if emoji
> +styling is enabled.  The default is a check box character; with the
> +non-emoji fallback being an asterisk.
> +
>  @item set style tui-current-position @samp{on|off}
>  Enable or disable styling of the source and assembly code highlighted
>  by the TUI's current position indicator.  The default is @samp{off}.
> diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
> index c10be3bc12aa10f5a13856cb14f7fad86f32ff90..9f2a288e953f0711d698673e723d6192c3862d79 100644
> --- a/gdb/testsuite/gdb.base/style.exp
> +++ b/gdb/testsuite/gdb.base/style.exp
> @@ -341,6 +341,10 @@ proc run_style_tests { } {
>  	gdb_test "maint translate-address" \
>  	    "abcd:requires argument.*" \
>  	    "error prefix"
> +
> +	gdb_test_no_output "set style check-mark @"
> +	gdb_test "info inferior" "\r\n@ 1 .*"
> +	gdb_test "show style check-mark" "Check mark is \"@\"\\."
>      }
>  }
>  
>
> -- 
> 2.49.0

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Tom Tromey <tromey@adacore.com> writes:
>> In keeping with the emojification of gdb, this patch changes the
>> default "current" marker to be a check-mark.  It adds a knob to allow
>> the character to be changed, and, as always, reverts to the old output
>> when emojis are disabled.

Andrew> I haven't tested this patch just yet (short of time), but I wonder if I
Andrew> set the check-mark character to a multi-character string, does that
Andrew> corrupt the output?  I'm guessing so as I don't think table columns
Andrew> auto-adjust.

Andrew> So my real question: at a minimum, should the docs mention that the
Andrew> STRING should only be a single character.  And better yet, can we check
Andrew> that the string is only a single character in width somewhere?

Yeah, I will update the docs in v2.

I don't think there's any way to figure out what the display width of a
string might be.  At least, not unless gdb adds a dependency on
something like libicu.

Tom

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Eli Zaretskii]

> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>,  gdb-patches@sourceware.org
> Date: Fri, 16 May 2025 08:20:03 -0600
> 
> >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
> 
> Andrew> Tom Tromey <tromey@adacore.com> writes:
> >> In keeping with the emojification of gdb, this patch changes the
> >> default "current" marker to be a check-mark.  It adds a knob to allow
> >> the character to be changed, and, as always, reverts to the old output
> >> when emojis are disabled.
> 
> Andrew> I haven't tested this patch just yet (short of time), but I wonder if I
> Andrew> set the check-mark character to a multi-character string, does that
> Andrew> corrupt the output?  I'm guessing so as I don't think table columns
> Andrew> auto-adjust.
> 
> Andrew> So my real question: at a minimum, should the docs mention that the
> Andrew> STRING should only be a single character.  And better yet, can we check
> Andrew> that the string is only a single character in width somewhere?
> 
> Yeah, I will update the docs in v2.
> 
> I don't think there's any way to figure out what the display width of a
> string might be.  At least, not unless gdb adds a dependency on
> something like libicu.

I don't think even libicu will be enough, because the terminal
emulators vary in this respect.

There's some recent protocol to help with this, but I don't think it's
supported widely enough for GDB to rely on it.

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Pedro Alves]

Hi,

On 2025-05-16 17:16, Eli Zaretskii wrote:
>> From: Tom Tromey <tromey@adacore.com>
>> Cc: Tom Tromey <tromey@adacore.com>,  gdb-patches@sourceware.org
>> Date: Fri, 16 May 2025 08:20:03 -0600
>>
>>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>>
>> Andrew> Tom Tromey <tromey@adacore.com> writes:
>>>> In keeping with the emojification of gdb, this patch changes the
>>>> default "current" marker to be a check-mark.  It adds a knob to allow
>>>> the character to be changed, and, as always, reverts to the old output
>>>> when emojis are disabled.
>>
>> Andrew> I haven't tested this patch just yet (short of time), but I wonder if I
>> Andrew> set the check-mark character to a multi-character string, does that
>> Andrew> corrupt the output?  I'm guessing so as I don't think table columns
>> Andrew> auto-adjust.
>>
>> Andrew> So my real question: at a minimum, should the docs mention that the
>> Andrew> STRING should only be a single character.  And better yet, can we check
>> Andrew> that the string is only a single character in width somewhere?
>>
>> Yeah, I will update the docs in v2.
>>
>> I don't think there's any way to figure out what the display width of a
>> string might be.  At least, not unless gdb adds a dependency on
>> something like libicu.
> 
> I don't think even libicu will be enough, because the terminal
> emulators vary in this respect.
> 
> There's some recent protocol to help with this, but I don't think it's
> supported widely enough for GDB to rely on it.

I'm curious on what protocol is this, and what it addresses.

If we assume monospace font in the terminal, what goes wrong?

I mean, if someone uses a non-monospace font, then everything will be unaligned anyhow.

Is there a downside to using the number of columns wcwidth says the character occupies,
and default to 1 if wcwidth returns -1?

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Eli Zaretskii]

> Date: Wed, 25 Jun 2025 20:11:07 +0100
> Cc: aburgess@redhat.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> 
> >> I don't think there's any way to figure out what the display width of a
> >> string might be.  At least, not unless gdb adds a dependency on
> >> something like libicu.
> > 
> > I don't think even libicu will be enough, because the terminal
> > emulators vary in this respect.
> > 
> > There's some recent protocol to help with this, but I don't think it's
> > supported widely enough for GDB to rely on it.
> 
> I'm curious on what protocol is this, and what it addresses.

That's "Mode 2027".  For the details, see

     https://github.com/contour-terminal/terminal-unicode-core
and
     https://mitchellh.com/writing/grapheme-clusters-in-terminals

> If we assume monospace font in the terminal, what goes wrong?

Monospace font is only relevant for single-character strings.  Once
you try to display a string that has more than one Unicode character,
a terminal may or may not combine those characters into one or more
font glyphs (it's a many-to-many relation, so you could have M
characters displayed as N glyphs, where M could be lass than, equal,
or greater than N, depending on the characters in the string and the
capabilities of the font used by the terminal).  If that happens,
wcswidth will usually produce an incorrect result, because it doesn't
(and cannot) know about these features of the terminal, and basically
just sums the results of wcwidth of each character in the string.

> Is there a downside to using the number of columns wcwidth says the character occupies,
> and default to 1 if wcwidth returns -1?

See above.  This discussion started in the context of showing Emoji,
where the case of combining several characters into one or more glyphs
happens very frequently.  For example, the sequence U+2713 followed by
U+FE0F VARIATION SELECTOR-16 should display a single glyph on capable
terminals (try it in a GUI session in Emacs to see that), although
it's two characters.  In some cases, wcswidth will do the job, because
control characters like U+FE0F have zero width in its database, but
that is not universally so.  For example, the following string of
characters:

  U+1F468 U+200D U+1F469 U+200D U+1F466

will be displayed by capable terminals as a single glyph (again, try
in Emacs to see), but wcswidth will probably return 6 for it.

And even for single-character strings wcwidth will sometimes produce
incorrect results, because the fonts used by modern terminal emulators
for some exotic characters (like Emoji) are not monospaced.

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Pedro Alves]

On 2025-06-26 06:51, Eli Zaretskii wrote:
>> Date: Wed, 25 Jun 2025 20:11:07 +0100
>> Cc: aburgess@redhat.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>>>> I don't think there's any way to figure out what the display width of a
>>>> string might be.  At least, not unless gdb adds a dependency on
>>>> something like libicu.
>>>
>>> I don't think even libicu will be enough, because the terminal
>>> emulators vary in this respect.
>>>
>>> There's some recent protocol to help with this, but I don't think it's
>>> supported widely enough for GDB to rely on it.
>>
>> I'm curious on what protocol is this, and what it addresses.
> 
> That's "Mode 2027".  For the details, see
> 
>      https://github.com/contour-terminal/terminal-unicode-core
> and
>      https://mitchellh.com/writing/grapheme-clusters-in-terminals
> 

Thank you, that's quite a good read.

As the blog explains, 

 "historically terminals used wcwidth, shells, editors, and other TUI apps also use wcwidth, and continue
  to do so today. Even though it produces the wrong values for multi-codepoint graphemes, at least the
  wrong values are often consistently wrong across terminal emulators."

So what can go wrong is grapheme clustering.  Both the terminal and the app must agree on
clustering for that to work properly.  We could implement that protocol in GDB today, as there's
a way to query if the terminal supports it.  But I'm not proposing that.

So if we ignore multi-codepoint graphemes, then wcwidth always matches what the
terminals do.


>> If we assume monospace font in the terminal, what goes wrong?
> 
> Monospace font is only relevant for single-character strings.  Once
> you try to display a string that has more than one Unicode character,
> a terminal may or may not combine those characters into one or more
> font glyphs (it's a many-to-many relation, so you could have M
> characters displayed as N glyphs, where M could be lass than, equal,
> or greater than N, depending on the characters in the string and the
> capabilities of the font used by the terminal).  If that happens,
> wcswidth will usually produce an incorrect result, because it doesn't
> (and cannot) know about these features of the terminal, and basically
> just sums the results of wcwidth of each character in the string.
> 
>> Is there a downside to using the number of columns wcwidth says the character occupies,
>> and default to 1 if wcwidth returns -1?
> 
> See above.  This discussion started in the context of showing Emoji,
> where the case of combining several characters into one or more glyphs
> happens very frequently.  For example, the sequence U+2713 followed by
> U+FE0F VARIATION SELECTOR-16 should display a single glyph on capable
> terminals (try it in a GUI session in Emacs to see that), although
> it's two characters.  In some cases, wcswidth will do the job, because
> control characters like U+FE0F have zero width in its database, but
> that is not universally so.

OK, but we can just avoid picking emojis that are encoded with variation selector
for the defaults where the gryphs would be used in a context where alignment
matters, and document that picking such an emoji in your custom style may misbehave.

> For example, the following string of
> characters:
> 
>   U+1F468 U+200D U+1F469 U+200D U+1F466
> 
> will be displayed by capable terminals as a single glyph (again, try
> in Emacs to see), but wcswidth will probably return 6 for it.
> 

Note that for the use case we're talking about, the character used as current-line indicator,
it's always going to be " <marker> ", with spaces or tabs or endline surrounding it,
there is no risk of the character we choose bumping into other characters we don't control
and forming a cluster sequence.  If <marker> itself is not a cluster, and is a single
(maybe wide) character, then wcwidth returns the right thing.

IOW, if we do use wcwidth, then it's going to be right for non-cluster multi-cell
characters, which is a large pool of reasonable emojis that we can choose from for the
use case at hand.

I'd say that we could even consider letting the user pick a multi-character sequence for the
marker, and we would wcswidth instead of wcwidth.  Then, if GDB and the terminal disagree on
grapheme width, well, the downside is that the user gets their tables misaligned, but that is no
worse from what happens if we _don't_ consider wcwidth at all.  But not going there, and only
allowing single character emojis, and using wcwidth, is already going to do the right thing and
be quite useful.

> And even for single-character strings wcwidth will sometimes produce
> incorrect results, because the fonts used by modern terminal emulators
> for some exotic characters (like Emoji) are not monospaced.
> 

As long as we don't pick such a character as the default one, it's going to
be on the user not to pick one.

The blog post you pointed at makes me believe that we _should_ indeed
use wcwidth.  If we work under the single-character constraint, then I don't
really see a downside to using wcwidth.

For the wcwidth returns -1 scenario, I see two choices: 

 - assume width of one cell, and print marker from style anyhow, or,

 - fallback to hardcoded "*" character, like today's output.

Pedro Alves

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Eli Zaretskii]

> Date: Thu, 26 Jun 2025 11:35:37 +0100
> Cc: tromey@adacore.com, aburgess@redhat.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> 
> As the blog explains, 
> 
>  "historically terminals used wcwidth, shells, editors, and other TUI apps also use wcwidth, and continue
>   to do so today. Even though it produces the wrong values for multi-codepoint graphemes, at least the
>   wrong values are often consistently wrong across terminal emulators."
> 
> So what can go wrong is grapheme clustering.  Both the terminal and the app must agree on
> clustering for that to work properly.  We could implement that protocol in GDB today, as there's
> a way to query if the terminal supports it.  But I'm not proposing that.
> 
> So if we ignore multi-codepoint graphemes, then wcwidth always matches what the
> terminals do.

Using wcwidth should probably work in many cases, but we still assume
that the terminal uses a font whose glyphs' width agrees with wcwidth.
That is not a given.

> > For example, the following string of
> > characters:
> > 
> >   U+1F468 U+200D U+1F469 U+200D U+1F466
> > 
> > will be displayed by capable terminals as a single glyph (again, try
> > in Emacs to see), but wcswidth will probably return 6 for it.
> 
> Note that for the use case we're talking about, the character used as current-line indicator,
> it's always going to be " <marker> ", with spaces or tabs or endline surrounding it,
> there is no risk of the character we choose bumping into other characters we don't control
> and forming a cluster sequence.  If <marker> itself is not a cluster, and is a single
> (maybe wide) character, then wcwidth returns the right thing.

Don't we allow the users to specify their own characters or strings
for this?  If not, all we have to do is verify that the characters we
pick for these purposes don't have the problems and always (or at
least in most cases) agree with wcwidth.

> For the wcwidth returns -1 scenario, I see two choices: 
> 
>  - assume width of one cell, and print marker from style anyhow, or,
> 
>  - fallback to hardcoded "*" character, like today's output.

This is a subset of a more general issue with wcwidth: it cannot be
relied upon to know about arbitrary characters, unless the locale's
codeset is UTF-8.  Because usually wcwidth only knows about characters
supported by the locale's codeset.

The best solution to overcome this is to have our own database of
character-width values.  (We actually only need the characters whose
width is different from 1, i.e. zero-width and double-width
characters.)  That's what Emacs does.  The downside is that we'd need
to maintain this database as characters are added to Unicode (we could
have a script that generates the database automatically from a
relevant Unicode data file).

Another alternative is to allow the definition of the width as part of
defining the character(s) used for each indicator.  Then we won't need
wcwidth/wcswidth at all.

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Pedro Alves]

Hi Eli,

On 2025-06-26 13:32, Eli Zaretskii wrote:
>> Date: Thu, 26 Jun 2025 11:35:37 +0100
>> Cc: tromey@adacore.com, aburgess@redhat.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>> As the blog explains, 
>>
>>  "historically terminals used wcwidth, shells, editors, and other TUI apps also use wcwidth, and continue
>>   to do so today. Even though it produces the wrong values for multi-codepoint graphemes, at least the
>>   wrong values are often consistently wrong across terminal emulators."
>>
>> So what can go wrong is grapheme clustering.  Both the terminal and the app must agree on
>> clustering for that to work properly.  We could implement that protocol in GDB today, as there's
>> a way to query if the terminal supports it.  But I'm not proposing that.
>>
>> So if we ignore multi-codepoint graphemes, then wcwidth always matches what the
>> terminals do.
> 
> Using wcwidth should probably work in many cases, but we still assume
> that the terminal uses a font whose glyphs' width agrees with wcwidth.
> That is not a given.

What I understood from the blog post or from reading a number of bug reports and discussions
of several different terminal projects, is that terminals lay the glyphs in a grid, and glyphs 
occupy one or more slots in the grid.  The font itself shouldn't really matter to compute the
number of grid slots, it's an independent decision.  Of course, there may well be broken terminals
out there.  But what should matter is whether the characters/emojis we pick by default work well.
If the user picks a character or emoji in their custom style that doesn't work in their broken
terminal, then well, they can just not use that particular character/emoji in their custom style.

> 
>>> For example, the following string of
>>> characters:
>>>
>>>   U+1F468 U+200D U+1F469 U+200D U+1F466
>>>
>>> will be displayed by capable terminals as a single glyph (again, try
>>> in Emacs to see), but wcswidth will probably return 6 for it.
>>
>> Note that for the use case we're talking about, the character used as current-line indicator,
>> it's always going to be " <marker> ", with spaces or tabs or endline surrounding it,
>> there is no risk of the character we choose bumping into other characters we don't control
>> and forming a cluster sequence.  If <marker> itself is not a cluster, and is a single
>> (maybe wide) character, then wcwidth returns the right thing.
> 
> Don't we allow the users to specify their own characters or strings
> for this?  If not, all we have to do is verify that the characters we
> pick for these purposes don't have the problems and always (or at
> least in most cases) agree with wcwidth.

Exactly, that's what I meant.  We do allow users to specify their own
characters (that's what the patch this discussion is under does), but if their terminal
is broken somehow, then it's going to be on the user to not use custom characters that
don't work on their terminal.  I think that we should use wcwidth/wcswidth like every
other terminal application as explained on that blog, and document that for the characters
used in styles that may effect table alignment, cluster sequences etc. may not work properly.
We don't need to worry about width of characters used in places that are not tables, like
the error or warning emojis.

> 
>> For the wcwidth returns -1 scenario, I see two choices: 
>>
>>  - assume width of one cell, and print marker from style anyhow, or,
>>
>>  - fallback to hardcoded "*" character, like today's output.
> 
> This is a subset of a more general issue with wcwidth: it cannot be
> relied upon to know about arbitrary characters, unless the locale's
> codeset is UTF-8.  Because usually wcwidth only knows about characters
> supported by the locale's codeset.

I guess Windows is the main concern here, though I was reading this:

https://github.com/alf-p-steinbach/C---how-to---make-non-English-text-work-in-Windows/blob/main/how-to-use-utf8-in-windows.md

... and it looks like there's a way nowadays to make the application use the UTF-8 codepage,
via a manifest.

> 
> The best solution to overcome this is to have our own database of
> character-width values.  (We actually only need the characters whose
> width is different from 1, i.e. zero-width and double-width
> characters.)  That's what Emacs does.> The downside is that we'd need
> to maintain this database as characters are added to Unicode (we could
> have a script that generates the database automatically from a
> relevant Unicode data file).
> 
> Another alternative is to allow the definition of the width as part of
> defining the character(s) used for each indicator.  Then we won't need
> wcwidth/wcswidth at all.
> 

Or something in between -- have a wrapper for wcwidth that only hardcodes
the width of the few emojis GDB uses by default?  I haven't looked at
the gnulib wrapper that Tromey mentioned though, maybe it already does
something like you mention for all characters.

Pedro Alves

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Pedro Alves]

On 2025-07-01 00:51, Pedro Alves wrote:

> What I understood from the blog post or from reading a number of bug reports and discussions
> of several different terminal projects, 

I wanted to share this link which I found particularly interesting, but of course I was
only able to re-find it 2 minutes after pressing send.  :-)  Here is it:

  https://github.com/microsoft/terminal/issues/16852

Particularly these two messages:

  https://github.com/microsoft/terminal/issues/16852#issuecomment-1987267067
  https://github.com/microsoft/terminal/issues/16852#issuecomment-1987267479

.. and the rest starting from those.

Pedro Alves

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Andrew Burgess]

Tom Tromey <tromey@adacore.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> Tom Tromey <tromey@adacore.com> writes:
>>> In keeping with the emojification of gdb, this patch changes the
>>> default "current" marker to be a check-mark.  It adds a knob to allow
>>> the character to be changed, and, as always, reverts to the old output
>>> when emojis are disabled.
>
> Andrew> I haven't tested this patch just yet (short of time), but I wonder if I
> Andrew> set the check-mark character to a multi-character string, does that
> Andrew> corrupt the output?  I'm guessing so as I don't think table columns
> Andrew> auto-adjust.
>
> Andrew> So my real question: at a minimum, should the docs mention that the
> Andrew> STRING should only be a single character.  And better yet, can we check
> Andrew> that the string is only a single character in width somewhere?
>
> Yeah, I will update the docs in v2.
>
> I don't think there's any way to figure out what the display width of a
> string might be.  At least, not unless gdb adds a dependency on
> something like libicu.

I did wonder if figuring out string length might be a challenge.  I
think just documenting the single character requirement is sufficient.

Thanks,
Andrew

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Pedro Alves]

Hi!

On 2025-05-09 21:18, Tom Tromey wrote:
> In keeping with the emojification of gdb, this patch changes the
> default "current" marker to be a check-mark.  It adds a knob to allow
> the character to be changed, and, as always, reverts to the old output
> when emojis are disabled.

I think it'd be great if such patches came with some output example in the commit log.

For this one, I have to say that I'm surprised that the emoji being proposed as
default is a "✓" check mark?

That to me would be more appropriate to indicate "completed" or "good" or "enabled".

To indicate the selected entry from a list, I think a right arrow, or "👉" (Backhand Index Pointing Right, U+1F449)
would make a lot more sense.

What's the rationale for going with a check mark?  I'd even claim that we shouldn't call this one check-mark,
as that would be better used for an emoji truly representing whether something is on/off.  Like e.g., I could
see us using that in "info breakpoints" output, in the column where we indicate a breakpoint location is
enabled or not.  And other columns in other tables.

I quite like the ongoing emojification, btw!

Pedro Alves

> ---
>  gdb/NEWS                         |  6 ++++++
>  gdb/cli-out.c                    |  8 ++++++++
>  gdb/cli-out.h                    |  2 ++
>  gdb/cli/cli-style.c              | 32 ++++++++++++++++++++++++++++++++
>  gdb/cli/cli-style.h              |  3 +++
>  gdb/doc/gdb.texinfo              | 10 ++++++++++
>  gdb/testsuite/gdb.base/style.exp |  4 ++++
>  7 files changed, 65 insertions(+)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index a82b7e3342c57b9b066e9012c5a961ad6ba839f0..07e72833a269ffe0ab034d4c4ea8b185ad217983 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -78,6 +78,12 @@ set style error-prefix STRING
>    with emoji display, and so the prefixes are only displayed if emoji
>    styling is enabled.
>  
> +set style check-mark STRING
> +  Set the string that is used to indicate the "current" row when a
> +  table is printed by the CLI.  This functionality is intended for use
> +  with emoji display, and so the prefixes are only displayed if emoji
> +  styling is enabled.
> +
>  info linker-namespaces
>  info linker-namespaces [[N]]
>    Print information about the given linker namespace (identified as N),
> diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> index 27a82f607eb3d6abcd9867880e99f824a0392312..c619bb4243dbfe4bdc0fb442e7496aa066b52575 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -421,6 +421,14 @@ cli_ui_out::do_progress_end ()
>    m_progress_info.pop_back ();
>  }
>  
> +/* See ui-out.h.  */
> +
> +const char *
> +cli_ui_out::get_check_mark ()
> +{
> +  return ::get_check_mark ();
> +}
> +
>  /* local functions */
>  
>  void
> diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> index c761dacc9a83cf5ffadfca2ee4751a37c2159402..342f2d0a15fb1d3823214d2f946f4ac10ca9346d 100644
> --- a/gdb/cli-out.h
> +++ b/gdb/cli-out.h
> @@ -81,6 +81,8 @@ class cli_ui_out : public ui_out
>  				   double, double) override;
>    virtual void do_progress_end () override;
>  
> +  const char *get_check_mark () override;
> +
>    bool suppress_output ()
>    { return m_suppress_output; }
>  
> diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
> index e6441277d05f5215ecf3f04a8c82be65d9fd031d..ea56bbccf255b1a11c1ef4b3dfb7e06c5dc9656e 100644
> --- a/gdb/cli/cli-style.c
> +++ b/gdb/cli/cli-style.c
> @@ -494,6 +494,29 @@ print_error_prefix (ui_file *file)
>      gdb_puts (error_prefix.c_str (), file);
>  }
>  
> +/* Emoji check-mark.  */
> +static std::string check_mark = "✓";
> +
> +/* Implement 'show style check-mark'.  */
> +
> +static void
> +show_check_mark (struct ui_file *file, int from_tty,
> +		 struct cmd_list_element *c, const char *value)
> +{
> +  gdb_printf (file, _("Check mark is \"%s\".\n"),
> +	      check_mark.c_str ());
> +}
> +
> +/* See cli-style.h.  */
> +
> +const char *
> +get_check_mark ()
> +{
> +  if (emojis_ok ())
> +    return check_mark.c_str ();
> +  return "*";
> +}
> +
>  void _initialize_cli_style ();
>  void
>  _initialize_cli_style ()
> @@ -737,4 +760,13 @@ The error prefix text is displayed before any error, when\n\
>  emoji output is enabled."),
>  			  nullptr, show_error_prefix,
>  			  &style_set_list, &style_show_list);
> +  add_setshow_string_cmd ("check-mark", no_class,
> +			  &check_mark,
> +			  _("Set the check mark text."),
> +			  _("Show the check mark text."),
> +			  _("\
> +The check mark text is displayed in a table to indicate which\n\
> +row is currently selected."),
> +			  nullptr, show_check_mark,
> +			  &style_set_list, &style_show_list);
>  }
> diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
> index b1a950a539cb8eaf5381f3ea546ee9fa3e87500b..7b88424d972303b592149499985e7d868494be03 100644
> --- a/gdb/cli/cli-style.h
> +++ b/gdb/cli/cli-style.h
> @@ -204,4 +204,7 @@ extern void print_warning_prefix (ui_file *file);
>  /* Print the error prefix, if desired.  */
>  extern void print_error_prefix (ui_file *file);
>  
> +/* Return the current check mark string.  */
> +extern const char *get_check_mark ();
> +
>  #endif /* GDB_CLI_CLI_STYLE_H */
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 5e5e888cf38cea33a9d7b4e77039207374ec2d5e..539b3cba92d6893dcd843f684b00c91037324c0b 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -28014,6 +28014,16 @@ emoji display, and so the prefixes are only displayed if emoji styling
>  is enabled.  The defaults are the warning sign emoji for warnings, and
>  and the cross mark emoji for errors.
>  
> +@item set style check-mark @var{string}
> +@itemx show style check-mark
> +These commands control the string that the CLI uses to indicate which
> +line in a table indicates the current state of @value{GDBN}.  For
> +example, this would be used to indicate the current thread in the
> +output of @samp{info threads}.  This functionality is intended for use
> +with emoji display, and so the chosen string only displayed if emoji
> +styling is enabled.  The default is a check box character; with the
> +non-emoji fallback being an asterisk.
> +
>  @item set style tui-current-position @samp{on|off}
>  Enable or disable styling of the source and assembly code highlighted
>  by the TUI's current position indicator.  The default is @samp{off}.
> diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
> index c10be3bc12aa10f5a13856cb14f7fad86f32ff90..9f2a288e953f0711d698673e723d6192c3862d79 100644
> --- a/gdb/testsuite/gdb.base/style.exp
> +++ b/gdb/testsuite/gdb.base/style.exp
> @@ -341,6 +341,10 @@ proc run_style_tests { } {
>  	gdb_test "maint translate-address" \
>  	    "abcd:requires argument.*" \
>  	    "error prefix"
> +
> +	gdb_test_no_output "set style check-mark @"
> +	gdb_test "info inferior" "\r\n@ 1 .*"
> +	gdb_test "show style check-mark" "Check mark is \"@\"\\."
>      }
>  }
>  
> 

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Tom Tromey]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

Pedro> To indicate the selected entry from a list, I think a right
Pedro> arrow, or "👉" (Backhand Index Pointing Right, U+1F449) would
Pedro> make a lot more sense.

I tried that one just now and at least on my terminal it is wider.
See the attached screenshot.

Now, we could make "current" take 2 columns.  That might be enough to
solve this in general.  But then a bit of space is wasted in the ASCII /
narrow case.

Tom


[-- Attachment #2: screenshot --]
[-- Type: image/png, Size: 18904 bytes --]

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Pedro Alves]

On 2025-06-24 17:58, Tom Tromey wrote:
> Pedro> To indicate the selected entry from a list, I think a right
> Pedro> arrow, or "👉" (Backhand Index Pointing Right, U+1F449) would
> Pedro> make a lot more sense.
> 
> I tried that one just now and at least on my terminal it is wider.
> See the attached screenshot.
> 
> Now, we could make "current" take 2 columns.  That might be enough to
> solve this in general.  But then a bit of space is wasted in the ASCII /
> narrow case.

Column widths don't have to be hardcoded.  For example, we compute the width of the "Target Id"
column dynamically.  See target_id_col_width in print_thread_info_1.  So maybe all we need is to
base the "current" column's width depend on the wcwidth of the character used.  Even if we end up
with a narrower character by default, the user can always configure their style to use a wider
one, so it seems to me like we should do that regardless.

I'm undecided myself between backhand and Right Arrow (➡️) or one of the latter's variants.  I like backhand
and I think its use is quite widespread for this.  OTOH, an arrow might look a bit more "professional".
Not sure, I've not really tried it any of this.

Pedro Alves

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Tom Tromey]

Pedro> So maybe all we need is to base the "current" column's width
Pedro> depend on the wcwidth of the character used.

I think to do this we'd need something better than wcwidth.  See
appended.  Also as Eli points out, the terminal itself can do something
other than whatever wcwidth reports.

Tom

#define _XOPEN_SOURCE
#include <wchar.h>
#include <stdio.h>

#define C L"👉"

int main() {
  printf ("width = %d\n", wcswidth (C, wcslen (C)));
  printf ("of char = %d\n", wcwidth (C[0]));
  return 0;
}

bapiya. gcc -o q q.c
bapiya. ./q
width = -1
of char = -1

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Pedro Alves]

On 2025-06-25 16:43, Tom Tromey wrote:
> Pedro> So maybe all we need is to base the "current" column's width
> Pedro> depend on the wcwidth of the character used.
> 
> I think to do this we'd need something better than wcwidth.  See
> appended.  Also as Eli points out, the terminal itself can do something
> other than whatever wcwidth reports.
> 
> Tom
> 
> #define _XOPEN_SOURCE
> #include <wchar.h>
> #include <stdio.h>
> 
> #define C L"👉"
> 
> int main() {
>   printf ("width = %d\n", wcswidth (C, wcslen (C)));
>   printf ("of char = %d\n", wcwidth (C[0]));
>   return 0;
> }
> 
> bapiya. gcc -o q q.c
> bapiya. ./q
> width = -1
> of char = -1
> 

That seems to be something with the locale.  Yours returns -1 for me too, but the below returns 2:

#define _XOPEN_SOURCE
#include <wchar.h>
#include <stdio.h>
#include <locale.h>

#define C L"👉"

int main()
{
  setlocale (LC_CTYPE, "");
  printf ("width = %d\n", wcswidth (C, wcslen (C)));
  printf ("of char = %d\n", wcwidth (C[0]));
  return 0;
}

$ gcc wcwidth.c -o wcwidth 
$ ./wcwidth 
width = 2
of char = 2

Re: [PATCH 2/2] Allow check-mark to be changed for CLI

[Tom Tromey]

Pedro> That seems to be something with the locale.

Ah yeah, oops.

This is also a bit depressing:

    https://www.gnu.org/software/gnulib/manual/html_node/wcswidth.html

Basically we'd need some wrapper.  Or have to import the gnulib "c32"
module.  Which I guess we could do, but that's the "ICU-style" route I
was hoping to avoid.

I'm pretty wary of using more of gnulib, given how fussy it's been in
the past.  I sometimes fantasize about removing it from the tree.

OTOH if the gnulib module isn't too painful, maybe it would let us get
rid of our home-grown wchar wrapper stuff.

gnulib also has "mbswidth" which maybe we could use instead.

Tom