This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA v2/code+DOCO+NEWS] new "set/show serial baud" command
- From: Pedro Alves <palves at redhat dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 09 Oct 2013 17:17:33 +0100
- Subject: Re: [RFA v2/code+DOCO+NEWS] new "set/show serial baud" command
- Authentication-results: sourceware.org; auth=none
- References: <CAAyhtXRSWTdpDc4jiZ2k5zWCNUXUzRxNokN1F_aRdMfuWGsBHQ at mail dot gmail dot com> <87bo6affrh dot fsf at fleche dot redhat dot com> <524ECCBB dot 5050307 at gmail dot com> <CAAyhtXQBSOz8sjxSmqGRQNtXHeOJpszMEYhRGyBydBGr-GkZCw at mail dot gmail dot com> <20131008035636 dot GC3092 at adacore dot com> <5253F497 dot 6040709 at redhat dot com> <20131008141639 dot GF3092 at adacore dot com> <87siwbhk47 dot fsf at fleche dot redhat dot com> <20131009112334 dot GJ3092 at adacore dot com>
On 10/09/2013 12:23 PM, Joel Brobecker wrote:
> Unfortunately, I hit a limitation when trying to deprecate the alias,
> and my guess is because the command is prefixed. I had a quick look,
> and basically, I think "lookup_cmd_1" recurses into himself by first
> finding the prefixed command, then does the recursion by trying to
> find the rest of the command within that prefix. It finds the command
> and notices the fact that it is deprecated, and even tries to warn
> the user by calling deprecated_cmd_warning. But this function only
> takes the deprecated command names, and the name it passes is lacking
> the prefix (due to the recursion). As a result, deprecated_cmd_warning
> fails to find the aliased command and returns immediately:
>
> if (!lookup_cmd_composition (text, &alias, &prefix_cmd, &cmd))
> /* Return if text doesn't evaluate to a command. */
> return;
>
Bummer.
> Lifting that limitation would take a little bit more time than I have,
> so I went with the sort of trick as the one used in remote.c, which
> is to duplicate the command.
Oh well. That's fine with me.
> Not pretty, but as the FIXME says, it's
> temporary (I've already added a TODO item in the GDB 7.8 release
> management wiki page).
...
> + /* FIXME: There is a limitation in the deprecation mechanism,
> + and the warning ends up not being displayed for prefixed
> + aliases. So use a real command instead of an alias.
> + This is temporary code anyway, so should go away soon
> + after the next release cycle starts. */
I actually think that "temporary anyway" comment should be removed. We
don't really have a habit of removing deprecated commands. Marking a
command deprecated already makes it less visible. E.g., it removes the
command from completion suggestions. As long as the old command doesn't
get in the way, there's really no pressing need to remove it.
"soon after the next release" has itself a tendency to get
old and outdated :-)
> This patch implements that change, as well as updates the GDB Manual.
> For now, it's a straight search-and-replace, as there was no real
> section where the option could go. Eventually, I think we will want
> to dissociate the options relative to the remote protocol, from the
> options specific to serial line handling. I think other option names
> might need to be renamed as well, but this I've already far exceeded
> the amount of time I could spend on this.
>
> gdb/ChangeLog:
>
> * cli/cli-cmds.c (show_baud_rate): Moved to serial.c as
> serial_baud_show_cmd.
> (_initialize_cli_cmds): Delete the code creating the
> "set/show remotebaud" commands.
> * serial.c (baud_rate): Move here from top.c.
> (serial_baud_show_cmd): Move here from cli/cli-cmds.c.
> (_initialize_serial): Create "set/show serial baud" commands.
> Add "set/show remotebaud" command aliases.
> * top.c (baud_rate): Moved to serial.c.
> * NEWS: Document the new "set/show serial baud" commands,
> replacing "set/show remotebaud".
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo: Replace "set remotebaud" and "show remotebaud"
> by "set serial baud" and "show serial baud" (resp) throughout.
>
> I hope it's a sufficient improvement in itself. OK to apply?
This is fine with me.
--
Pedro Alves