Message ID | 1558814323.1454.22.camel@skynet.be |
---|---|
State | New, archived |
Headers |
Received: (qmail 118780 invoked by alias); 25 May 2019 19:58:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 118772 invoked by uid 89); 25 May 2019 19:58:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.1 spammy=apropos, HContent-Transfer-Encoding:8bit X-HELO: mailsec117.isp.belgacom.be Received: from mailsec117.isp.belgacom.be (HELO mailsec117.isp.belgacom.be) (195.238.20.113) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 25 May 2019 19:58:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1558814326; x=1590350326; h=message-id:subject:from:to:date:in-reply-to:references: mime-version:content-transfer-encoding; bh=+m3DyVhWOlOKdpfvV7jI+muEKWZS0f23Sygsdkchyv4=; b=B9G+OrkqnflZoKide1YmGsxFt4dPhmRDyBdVatosy277VfzGf4KSKYQb rHlOwKRcH5lM6nGDOg3rK5gctiIDqA==; Received: from 161.32-242-81.adsl-dyn.isp.belgacom.be (HELO md) ([81.242.32.161]) by relay.skynet.be with ESMTP/TLS/AES256-GCM-SHA384; 25 May 2019 21:58:43 +0200 Message-ID: <1558814323.1454.22.camel@skynet.be> Subject: Re: [PATCH 15/24] Introduce rename_cmd From: Philippe Waroquiers <philippe.waroquiers@skynet.be> To: Pedro Alves <palves@redhat.com>, gdb-patches@sourceware.org Date: Sat, 25 May 2019 21:58:43 +0200 In-Reply-To: <20190522205327.2568-16-palves@redhat.com> References: <20190522205327.2568-1-palves@redhat.com> <20190522205327.2568-16-palves@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes |
Commit Message
Philippe Waroquiers
May 25, 2019, 7:58 p.m. UTC
On Wed, 2019-05-22 at 21:53 +0100, Pedro Alves wrote: > diff --git a/gdb/command.h b/gdb/command.h > index 35006cc339e..ed848114b85 100644 > --- a/gdb/command.h > +++ b/gdb/command.h > @@ -439,6 +439,11 @@ extern void > struct cmd_list_element **set_list, > struct cmd_list_element **show_list); > > +/* Move command OLD_NAME from OLD_LIST to NEW_LIST and rename it from > + OLD_NAME to NEW_NAME. */ > +extern void rename_cmd (const char *old_name, cmd_list_element **old_list, > + const char *new_name, cmd_list_element **new_list); > + I have some difficulties to understand the idea here. If the command is moved from one list to another, how is the renamed command still available via the 'normal' list ? Effectively, it looks like set/show print raw-frame-arguments commands are not known. Why not just keep the old command untouched, and define a new command via the new framework. Below is just a patch (based on HEAD) that defines 2 different set/show changing the same variable: (gdb) apropos raw form set print raw frame-arguments -- Set whether to print frame arguments in raw form set print raw-frame-arguments -- Set whether to print frame arguments in raw form show print raw frame-arguments -- Show whether to print frame arguments in raw form show print raw-frame-arguments -- Show whether to print frame arguments in raw form (gdb) set print raw frame-arguments on (gdb) show print raw frame-arguments Whether to print frame arguments in raw form is on. (gdb) show print raw-frame-arguments Whether to print frame arguments in raw form is on. (gdb) set print raw-frame-arguments off (gdb) show print raw frame-arguments Whether to print frame arguments in raw form is off. (gdb) show print raw-frame-arguments Whether to print frame arguments in raw form is off. (gdb) I guess this should still work if the same variable is changed via the new framework and/or via add_setshow_boolean_cmd. The old commands might be marked as obsolete. Philippe
Comments
On 5/25/19 8:58 PM, Philippe Waroquiers wrote: > On Wed, 2019-05-22 at 21:53 +0100, Pedro Alves wrote: > >> diff --git a/gdb/command.h b/gdb/command.h >> index 35006cc339e..ed848114b85 100644 >> --- a/gdb/command.h >> +++ b/gdb/command.h >> @@ -439,6 +439,11 @@ extern void >> struct cmd_list_element **set_list, >> struct cmd_list_element **show_list); >> >> +/* Move command OLD_NAME from OLD_LIST to NEW_LIST and rename it from >> + OLD_NAME to NEW_NAME. */ >> +extern void rename_cmd (const char *old_name, cmd_list_element **old_list, >> + const char *new_name, cmd_list_element **new_list); >> + > I have some difficulties to understand the idea here. > If the command is moved from one list to another, how is the renamed > command still available via the 'normal' list ? So the existing command is called "set print raw frame-arguments" I think "set print raw-frame-arguments" would have been a better command name, but since that command already exists with that spelling since gdb 7.7, and I didn't have a huge reason to change it, in the end I opted to leave it as it was. With that decision, the next problem is, how to implement both the option and the set command without duplicating the help bits. So what I did was, define the option like this: + 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.") + }, and then let: + gdb::option::add_setshow_cmds_for_options + (class_stack, &user_frame_print_options, + frame_print_option_defs, &setprintlist, &showprintlist); + create the "set print raw-frame-arguments" command from the option definition shown above. And now that's where the renaming takes place, with: + /* 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); 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. > > Effectively, it looks like set/show print raw-frame-arguments commands > are not known. Yes, that was the intention. > > Why not just keep the old command untouched, and define a new > command via the new framework. > It didn't seem worth it to me. > Below is just a patch (based on HEAD) that defines 2 different set/show > changing the same variable: > (gdb) apropos raw form > set print raw frame-arguments -- Set whether to print frame arguments in raw form > set print raw-frame-arguments -- Set whether to print frame arguments in raw form > show print raw frame-arguments -- Show whether to print frame arguments in raw form > show print raw-frame-arguments -- Show whether to print frame arguments in raw form > (gdb) set print raw frame-arguments on > (gdb) show print raw frame-arguments > Whether to print frame arguments in raw form is on. > (gdb) show print raw-frame-arguments > Whether to print frame arguments in raw form is on. > (gdb) set print raw-frame-arguments off > (gdb) show print raw frame-arguments > Whether to print frame arguments in raw form is off. > (gdb) show print raw-frame-arguments > Whether to print frame arguments in raw form is off. > (gdb) > > I guess this should still work if the same variable is changed via > the new framework and/or via add_setshow_boolean_cmd. TBC, the new framework does not change how the set commands work at all. The integration that exists is that the new gdb::option::add_setshow_cmds_for_options function goes over the option definitions and calls add_setshow_xxx_cmd for each option. The option_def structure has some fields that only exists for this, such as the show_doc, and the show_cmd_cb. That was all done as a way to conveniently describe the help bits for both the options and the commands in a single place. > The old commands might be marked as obsolete. Yes, if we add a "raw-frame-arguments", we should deprecate "raw frame-arguments" so that TAB completion doesn't suggest the latter, which would get in the way of "set print raw[TAB]". I actually tried doing that, and I don't recall the details, but it didn't seem that easy, because there's more than one command involved -- "raw" is a command, and then "frame-arguments" is a subcommand. I think it was at that point that I had decided to just leave it as it was. With the info above, any preference on how to proceed? Thanks, Pedro Alves
diff --git a/gdb/stack.c b/gdb/stack.c index 408c795e38..05b11e6ecb 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -3106,6 +3106,16 @@ Usage: func NAME")); Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _("Show printing of non-scalar frame arguments"), Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â NULL, NULL, NULL, &setprintlist, &showprintlist); Â + +Â Â add_setshow_boolean_cmd ("raw-frame-arguments", no_class, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &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, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &setprintlist, &showprintlist); + Â Â Â add_setshow_boolean_cmd ("frame-arguments", no_class, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &print_raw_frame_arguments, _("\ Â Set whether to print frame arguments in raw form."), _("\