[virt-tools-list] [virt-viewer][PATCH v2] Introduce bash completion

[Michal Privoznik]

On 5/7/19 8:32 PM, Jonathon Jongsma wrote:
> On Mon, 2019-05-06 at 13:27 +0200, Michal Privoznik wrote:
>> With this change one can get list of domains on the command line:
>>
>>    $ virt-viewer -c qemu:///system <TAB><TAB>
>>    dom1   dom2   ... domN
>>
>> The list of domains is fetched using virsh, hence the dependency
>> on libvirt-client recorded in the spec file. I think it's fair
>> to assume that Linux hosts with virt-viewer will have virsh
>> available too. If they don't, nothing breaks and no error message
>> is printed.
>>
>> The completer script is inspired by libvirt.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
> 
> I just sent in a little proposed fixup patch, but I also had a few
> findings from testing it briefly. It works well enough, though there
> are a few oddities. I'm not sure that they're worth fixing if they're
> difficult, but I'll mention them here anyway for discussion.
> 
> ====
> 
> If you've already specified a domain, tab completion tries to suggest
> additional domains to add to the command:
> 
> virt-viewer --id 1 --reconnect <tab><tab>
> 1   13  2


Yeah. To fix this we'd need some more clever command parsing which would 
understand what each --option does and if it takes arguments.

Your example is equivallent to:

virt-viewer --id --reconnect 1 <tab><tab>

because nor --id nor --domain-name nor --uuid really take any arguments. 
They are just boolean flags.

I think it's fair to say that completer doesn't guarantee overall 
command validity. It merely gives you suggestions and speeds up your 
workflow.
But if you can think of some clever simple trick, then I'm all ears.

> 
> ====
> 
> X display completion doesn't seem to work for me:
> 
> $ virt-viewer --display=<tab>
> 
> (appends a colon to the command):
> $ virt-viewer --display=:<tab>
> 
> (appends another colon):
> $ virt-viewer --display=::
> 
> Further <tab> presses do nothing.
> 

Ah, okay, so my assumption about display ID and X socket name was not 
correct. Can you please share listing of /tmp/.X11-unix/ ? Thanks.

> ====
> 
> Additionally, is it useful to specify --all to the `virsh list`
> command? How often is a user going to want to connect to a non-running
> domain?

Okay, I don't care that much. I can remove it and we can add it back if 
somebody asks for it.

> 
> In general it looks good though.
> 
> Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>

Thanks. I'll post v3 with your suggestions worked in (as soon as I 
figure out the --display= completer).

Michal