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

[PATCH v2 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>
---
Changes in v2:
- Addressed review comments
- Changed default to heavy check mark with emoji presentation
- Link to v1: https://inbox.sourceware.org/gdb-patches/20250509-emoji-check-mark-v1-0-63b6c52411f3@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              | 31 +++++++++++++++++++++++++++++++
 gdb/cli/cli-style.h              |  3 +++
 gdb/doc/gdb.texinfo              | 12 ++++++++++++
 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, 95 insertions(+), 27 deletions(-)
---
base-commit: e82bc90812c54d32991326cdf3be973a5784a507
change-id: 20250509-emoji-check-mark-36415e813a85

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

[PATCH v2 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.

Approved-by: Kevin Buettner <kevinb@redhat.com>
---
 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 1c27165743738f59d21f8f57e7f758206d85e210..0d543a1f197b3c1c039a2db0449295f8e7c46be7 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -304,11 +304,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 0228027fb928160fde98e338a4eb8498722dd539..2c761cbae7f6f4ca0a3767fad10a9c2fb7df6c3a 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1165,11 +1165,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 v2 1/2] Introduce ui_out::field_check_mark

[Kevin Buettner]

On Wed, 11 Jun 2025 07:58:43 -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.
> 
> Approved-by: Kevin Buettner <kevinb@redhat.com>

Still okay, still approved.

Kevin

[PATCH v2 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.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                         |  6 ++++++
 gdb/cli-out.c                    |  8 ++++++++
 gdb/cli-out.h                    |  2 ++
 gdb/cli/cli-style.c              | 31 +++++++++++++++++++++++++++++++
 gdb/cli/cli-style.h              |  3 +++
 gdb/doc/gdb.texinfo              | 12 ++++++++++++
 gdb/testsuite/gdb.base/style.exp |  4 ++++
 7 files changed, 66 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 4dcc344b07278654b1d54ca71db6749d62890d01..fd2e909595132927ed0c2eb9e5739824f5d9942a 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..831f5051ee03aa8eae7c1f23ca6faa496052d538 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,12 @@ 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 string."),
+			  _("Show the check mark string."),
+			  _("\
+The check mark is the string used to indicate the current line in a table."),
+			  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 7d06e1f5fa4eb1fc348f8b22a90f79f019ae5e6b..916caced26cb86b14fa9fed9c03f0fd70a78ad5b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -28022,6 +28022,18 @@ 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 is only displayed if
+emoji styling is enabled.  The default is a check mark character; with
+the non-emoji fallback being an asterisk.  Note that @value{GDBN} will
+not adjust the column widths of the table, so normally this should
+only be set to a single character.
+
 @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 v2 2/2] Allow check-mark to be changed for CLI

[Simon Marchi]

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

On 6/11/25 9:58 AM, 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.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>

I have not read all the discussions about the choice of the character,
but I just wanted to report that out of the box, this doesn't look very
good for me (the width of a checkmark isn't equal to the width of a
character).  See attached `checkmark.png`.

Are we allowed to nitpick / bikeshed on the choice of character?  My
preference would be for something a bit more subtle, like an arrow that
points to the selected thing.  See `arrow.png`, which uses U+27A4.  I
find it stylish, and at least on my setup, the width is correct.

I also don't think a checkmark is "semantically" appropriate for this
case.  In my mind, it means that something is done or has completed
successfully.

Simon

[-- Attachment #2: arrow.png --]
[-- Type: image/png, Size: 110359 bytes --]

[-- Attachment #3: checkmark.png --]
[-- Type: image/png, Size: 108845 bytes --]

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

[Simon Marchi]

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

On 6/11/25 10:33 AM, Simon Marchi wrote:
> On 6/11/25 9:58 AM, 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.
>>
>> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> 
> I have not read all the discussions about the choice of the character,
> but I just wanted to report that out of the box, this doesn't look very
> good for me (the width of a checkmark isn't equal to the width of a
> character).  See attached `checkmark.png`.
> 
> Are we allowed to nitpick / bikeshed on the choice of character?  My
> preference would be for something a bit more subtle, like an arrow that
> points to the selected thing.  See `arrow.png`, which uses U+27A4.  I
> find it stylish, and at least on my setup, the width is correct.
> 
> I also don't think a checkmark is "semantically" appropriate for this
> case.  In my mind, it means that something is done or has completed
> successfully.

I actually went ahead and read the thread of v1.  It was suggested to
use the non-emoji version of U+2714.  I think I still prefer the arrow
head, but I think the non-emoji U+2714 looks good too.  See attached
screenshot.

More nitpicking: perhaps the setting should be named after what its use
is, rather than what is its default appearance.  Imagine we decide to
change the selection indicator to be a duck (U+1F986) in a later
version.  Then the "set style checkmark" setting name would not make
sense anymore.  So it should perhaps be named "set style
selection-indicator" or something like that.

Simon

[-- Attachment #2: checkmark2.png --]
[-- Type: image/png, Size: 73447 bytes --]

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

[Eli Zaretskii]

> Date: Wed, 11 Jun 2025 10:33:56 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>
> From: Simon Marchi <simark@simark.ca>
> 
> I have not read all the discussions about the choice of the character,
> but I just wanted to report that out of the box, this doesn't look very
> good for me (the width of a checkmark isn't equal to the width of a
> character).  See attached `checkmark.png`.

That's because this emoji (as many others) is a wide character and
takes 2 columns on display, and GDB evidently doesn't take that into
account in this table-like display.

> Are we allowed to nitpick / bikeshed on the choice of character?  My
> preference would be for something a bit more subtle, like an arrow that
> points to the selected thing.  See `arrow.png`, which uses U+27A4.  I
> find it stylish, and at least on my setup, the width is correct.

An arrow is not a check-mark, though.

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

[Simon Marchi]

On 6/11/25 11:01 AM, Eli Zaretskii wrote:
>> Date: Wed, 11 Jun 2025 10:33:56 -0400
>> Cc: Eli Zaretskii <eliz@gnu.org>
>> From: Simon Marchi <simark@simark.ca>
>>
>> I have not read all the discussions about the choice of the character,
>> but I just wanted to report that out of the box, this doesn't look very
>> good for me (the width of a checkmark isn't equal to the width of a
>> character).  See attached `checkmark.png`.
> 
> That's because this emoji (as many others) is a wide character and
> takes 2 columns on display, and GDB evidently doesn't take that into
> account in this table-like display.

Yeah, even the non-emoji versions do take up 2 columns, I'm not sure why
those don't result in the rest of the line being shifter.  I'm also
surprised that others didn't see this...

GDB uses strlen here to compute the number of spaces to print, in order
to align stuff correctly:

https://gitlab.com/gnutools/binutils-gdb/-/blob/master/gdb/cli-out.c#L147-168

So it necessarily won't play well with multi-byte characters.

> 
>> Are we allowed to nitpick / bikeshed on the choice of character?  My
>> preference would be for something a bit more subtle, like an arrow that
>> points to the selected thing.  See `arrow.png`, which uses U+27A4.  I
>> find it stylish, and at least on my setup, the width is correct.
> 
> An arrow is not a check-mark, though.

Why does it have to be a checkmark?

Simon

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

[Eli Zaretskii]

> X-Spam-Status: No, score=-3.0 required=5.0 tests=ALL_TRUSTED,BAYES_00,
> 	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU autolearn=unavailable
> 	autolearn_force=no version=4.0.1
> Date: Wed, 11 Jun 2025 11:18:39 -0400
> Cc: tromey@adacore.com, gdb-patches@sourceware.org
> From: Simon Marchi <simark@simark.ca>
> 
> On 6/11/25 11:01 AM, Eli Zaretskii wrote:
> >> Date: Wed, 11 Jun 2025 10:33:56 -0400
> >> Cc: Eli Zaretskii <eliz@gnu.org>
> >> From: Simon Marchi <simark@simark.ca>
> >>
> >> I have not read all the discussions about the choice of the character,
> >> but I just wanted to report that out of the box, this doesn't look very
> >> good for me (the width of a checkmark isn't equal to the width of a
> >> character).  See attached `checkmark.png`.
> > 
> > That's because this emoji (as many others) is a wide character and
> > takes 2 columns on display, and GDB evidently doesn't take that into
> > account in this table-like display.
> 
> Yeah, even the non-emoji versions do take up 2 columns, I'm not sure why
> those don't result in the rest of the line being shifter.  I'm also
> surprised that others didn't see this...
> 
> GDB uses strlen here to compute the number of spaces to print, in order
> to align stuff correctly:
> 
> https://gitlab.com/gnutools/binutils-gdb/-/blob/master/gdb/cli-out.c#L147-168
> 
> So it necessarily won't play well with multi-byte characters.

We should use wcwidth, I think.

> >> Are we allowed to nitpick / bikeshed on the choice of character?  My
> >> preference would be for something a bit more subtle, like an arrow that
> >> points to the selected thing.  See `arrow.png`, which uses U+27A4.  I
> >> find it stylish, and at least on my setup, the width is correct.
> > 
> > An arrow is not a check-mark, though.
> 
> Why does it have to be a checkmark?

It doesn't have to be, but its name is check-mark.  We can change the
name, of course.

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

[Kevin Buettner]

On Wed, 11 Jun 2025 10:33:56 -0400
Simon Marchi <simark@simark.ca> wrote:

> Are we allowed to nitpick / bikeshed on the choice of character?  My
> preference would be for something a bit more subtle, like an arrow that
> points to the selected thing.  See `arrow.png`, which uses U+27A4.  I
> find it stylish, and at least on my setup, the width is correct.

FWIW, I prefer this one too.

Kevin