On 03/12/2010, at 4:47 AM, Eric Blake wrote:
On the other hand, I'm thinking we implemented the help command
slightly
wrong by specifying that it takes two optional strings. Really, it only
takes one optional string, which is a command-or-group. For instance,
with the current code, 'virsh help --group help' lists a command help,
rather than a group help, and 'virsh help --command virsh' lists the
group help, rather than a command help. Meanwhile, 'virsh help help
virsh' is accepted by the parser, but silently ignores the virsh group
argument.
Yeah, the current approach is a bit wrong. It can take either the
string "--command" or "--group", both of which do exactly the same
thing
rather than making a distinction.
So I'm thinking we need yet another patch to virsh.c that
reduces
opts_help to just one VSH_OT_DATA flag name (whether we keep it named
--command, or rename it to --command-or-group, is another question,
which is also impacted by whether we decide to implement unambiguous
prefix parsing like getopt_long).
Good point. With the naming of the new option, I'm not sure how much stock
we should put in maintaining backwards compatibility in this instance.
With a new patch we could probably drop the "--group" keyword, plus make
the "--command" keyword a null operation (ie ignore it). Then, if a command
or group has been given on the line, display that as is presently being done.
In the meantime, how about we list
this line as:
=item B<help> optional I<command-or-group>
ACK with that one-line change; the rest of the patch is uncontroversial,
and the virsh.c cleanup can be a separate patch.
That's better wording, thanks Eric. I'll make that tweak and push it.