Re: [libvirt] [PATCH 1/2] virsh-completer: Separate comma list construction into a function
On Wed, Jun 19, 2019 at 01:50:14PM +0200, Michal Privoznik wrote:
On 6/19/19 12:59 PM, Erik Skultety wrote:
> On Tue, Jun 18, 2019 at 03:46:15PM +0200, Michal Privoznik wrote:
> > There are more arguments than 'shutdown --mode' that accept a
> > list of strings separated by commas. 'nodedev-list --cap' is one
> > of them. To avoid duplicating code, let's separate interesting
> > bits of virshDomainShutdownModeCompleter() into a function that
> > can then be reused.
> >
> > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> > ---
> > tools/virsh-completer.c | 120 ++++++++++++++++++++++++++--------------
> > 1 file changed, 77 insertions(+), 43 deletions(-)
> >
> > diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
> > index 7d5cf8cb90..ef2f39320e 100644
> > --- a/tools/virsh-completer.c
> > +++ b/tools/virsh-completer.c
> > @@ -69,6 +69,79 @@
> > */
> >
> >
> > +/**
> > + * virshCommaStringListComplete:
> > + * @input: user input so far
> > + * @options: ALL options available for argument
> > + *
> > + * Some arguments to our commands accept the following form:
> > + *
> > + * virsh command --arg str1,str2,str3
> > + *
> > + * This does not play nicely with our completer funtions, because
> > + * they have to return strings prepended with user's input. For
> > + * instance:
> > + *
> > + * str1,str2,str3,strA
> > + * str1,str2,str3,strB
> > + * str1,str2,str3,strC
>
> ^This sounds rather sub-optimal. I wouldn't even insist on making the
> suggestions contextual like it is now, IOW not suggesting options which have
> already been specified and would rather return the same list of possible
> options than a string with the user input prepended.
So IIUC, for 'shutdown --mode <TAB><TAB>' you want to see:
"acpi", "agent", "initctl", "signal",
"paravirt"
and for 'shutdown --mode acpi,agent,<TAB><TAB>' you want to see the
same
list again (optionally with already specified strings removed)? Yep, that
would be great but I don't think that is how readline works. At least, I
don't know how to achieve that. Do you perhaps have an idea?
It very well may be the case that it doesn't work the way we'd like to and I
don't understand how it actually works, but why does readline even matter here?
Readline calls our completers which generate the output presented to the user,
so we should be in charge what is returned, so why do we need to prepend the
user input then? In fact, I found there's a function called vshCompleterFilter
which removes the whole output list if the items are not prepended with the
original user input, why is that? When I commented out the bit dropping items
from the list and stopped pre-pending the user input, I achieved what I
suggested in my original response to this series, a context-based list without
unnecessary prefixes. I also tried a few other random completions to see
whether I didn't break something by stripping some code from
vshCompleterFilter and it looks like it worked, so the question is, what was
the reason for that function in the first place, since I haven't experienced
the effects described by commit d4e63aff5d0 which introduced it?
Thanks,
Erik