On 10/25/2010 05:51 PM, Stefan Berger wrote:
case VSH_OT_INT: - snprintf(buf, sizeof(buf), _("--%s<number>"), opt->name); + snprintf(buf, sizeof(buf), + (opt->flag& VSH_OFLAG_REQ) ? _("[--%s]<number>") + : _("--%s<number>"), opt->name); Looks to me like the first string (VSH_OFLAG_REQ set) should not have the [--%s] and the second one should have it, no?
It was correct as-is. When you use VSH_OFLAG_REQ, then the option must appear, so the parser is generous and lets you omit the --option prefix. For example, with setvcpus, you can do: virsh setvcpus dom 1 instead of virsh setvcpus --domain dom --count 1 so the help text shows up as: setvcpus <domain> <count> [--maximum] [--config] [--live] ... [--domain] <string> domain name, id or uuid [--count] <number> number of virtual CPUs On the other hand, with memtune, the VSH_OFLAG_REQ is not set, so the parser cannot tell which option the argument appear with unless the option name is also present. Thus: memtune dom 1024 is an error, and you have to do: memtune dome --hard_limit 1024 (hmm; from a cli perspective, it's better to name options with - than _; I'll propose a followup patch to name it hard-limit). The help for memtune is also correct: memtune <domain> [--hard_limit <number>] [--soft_limit <number>] [--swap_hard_limit <number>] [--min_guarantee <number>] ... [--domain] <string> domain name, id or uuid --hard_limit <number> Max memory in kilobytes
break; case VSH_OT_STRING: + /* OT_STRING should never be VSH_OFLAG_REQ */ snprintf(buf, sizeof(buf), _("--%s<string>"), opt->name); break; case VSH_OT_DATA:
ACK.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org