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. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list