Re: [PATCH v2]virsh: track alias option and improve error message when option duplicates its alias

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/30/2013 06:18 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> 
> commit 2b172a8effa712aee97a21a64d2d02060958f9b2 allow
> alias to expand to opt=value pair.
> That means alias may not look alike since then.
> 
> With this patch we will also track alias.
> If we type command with one option and another marked
> as its alias, we will get an error message like:
> error: option '--AA' duplicates its alias '--AAA'

In addition to Peter's review, it helps if you make your commit message
smarter by showing before-and-after scenarios, to make it obvious how
the error message improved.  My quick testing of the 4 possible 'before'
scenarios:

$ args="-c test:///default attach-disk test /dev/null vda"
$ tools/virsh $args --shareable --shareable
error: option --mode already seen
$ tools/virsh $args --shareable --mode=shareable
error: option --mode already seen
$ tools/virsh $args --mode=shareable --mode=shareable
error: option --mode already seen
$ tools/virsh $args --mode=shareable --shareable
error: option --mode already seen

The existing message of "--mode already seen" is absolutely confusing
for scenario 1, correct for scenario 3, and sort of confusing for
scenarios 2 and 4.  Your proposed message of "'--shareable' duplicates
its alias '--mode'" would be wrong for scenarios 1 and 3, but works for
scenarios 2 and 4.  Are you improving the message for one scenario at
the expense of a regression in quality of other scenarios?

Furthermore, since aliases are (mostly) undocumented, the ONLY people
that should be using them are historical scripts that already had a
working command line, and not new users trying to experiment with the
various options; and working command lines don't have a problem with
duplicate options.  I think you're trying to address a non-issue.  Since
neither the '--help' text nor tab completion should be offering an alias
as a preferred spelling in the first place, it's unlikely that a human
user is going to be attempting a collision between its alias and the
canonical spelling.

I'm inclined to just say leave good enough alone, without trying to make
any further "improvements", as we have reached the point of diminishing
returns.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]