Re: [libvirt] [PATCHv3 2/6] virsh: Add vshCmdCompleter and vshOptCompleter

[re-adding the list, which was accidentally omitted] On 08/28/2013 05:26 AM, Tomas Meszaros wrote:
Per-option completions make sense. For example, 'virsh vol-key --pool <TAB>' wants to use a pool completer, while 'virsh vol-key --vol <TAB>' wants to use a volume completer; furthermore, 'virsh vol-key <TAB>' should be the combination of the option completer (showing --vol and --pool) AND the volume completer filtered to names not starting with '-' (since virsh has the semantics that arguments are positional, where the option '--vol' is implied if the argument that appears in that position does not resemble an option).
So If I get it right, you are suggesting that it should work like this:
virsh # vol-key <TAB> vol1 vol2 pool1 pool2
as you said, combination of --vol and --pool completers.
No, it should work like this: virsh# vol-key <TAB> vol1 vol2 --vol --pool the combination of all (non-option) completions for the current available mandatory option (volume completer), and of all possible options that still make sense at this point in the command line. Likewise: virsh# vol-key vol <TAB> pool1 pool2 --pool virsh# vol-key -<TAB> --vol --pool virsh# vol-key v<TAB> vol1 vol2 virsh# vol-key --pool <TAB> pool1 pool2 virsh# vol-key --pool pool1 <TAB> vol1 vol2 --vol and so forth.
I was initially thinking that completion should work like this:
virsh # vol-key <TAB> vol1 vol2
It is completing <vol> first becasue <vol> is only mandatory argument for this command.
It is a mandatory option, but mandatory options need not come first. Remember, our option parser allows mandatory options to occur positionally without an option name, OR to be interleaved in any other order by including the option string.
Only if someone type:
virsh # vol-key --pool <TAB> pool1 pool2
then call --pool completer.
This is correct - once an option is awaiting an argument, then the option completer must return nothing at that point in time. But if you look at it as the union of two completers - the set of options that are still valid in the current context, and the set of strings that are valid assuming positional semantics of the current option, you will always get the right answer, without needing a per-command completer (just per-option completers). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 28/08/13 at 06:00am, Eric Blake wrote:
[re-adding the list, which was accidentally omitted]
On 08/28/2013 05:26 AM, Tomas Meszaros wrote:
Per-option completions make sense. For example, 'virsh vol-key --pool <TAB>' wants to use a pool completer, while 'virsh vol-key --vol <TAB>' wants to use a volume completer; furthermore, 'virsh vol-key <TAB>' should be the combination of the option completer (showing --vol and --pool) AND the volume completer filtered to names not starting with '-' (since virsh has the semantics that arguments are positional, where the option '--vol' is implied if the argument that appears in that position does not resemble an option).
So If I get it right, you are suggesting that it should work like this:
virsh # vol-key <TAB> vol1 vol2 pool1 pool2
as you said, combination of --vol and --pool completers.
No, it should work like this:
virsh# vol-key <TAB> vol1 vol2 --vol --pool
the combination of all (non-option) completions for the current available mandatory option (volume completer), and of all possible options that still make sense at this point in the command line.
Likewise:
virsh# vol-key vol <TAB> pool1 pool2 --pool
virsh# vol-key -<TAB> --vol --pool
virsh# vol-key v<TAB> vol1 vol2
virsh# vol-key --pool <TAB> pool1 pool2
virsh# vol-key --pool pool1 <TAB> vol1 vol2 --vol
and so forth.
I was initially thinking that completion should work like this:
virsh # vol-key <TAB> vol1 vol2
It is completing <vol> first becasue <vol> is only mandatory argument for this command.
It is a mandatory option, but mandatory options need not come first. Remember, our option parser allows mandatory options to occur positionally without an option name, OR to be interleaved in any other order by including the option string.
Only if someone type:
virsh # vol-key --pool <TAB> pool1 pool2
then call --pool completer.
This is correct - once an option is awaiting an argument, then the option completer must return nothing at that point in time. But if you look at it as the union of two completers - the set of options that are still valid in the current context, and the set of strings that are valid assuming positional semantics of the current option, you will always get the right answer, without needing a per-command completer (just per-option completers).
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Yeah, It makes more sense. I'm now rewriting the whole stuff to just use opt completers. -- Tomas Meszaros

On 09/01/2013 01:22 PM, Tomas Meszaros wrote:
No, it should work like this:
virsh# vol-key <TAB> vol1 vol2 --vol --pool
virsh# vol-key --pool <TAB> pool1 pool2
Another twist to remember - options can be spelled with '=' in one argument, instead of two arguments. This means you have a choice in display between: virsh# vol-key --pool=<TAB> --pool=pool1 --pool=pool2 or virsh# vol-key --pool=<TAB> pool1 pool2 when there is an ambiguity, but the resolution of the tab completion must be the full option=value (that is, don't lose the --pool= prefix by expanding to just the value).
Yeah, It makes more sense. I'm now rewriting the whole stuff to just use opt completers.
Looking forward to another round of review - this is interesting code from the UI point of view. And I am still working on the './configure --disable-readline' patch from my side. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Tomas Meszaros