[libvirt] consistency of virsh help output
Hello, I've found virsh help output to be inconsistent. ~~~ SNIP ~~~ attach-disk <domain> <source> <target> [--driver <string>]... X attach-interface <domain> <type> <source> [<target>] ... ~~~ SNIP ~~~ where <KEYWORD> is usually explained in OPTIONS as: ~~~ SNIP ~~~ [--domain] <string> domain name, id or uuid ~~~ SNIP ~~~ Substitution of <domain> -> [--domain] <string> resp. that you can only provide a <string> without [--domain] and it's still going to work. This took me some time to figure out. But back on point. As you can see from snips, once you use eg. <domain> and substitution resp. explanation is in OPTIONS and second you put "explanation" right into SYNOPSIS. I think it would be worth of cleaning out and make everything one way or another. Would it be a problem? I presume it's just matter of modifying "couple" printf()-s. Have a nice day, Z. -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla@turnovfree.net jabber: stybla@jabber.turnovfree.net
On 03/02/2011, at 8:31 PM, Zdenek Styblik wrote:
Hello,
I've found virsh help output to be inconsistent.
~~~ SNIP ~~~ attach-disk <domain> <source> <target> [--driver <string>]...
X
attach-interface <domain> <type> <source> [<target>] ... ~~~ SNIP ~~~
where <KEYWORD> is usually explained in OPTIONS as: ~~~ SNIP ~~~ [--domain] <string> domain name, id or uuid ~~~ SNIP ~~~
Substitution of <domain> -> [--domain] <string> resp. that you can only provide a <string> without [--domain] and it's still going to work. This took me some time to figure out. But back on point.
As you can see from snips, once you use eg. <domain> and substitution resp. explanation is in OPTIONS and second you put "explanation" right into SYNOPSIS.
I think it would be worth of cleaning out and make everything one way or another. Would it be a problem? I presume it's just matter of modifying "couple" printf()-s.
Yeah, consistency would definitely be good, and you're right, it probably would be just a matter of modifying printf() types of statements. You're definitely welcome to create patches to accomplish this if you want. :) + Justin
On 02/03/2011 10:44 AM, Justin Clift wrote:
Yeah, consistency would definitely be good, and you're right, it probably would be just a matter of modifying printf() types of statements.
You're definitely welcome to create patches to accomplish this if you want. :)
+ Justin
Good. Thanks, Z. -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla@turnovfree.net jabber: stybla@jabber.turnovfree.net
On 02/03/2011 02:31 AM, Zdenek Styblik wrote:
Hello,
I've found virsh help output to be inconsistent.
~~~ SNIP ~~~ attach-disk <domain> <source> <target> [--driver <string>]...
X
attach-interface <domain> <type> <source> [<target>] ... ~~~ SNIP ~~~
where <KEYWORD> is usually explained in OPTIONS as: ~~~ SNIP ~~~ [--domain] <string> domain name, id or uuid ~~~ SNIP ~~~
While I agree that a patch to make things more consistent is probably worthwhile, I was unclear from your report on which way you thought things should be changed. In this case, patches speak louder than words :) Did you have in mind changing the synopsis to match the options? attach-disk [--domain] <string> [--source] <string> [--target] <string> [--driver <string>] or were you thinking of making the synopsis even more compact? attach-disk <domain> <source> <target> [<driver>] or are you thinking of changing the layout in options? Remember, the goal is to convey which parameters are required (but which can still be specified out of order by using the optional --flag prefix), vs. which parameters are optional (and therefore require a --flag prefix to make it obvious which optional parameter is being provided), while also being clear on which optional parameters require an argument (and whether that argument is text or numeric) vs. those that are just boolean flags.
I think it would be worth of cleaning out and make everything one way or another. Would it be a problem? I presume it's just matter of modifying "couple" printf()-s.
That's true - the option printing is all controlled in tools/virsh.c:vshCmddefHelp. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/03/11 19:05, Eric Blake wrote: [...]
While I agree that a patch to make things more consistent is probably worthwhile, I was unclear from your report on which way you thought things should be changed. In this case, patches speak louder than words :)
I can't agree more.
Did you have in mind changing the synopsis to match the options?
attach-disk [--domain] <string> [--source] <string> [--target] <string> [--driver <string>]
or were you thinking of making the synopsis even more compact?
attach-disk <domain> <source> <target> [<driver>]
How about a vote? Because the first is domain of man pages, I've seen latter only in virsh(Note: "only", because I can't remember anything else. It doesn't mean virsh is unique). Since the most is in latter, short, then it would be better to go that way, or not? Once understood the latter, it's just fine and makes sense. Justin can confirm it took me some time. Thus I think it should be documented somewhere - perhaps in virsh-cmd-reference. Frankly, I'm a bit afraid if command help wouldn't be too long(80chars limit) and what would happen if it is. I hope this one makes sense.
or are you thinking of changing the layout in options? Remember, the goal is to convey which parameters are required (but which can still be specified out of order by using the optional --flag prefix), vs. which parameters are optional (and therefore require a --flag prefix to make it obvious which optional parameter is being provided), while also being clear on which optional parameters require an argument (and whether that argument is text or numeric) vs. those that are just boolean flags.
I've got a bit lost in all those ORs. Anyway, I don't think there is anything wrong with the options layout once it's understood. Or is it? I've tried to find comparable shell command and its man page, but I've failed. virsh seems to be a bit specific, to me, with its variability of parameters ... [--flag] <string> thing.
I think it would be worth of cleaning out and make everything one way or another. Would it be a problem? I presume it's just matter of modifying "couple" printf()-s.
That's true - the option printing is all controlled in tools/virsh.c:vshCmddefHelp.
Ok, thank you for the tip. Regards, Zdenek - -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla@turnovfree.net jabber: stybla@jabber.turnovfree.net -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk1K/GMACgkQ8MreUbSH7inNvgCeOj5vTq4NPahEiGzSSzQBZP0D QVwAoKmG4RveXyKtBTeXFeto9DNPgWHp =ijIh -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Heya, I've a bit of re-shuffled original e-mail and for that I'm sorry. On 02/03/11 19:05, Eric Blake wrote:
In this case, patches speak louder than words :)
Patch is attached.
While I agree that a patch to make things more consistent is probably worthwhile, I was unclear from your report on which way you thought things should be changed.
I didn't say we should take one way or another; neither I've said I'm going to. I merely wanted to point out at differences, which I think, create a bit of confusion. As suggested in my hasty reply, perhaps we should have a vote or poll. It doesn't seem to be easy decision to me.
attach-disk [--domain] <string> [--source] <string> [--target] <string> [--driver <string>]
This is domain of man pages and might be (the best) understood. But it's damn-o-damn long. And although we are way past 80 char terminals, I still tend to keep within this limit etc. :)
attach-disk <domain> <source> <target> [<driver>]
When understood, it's ok. And the majority of virsh commands have this format; except 19; I would stick with it. I have nothing against either of these format, although this short one requires '<domain> -> [--domain] <string>' substitution and a bit of thinking :) I wouldn't make boolean/switches shorter though -> keep eg. [--print-xml] in Synopsis as it is.
or are you thinking of changing the layout in options? Remember, the goal is to convey which parameters are required (but which can still be specified out of order by using the optional --flag prefix), vs. which parameters are optional (and therefore require a --flag prefix to make it obvious which optional parameter is being provided), while also being clear on which optional parameters require an argument (and whether that argument is text or numeric) vs. those that are just boolean flags.
I'm still a bit confused about this part. :| I don't think Options part needs changes, although one could split params(domain) and booleans/switches(print-xml) aside. Regards, Zdenek - -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla@turnovfree.net jabber: stybla@jabber.turnovfree.net -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk1Oag8ACgkQ8MreUbSH7ik1ZQCfWXHJCiVCmrwXOWyM6YNov8i/ gcEAnR9QycO0RuUCiKQyjzh6pv31Gces =7SO+ -----END PGP SIGNATURE-----
On 02/06/2011 02:29 AM, Zdenek Styblik wrote:
I didn't say we should take one way or another; neither I've said I'm going to. I merely wanted to point out at differences, which I think, create a bit of confusion.
As suggested in my hasty reply, perhaps we should have a vote or poll. It doesn't seem to be easy decision to me.
Well, I'll start by casting my vote :)
attach-disk [--domain] <string> [--source] <string> [--target] <string> [--driver <string>]
This is domain of man pages and might be (the best) understood. But it's damn-o-damn long. And although we are way past 80 char terminals, I still tend to keep within this limit etc. :)
attach-disk <domain> <source> <target> [<driver>]
When understood, it's ok. And the majority of virsh commands have this format; except 19; I would stick with it.
I also like the shorter form.
0001-Unify-virsh-help-synopsis-output.patch>From 2ce39caeb82a9a0b2dccc409ac986648c4017191 Mon Sep 17 00:00:00 2001 From: Zdenek Styblik <stybla@turnovfree.net> Date: Sun, 6 Feb 2011 08:50:43 +0100 Subject: [PATCH] Unify virsh help synopsis output Signed-off-by: Zdenek Styblik <stybla@turnovfree.net>
--- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 1f820e8..cc8efcf 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10596,11 +10596,11 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) case VSH_OT_INT: /* xgettext:c-format */ fmt = ((opt->flag & VSH_OFLAG_REQ) ? "<%s>" - : _("[--%s <number>]")); + : _("[%s]")); break; case VSH_OT_STRING: /* xgettext:c-format */ - fmt = _("[--%s <string>]"); + fmt = _("[%s]"); break;
Actually, I think these would be _("[<%s>]") (note the <> to signify the meta-syntactic term). But I like the idea; this shortens the synopsis form, while still leaving the usage form to explain the meta-syntactic term. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 02/09/2011 07:26 PM, Eric Blake wrote: [...]
attach-disk<domain> <source> <target> [<driver>]
When understood, it's ok. And the majority of virsh commands have this format; except 19; I would stick with it.
I also like the shorter form.
Fine :o) I don't mind either of those.
0001-Unify-virsh-help-synopsis-output.patch> From 2ce39caeb82a9a0b2dccc409ac986648c4017191 Mon Sep 17 00:00:00 2001 From: Zdenek Styblik<stybla@turnovfree.net> Date: Sun, 6 Feb 2011 08:50:43 +0100 Subject: [PATCH] Unify virsh help synopsis output Signed-off-by: Zdenek Styblik<stybla@turnovfree.net>
--- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 1f820e8..cc8efcf 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10596,11 +10596,11 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) case VSH_OT_INT: /* xgettext:c-format */ fmt = ((opt->flag& VSH_OFLAG_REQ) ? "<%s>" - : _("[--%s<number>]")); + : _("[%s]")); break; case VSH_OT_STRING: /* xgettext:c-format */ - fmt = _("[--%s<string>]"); + fmt = _("[%s]"); break;
Actually, I think these would be _("[<%s>]") (note the<> to signify the meta-syntactic term). But I like the idea; this shortens the synopsis form, while still leaving the usage form to explain the meta-syntactic term.
Right, because '--%s' is mandatory. Should I re-post the patch, or...? Thanks, Z. -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla@turnovfree.net jabber: stybla@jabber.turnovfree.net
On 02/10/2011 08:01 AM, Zdenek Styblik wrote: [...]
Actually, I think these would be _("[<%s>]") (note the<> to signify the meta-syntactic term). But I like the idea; this shortens the synopsis form, while still leaving the usage form to explain the meta-syntactic term.
Right, because '--%s' is mandatory. Should I re-post the patch, or...?
Thanks, Z.
Should be fine. Z. -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla@turnovfree.net jabber: stybla@jabber.turnovfree.net
participants (3)
-
Eric Blake -
Justin Clift -
Zdenek Styblik