[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] man pages: update the description for the virsh help command

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.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]