[15/24] Introduce rename_cmd

  On 5/29/19 5:03 PM, Pedro Alves wrote:

> There are other ways to implement this.  I could move the
> "raw-frame-arguments" option_def out of the frame_print_option_defs
> array, so that it doesn't get installed as a command, and leave the
> "set print raw frame-arguments" add_setshow... call in place.
> I guess it wouldn't be a big deal.

So turns out the change would be pretty simple.

I'm undecided which version to use...  I suppose this version
is simpler and a little bit more efficient.  Preferences?

From fcbbc6a33276a8ec59425174f6868379d0a239a3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 29 May 2019 19:04:05 +0100
Subject: [PATCH] no-rename

---
 gdb/stack.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)
  

On Wed, 2019-05-29 at 19:30 +0100, Pedro Alves wrote:
> On 5/29/19 5:03 PM, Pedro Alves wrote:
> 
> > There are other ways to implement this.  I could move the
> > "raw-frame-arguments" option_def out of the frame_print_option_defs
> > array, so that it doesn't get installed as a command, and leave the
> > "set print raw frame-arguments" add_setshow... call in place.
> > I guess it wouldn't be a big deal.
> 
> So turns out the change would be pretty simple.
> 
> I'm undecided which version to use...  I suppose this version
> is simpler and a little bit more efficient.  Preferences?

As 'frame-arguments' is the only command that uses the prefix 'set print raw'
and that this prefix should not be used for further options, I would mark
'set print raw' obsolete, to avoid an inconsistency (at user level)
only for this option.
But not a big deal ...

> 
> From fcbbc6a33276a8ec59425174f6868379d0a239a3 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 29 May 2019 19:04:05 +0100
> Subject: [PATCH] no-rename
> 
> ---
>  gdb/stack.c | 44 ++++++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 3054c797c74..2c58b0d728b 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -126,16 +126,16 @@ or both.  Note that one or both of these values may be <optimized out>."),
>      N_("Show printing of non-scalar frame arguments"),
>      NULL /* help_doc */
>    },
> +};
>  
> -  boolean_option_def {
> -    "raw-frame-arguments",
> -    [] (frame_print_options *opt) { return &opt->print_raw_frame_arguments; },
> -    NULL, /* show_cmd_cb */
> -    N_("Set whether to print frame arguments in raw form."),
> -    N_("Show whether to print frame arguments in raw form."),
> -    N_("If set, frame arguments are printed in raw form, bypassing any\n\
> -pretty-printers for that value.")
> -  },
> +/* The "raw-frame-arguments" option.  Defined separately because the
> +   corresponding "set" command is called "set print raw
> +   frame-arguments", with space instead of dash.  */
> +static const boolean_option_def raw_frame_arguments_option_def {
> +  "raw-frame-arguments",
> +  [] (frame_print_options *opt) { return &opt->print_raw_frame_arguments; },
> +  NULL, /* show_cmd_cb */
> +  N_("Set whether to print frame arguments in raw form."),
>  };
>  
>  /* Options for the "backtrace" command.  */
> @@ -2015,13 +2015,14 @@ backtrace_command_1 (const frame_print_options &fp_opts,
>  /* Create an option_def_group array grouping all the "backtrace"
>     options, with FP_OPTS, BT_CMD_OPT, SET_BT_OPTS as contexts.  */
>  
> -static inline std::array<gdb::option::option_def_group, 3>
> +static inline std::array<gdb::option::option_def_group, 4>
>  make_backtrace_options_def_group (frame_print_options *fp_opts,
>  				  backtrace_cmd_options *bt_cmd_opts,
>  				  set_backtrace_options *set_bt_opts)
>  {
>    return {{
>      { {frame_print_option_defs}, fp_opts },
> +    { raw_frame_arguments_option_def.def (), fp_opts },
>      { {set_backtrace_option_defs}, set_bt_opts },
>      { {backtrace_command_option_defs}, bt_cmd_opts }
>    }};
> @@ -3288,14 +3289,17 @@ source line."),
>      (class_stack, &user_frame_print_options,
>       frame_print_option_defs, &setprintlist, &showprintlist);
>  
> -  /* The above installs a "set print raw-frame-arguments" command,
> -     because there's an option called "print -raw-frame-arguments".
> -     Rename the command to "set print raw frame-arguments" (space
> -     instead of dash), to keep backward compatibility -- the "raw
> -     frame-arguments" command already existed when print options were
> -     first added.  */
> -  rename_cmd ("raw-frame-arguments", &setprintlist,
> -	      "frame-arguments", &setprintrawlist);
> -  rename_cmd ("raw-frame-arguments", &showprintlist,
> -	      "frame-arguments", &showprintrawlist);
> +  /* This one is not defined by the add_setshow_cmds_for_options call
> +     above, because the option is called "raw-frame-arguments", while
> +     the command is called "set print raw frame-arguments", with space
> +     instead of dash.  */
> +  add_setshow_boolean_cmd ("frame-arguments", no_class,
> +			   &user_frame_print_options.print_raw_frame_arguments,
> +			   _("\
> +Set whether to print frame arguments in raw form."), _("\
> +Show whether to print frame arguments in raw form."), _("\
> +If set, frame arguments are printed in raw form, bypassing any\n\
> +pretty-printers for that value."),
> +			   NULL, NULL,
> +			   &setprintrawlist, &showprintrawlist);
>  }
  

On 5/30/19 11:22 AM, Philippe Waroquiers wrote:
> On Wed, 2019-05-29 at 19:30 +0100, Pedro Alves wrote:
>> On 5/29/19 5:03 PM, Pedro Alves wrote:
>>
>>> There are other ways to implement this.  I could move the
>>> "raw-frame-arguments" option_def out of the frame_print_option_defs
>>> array, so that it doesn't get installed as a command, and leave the
>>> "set print raw frame-arguments" add_setshow... call in place.
>>> I guess it wouldn't be a big deal.
>>
>> So turns out the change would be pretty simple.
>>
>> I'm undecided which version to use...  I suppose this version
>> is simpler and a little bit more efficient.  Preferences?
> 
> As 'frame-arguments' is the only command that uses the prefix 'set print raw'
> and that this prefix should not be used for further options, I would mark
> 'set print raw' obsolete, to avoid an inconsistency (at user level)
> only for this option.
> But not a big deal ...

Alright, you've convinced me.  And it turned out to be quite simple.
I don't remember exactly what problems I had run into the first time...

I've now posted a v2 of the series, 
https://sourceware.org/ml/gdb-patches/2019-05/msg00712.html

Of course I forgot to mention that I pushed it to
users/palves/cli-options-v2...  

I've kept the v1 branch around for now, if someone wants to
compare, renamed to users/palves/cli-options-v1.

Thanks,
Pedro Alves