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

[Pedro Alves <pedro@palves.net>]

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 \"@\"\\."
>      }
>  }
>  
>