3:31 a.m.
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(a)turnovfree.net
jabber: stybla(a)jabber.turnovfree.net
3:44 a.m.
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
3:47 a.m.
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(a)turnovfree.net
jabber: stybla(a)jabber.turnovfree.net
12:05 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:05 p.m.
-----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(a)turnovfree.net
jabber: stybla(a)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-----
3:29 a.m.
-----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(a)turnovfree.net
jabber: stybla(a)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-----
12:26 p.m.
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(a)turnovfree.net>
Date: Sun, 6 Feb 2011 08:50:43 +0100
Subject: [PATCH] Unify virsh help synopsis output
Signed-off-by: Zdenek Styblik <stybla(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:01 a.m.
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(a)turnovfree.net>
> Date: Sun, 6 Feb 2011 08:50:43 +0100
> Subject: [PATCH] Unify virsh help synopsis output
> Signed-off-by: Zdenek Styblik<stybla(a)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(a)turnovfree.net
jabber: stybla(a)jabber.turnovfree.net
5:28 a.m.
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(a)turnovfree.net
jabber: stybla(a)jabber.turnovfree.net
5033
days inactive
5041
days old
8 comments
3 participants
participants (3)
-
Eric Blake -
Justin Clift -
Zdenek Styblik