Re: [PATCH 03/10] gdb: improve show text and help text for 'remote exec-file'

[Mark Wielaard <mark@klomp.org>]

Hi Andrew,

On Wed, 2023-08-16 at 16:54 +0100, Andrew Burgess wrote:
> The current behaviour for 'show remote exec-file' is this:
> 
>   (gdb) show remote exec-file
> 
>   (gdb) set remote exec-file /abc
>   (gdb) show remote exec-file
>   /abc
>   (gdb)
> 
> The first output, the blank line, is just GDB showing the default
> empty value.
> 
> This output is not really inline with GDB's more full sentence style
> output, so in this commit I've updated things, the output is now:
> 
>   (gdb) show remote exec-file
>   The remote exec-file is unset, the default remote executable will be used.
>   (gdb) set remote exec-file /abc
>   (gdb) show remote exec-file
>   The remote exec-file is "/abc".
>   (gdb)
> 
> Which I think is more helpful to the user.
> 
> I have also updated the help text for this setting.  Previously we had
> a set/show header line, but no body text, now we have:
> 
>   (gdb) help show remote exec-file
>   Show the remote pathname for starting inferiors.
>   This is the path, on the remote target, used when starting an inferior,
>   for example with the "run", "start", or "starti" commands.
>   This setting is only useful when debugging a remote target, otherwise,
>   this setting is not used.
>   (gdb)
> 
> Which is hopefully more helpful.

Yes, I like this, much more helpful to the user.

Reviewed-by: Mark Wielaard <mark@klomp.org>
Tested-by: Mark Wielaard <mark@klomp.org>

> ---
>  gdb/remote.c                                | 15 ++++++++++++---
>  gdb/testsuite/gdb.base/remote-exec-file.exp |  7 +++++--
>  gdb/testsuite/gdb.multi/gdb-settings.exp    |  2 +-
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index dc5dd24797e..6a61a0e41ac 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1705,7 +1705,12 @@ static void
>  show_remote_exec_file (struct ui_file *file, int from_tty,
>  		       struct cmd_list_element *cmd, const char *value)
>  {
> -  gdb_printf (file, "%s\n", get_remote_exec_file ().c_str ());
> +  const std::string &filename = get_remote_exec_file ();
> +  if (filename.empty ())
> +    gdb_printf (file, _("The remote exec-file is unset, the default remote "
> +			"executable will be used.\n"));
> +  else
> +    gdb_printf (file, "The remote exec-file is \"%s\".\n", filename.c_str ());
>  }

OK.

>  static int
> @@ -15475,8 +15480,12 @@ Transfer files to and from the remote target system."),
>  
>    add_setshow_string_noescape_cmd ("exec-file", class_files,
>  				   _("\
> -Set the remote pathname for \"run\"."), _("\
> -Show the remote pathname for \"run\"."), NULL,
> +Set the remote pathname for starting inferiors."), _("\
> +Show the remote pathname for starting inferiors."), _("\
> +This is the path, on the remote target, used when starting an inferior,\n\
> +for example with the \"run\", \"start\", or \"starti\" commands.\n\
> +This setting is only useful when debugging a remote target, otherwise,\n\
> +this setting is not used."),
>  				   set_remote_exec_file_cb,
>  				   get_remote_exec_file_cb,
>  				   show_remote_exec_file,

OK.

> diff --git a/gdb/testsuite/gdb.base/remote-exec-file.exp b/gdb/testsuite/gdb.base/remote-exec-file.exp
> index 0b198630a07..1411f9636be 100644
> --- a/gdb/testsuite/gdb.base/remote-exec-file.exp
> +++ b/gdb/testsuite/gdb.base/remote-exec-file.exp
> @@ -37,10 +37,13 @@ with_test_prefix "set inf 2" {
>  
>  with_test_prefix "show inf 1" {
>      gdb_test "inferior 1" "Switching to inferior 1.*"
> -    gdb_test "show remote exec-file" "prog1"
> +    gdb_test "show remote exec-file" \
> +	"The remote exec-file is \"prog1\"\\."
> +
>  }
>  
>  with_test_prefix "show inf 2" {
>      gdb_test "inferior 2" "Switching to inferior 2.*"
> -    gdb_test "show remote exec-file" "prog2"
> +    gdb_test "show remote exec-file" \
> +	"The remote exec-file is \"prog2\"\\."
>  }

OK. New expected ouput.

> diff --git a/gdb/testsuite/gdb.multi/gdb-settings.exp b/gdb/testsuite/gdb.multi/gdb-settings.exp
> index e5922221d47..2432192ca9a 100644
> --- a/gdb/testsuite/gdb.multi/gdb-settings.exp
> +++ b/gdb/testsuite/gdb.multi/gdb-settings.exp
> @@ -90,7 +90,7 @@ foreach_with_prefix inf $inferiors {
>      gdb_test "show inferior-tty" "/inf${inf}-tty.*"
>  
>      gdb_test "with remote exec-file tmp-value -- print 1" " = 1"
> -    gdb_test "show remote exec-file" "/inf${inf}-remote-exec"
> +    gdb_test "show remote exec-file" "/inf${inf}-remote-exec.*"
>  
>      # If the inferiors are running check $_gdb_setting_str and
>      # $_gdb_setting return the correct values.

OK. the .* is necessary because there is extra text to match now.