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

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

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.
  

On Wed, Aug 16, 2023 at 5:56 PM Andrew Burgess via Gdb-patches <
gdb-patches@sourceware.org> 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.
>
>
I really like the change in behaviour. Showing the blank line for the
default remote exec-file is confusing. The more verbose help is also much
more informative. I think more verbose is almost always better.

I can confirm this change causes no regressions on ppc64le Fedora-Rawhide.