Re: [RFA v2/code+DOCO+NEWS] new "set/show serial baud" command

[Pedro Alves <palves@redhat.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