[libvirt] [PATCH] virsh: Remove redundant optional option for cmdHelp
Remove the optional option "group", as cmdHelp should accepts only one option, and rename option "command" as "command-or-group", ("virsh help" supports both command and command group now, and user nealy uses the options, so it doesn't matter much for it being longer, :-) * tools/virsh.c --- tools/virsh.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 31de80f..c2d717f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -569,8 +569,7 @@ static const vshCmdInfo info_help[] = { }; static const vshCmdOptDef opts_help[] = { - {"command", VSH_OT_DATA, 0, N_("Prints global help or command specific help.")}, - {"group", VSH_OT_DATA, 0, N_("Prints global help or help for a group of related commands.")}, + {"command-or-group", VSH_OT_DATA, 0, N_("Prints global help, command specific help, or help for a group of related commands")}, {NULL, 0, 0, NULL} }; @@ -581,10 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) const vshCmdGrp *g; const char *name; - name = vshCommandOptString(cmd, "command", NULL); - - if (!name) - name = vshCommandOptString(cmd, "group", NULL); + name = vshCommandOptString(cmd, "command-or-group", NULL); if (!name) { const vshCmdGrp *grp; -- 1.7.3.2
On 03/12/2010, at 6:34 PM, Osier Yang wrote:
static const vshCmdOptDef opts_help[] = { - {"command", VSH_OT_DATA, 0, N_("Prints global help or command specific help.")}, - {"group", VSH_OT_DATA, 0, N_("Prints global help or help for a group of related commands.")}, + {"command-or-group", VSH_OT_DATA, 0, N_("Prints global help, command specific help, or help for a group of related commands")}, {NULL, 0, 0, NULL} };
@@ -581,10 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) const vshCmdGrp *g; const char *name;
- name = vshCommandOptString(cmd, "command", NULL); - - if (!name) - name = vshCommandOptString(cmd, "group", NULL); + name = vshCommandOptString(cmd, "command-or-group", NULL);
NACK. This breaks backwards compatibility for anyone with scripts already using "--command": virsh # help --command domname error: command 'help' doesn't support option --command With 0.8.6: virsh # help --command domname NAME domname - convert a domain id or UUID to domain name SYNOPSIS domname <domain> OPTIONS [--domain] <string> domain id or uuid Thinking it would be better to show/have "--command" in the allowable options, but never care if it is or isn't specified. Just make use of the option or group name the user gives. ?
- name = vshCommandOptString(cmd, "command", NULL); - - if (!name) - name = vshCommandOptString(cmd, "group", NULL); + name = vshCommandOptString(cmd, "command-or-group", NULL);
NACK. This breaks backwards compatibility for anyone with scripts already using "--command":
Don't think anyone really used "--command", our test programs even doesn't use it. So then it's reasonable to be consistent, as virsh supports both command and command group now, and actually I save you pushed the doc patch. Regards - Osier
On 03/12/2010, at 8:33 PM, Osier Yang wrote:
Don't think anyone really used "--command", our test programs even doesn't use it.
Yeah. The problem is that we don't know, even if it's unlikely. I'm thinking removing the option, when it's not important that we do, is probably the safer idea.
So then it's reasonable to be consistent, as virsh supports both command and command group now, and actually I save you pushed the doc patch.
Sure. And it's easy to update again as well. ;)
On 12/03/2010 02:17 AM, Justin Clift wrote:
On 03/12/2010, at 6:34 PM, Osier Yang wrote:
static const vshCmdOptDef opts_help[] = { - {"command", VSH_OT_DATA, 0, N_("Prints global help or command specific help.")}, - {"group", VSH_OT_DATA, 0, N_("Prints global help or help for a group of related commands.")}, + {"command-or-group", VSH_OT_DATA, 0, N_("Prints global help, command specific help, or help for a group of related commands")}, {NULL, 0, 0, NULL} };
@@ -581,10 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) const vshCmdGrp *g; const char *name;
- name = vshCommandOptString(cmd, "command", NULL); - - if (!name) - name = vshCommandOptString(cmd, "group", NULL); + name = vshCommandOptString(cmd, "command-or-group", NULL);
NACK. This breaks backwards compatibility for anyone with scripts already using "--command":
Two points: 1. it is very unlikely that any scripts were already using --command, since 'virsh help --command xyz' has always been equivalent to 'virsh help xyz', and few people type a longer command when a short will do. 2. if we install unambiguous prefix parsing, then --command is an unambiguous prefix of --command-or-group. For now, since there has been no release with --group, I think the better course of action is to just keep the name --command. Only when we introduce abbreviation support can we lengthen it to --command-or-group. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2010年12月03日 23:15, Eric Blake 写道:
On 12/03/2010 02:17 AM, Justin Clift wrote:
On 03/12/2010, at 6:34 PM, Osier Yang wrote:
static const vshCmdOptDef opts_help[] = { - {"command", VSH_OT_DATA, 0, N_("Prints global help or command specific help.")}, - {"group", VSH_OT_DATA, 0, N_("Prints global help or help for a group of related commands.")}, + {"command-or-group", VSH_OT_DATA, 0, N_("Prints global help, command specific help, or help for a group of related commands")}, {NULL, 0, 0, NULL} };
@@ -581,10 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) const vshCmdGrp *g; const char *name;
- name = vshCommandOptString(cmd, "command", NULL); - - if (!name) - name = vshCommandOptString(cmd, "group", NULL); + name = vshCommandOptString(cmd, "command-or-group", NULL);
NACK. This breaks backwards compatibility for anyone with scripts already using "--command":
Two points: 1. it is very unlikely that any scripts were already using --command, since 'virsh help --command xyz' has always been equivalent to 'virsh help xyz', and few people type a longer command when a short will do. 2. if we install unambiguous prefix parsing, then --command is an unambiguous prefix of --command-or-group.
We have plan to add "prefix parsing" support? That's good idea.. - Osier
On 12/03/2010 12:34 AM, Osier Yang wrote:
Remove the optional option "group", as cmdHelp should accepts only one option, and rename option "command" as "command-or-group", ("virsh help" supports both command and command group now, and user nealy uses the options, so it doesn't matter much for it being longer, :-)
* tools/virsh.c --- tools/virsh.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
Given the ensuing discussion, I'm pushing with this squashed in. diff --git i/tools/virsh.c w/tools/virsh.c index c2d717f..3e1bde1 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -569,7 +569,7 @@ static const vshCmdInfo info_help[] = { }; static const vshCmdOptDef opts_help[] = { - {"command-or-group", VSH_OT_DATA, 0, N_("Prints global help, command specific help, or help for a group of related commands")}, + {"command", VSH_OT_DATA, 0, N_("Prints global help, command specific help, or help for a group of related commands")}, {NULL, 0, 0, NULL} }; @@ -580,7 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) const vshCmdGrp *g; const char *name; - name = vshCommandOptString(cmd, "command-or-group", NULL); + name = vshCommandOptString(cmd, "command", NULL); if (!name) { const vshCmdGrp *grp; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 04/12/2010, at 2:24 AM, Eric Blake wrote:
On 12/03/2010 12:34 AM, Osier Yang wrote:
Remove the optional option "group", as cmdHelp should accepts only one option, and rename option "command" as "command-or-group", ("virsh help" supports both command and command group now, and user nealy uses the options, so it doesn't matter much for it being longer, :-)
* tools/virsh.c --- tools/virsh.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
Given the ensuing discussion, I'm pushing with this squashed in.
Thanks Eric, that works well. I'll push a matching tweak to the man page in a minute too (under the trivial rule).
On 04/12/2010, at 4:12 AM, Justin Clift wrote:
On 04/12/2010, at 2:24 AM, Eric Blake wrote:
On 12/03/2010 12:34 AM, Osier Yang wrote:
Remove the optional option "group", as cmdHelp should accepts only one option, and rename option "command" as "command-or-group", ("virsh help" supports both command and command group now, and user nealy uses the options, so it doesn't matter much for it being longer, :-)
* tools/virsh.c --- tools/virsh.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
Given the ensuing discussion, I'm pushing with this squashed in.
Thanks Eric, that works well. I'll push a matching tweak to the man page in a minute too (under the trivial rule).
Actually, in this instance it's probably better if I submit it for ACKing rather than just push it. It should hit the list in a sec.
Updated to reflect that the optional --command parameter can be given, as well as either a command name or group keyword. --- tools/virsh.pod | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 9cb6829..a8f2912 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -115,7 +115,7 @@ The following commands are generic i.e. not specific to a domain. =over 4 -=item B<help> optional I<command-or-group> +=item B<help> optional I<--command> I<command-or-group-keyword> This lists each of the virsh commands. When used without options, all commands are listed, one per line, grouped into related categories, -- 1.7.3.2
On 12/03/2010 10:23 AM, Justin Clift wrote:
Updated to reflect that the optional --command parameter can be given, as well as either a command name or group keyword. --- tools/virsh.pod | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 9cb6829..a8f2912 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -115,7 +115,7 @@ The following commands are generic i.e. not specific to a domain.
=over 4
-=item B<help> optional I<command-or-group> +=item B<help> optional I<--command> I<command-or-group-keyword>
NACK. We already have the practice of omitting the I<--command> argument for a command where the string is recognized purely by position (where the --option is optional). For example, look at 'virsh suspend', where <domain> is required (that is, where 'suspend --domain xyz' is equivalent to 'suspend xyz'): =item B<suspend> I<domain-id> Note that the virsh.pod is spelled domain-id, even though the optional option is only spelled --domain, so we already have precedence for using a longer, more-descriptive string in the man page than what you get for 'virsh help suspend'. I don't think we need to make any further tweaks to virsh.pod for this issue. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2010年12月03日 23:24, Eric Blake 写道:
On 12/03/2010 12:34 AM, Osier Yang wrote:
Remove the optional option "group", as cmdHelp should accepts only one option, and rename option "command" as "command-or-group", ("virsh help" supports both command and command group now, and user nealy uses the options, so it doesn't matter much for it being longer, :-)
* tools/virsh.c --- tools/virsh.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
Given the ensuing discussion, I'm pushing with this squashed in.
Thanks Eric - Osier
participants (3)
-
Eric Blake -
Justin Clift -
Osier Yang