feat(completion): support no space format of short flags

[fiftin]

Cobra supports a "no space" format for short flags but doesn't support auto complete for this case.

Related to issue #1629

[CLAassistant]

All committers have signed the CLA.

[marckhouzam]

Supet exciting @fiftin ! I didn't expect it to be that simple 😁. I'll give it a try ASAP 🚀

You will have to sign the CLA before we can accept the contribution however.

[fiftin]

@marckhouzam ,

How to do this?

UPDATE: Done.

[marckhouzam]

@fiftin I'm having trouble understanding what this change is supposed to provide.
Could you provide an example output of before and after the change please?

[fiftin]

-f123[TAB]

Output

Before: Empty

After:

-f flag description

[marckhouzam]

@fiftin Sorry but I'm still having trouble understanding the behaviour you seek.

If a program has a short flag -f like your example, and the user does:
prog -f123<TAB> why would the completion show -f? And what is the 123 string; is it a value for -f or are they other short flags?

[fiftin]

@marckhouzam,

I orientated to issue #1629. Your example in the issue:

kubectl -ninfra-pro<TAB>
# Nothing suggested

For my opinion it should works similar to kubectl -n<TAB>. How it should works for your opinion? Please provide more examples.

[marckhouzam]

@fiftin
So let's say the user currently does:

$ kubectl -n infra-pro<TAB>
# or
$ kubectl -n=infra-pro<TAB>

in both cases, the completion logic would suggest all namespaces that start with infra-pro, for example infra-production1 and infra-production2.

I believe the same behaviour should happen when there is no space after the short flag.

$ kubectl -ninfra-pro<TAB>
infra-production1      infra-production2

But there are special cases we would need to treat differently:

$ kubectl -n<TAB>
# Should probably complete to -n because it is a valid short flag

$ kubectl -itn<TAB>
# Should probably complete to -n because each letter 'i', 't', 'n' represent a valid short flag and the first two don't take an argument

$ kubectl -itni<TAB>
# Should probably complete to all namespaces starting with 'i' because the first letter 'i' and second letter 't' represent
# valid short flags not needing an argument, but the third letter 'n' represents a short flag that requires a namespace
# as an argument, so the forth letter 'i' no longer represents a flag but the prefix of a namespace name.

Does this make sense to you?

[fiftin]

Hi @marckhouzam,

Done. Please see test TestShorthandFlagCompletionInGoWithDesc for details.

cobra/completions_test.go

Line 476 in 59f7a0c

func TestShorthandFlagCompletionInGoWithDesc(t *testing.T) {

[marckhouzam]

This is a great start @fiftin, thanks!

Testing with a real program, the go logic to handle short flags without a space seems to work as expected.

I like your idea of checking if a completion function exists to decide if
we should complete a short flag value, or if we should add a space after
the short flag.

So, when testing with the __complete command things seem good nicely.

However when testing with the actual shell while performing completions
things don't work just yet. For zsh for example, we need to set the variable flagPrefix
in the script. What I mean is that if we do

$ helm -n<tab>

and get the completion choices 'firstns' and 'secondns', the zsh completion
script must convert them into '-nfirstns' and '-nsecondns'. We already
do this for the = case:

$ helm -n=<tab>
firstns       secondns

So the zsh shell script would need to be modified a little to handle the new case
of completion of short flags. The completion for bash v2 will also need
to be fixed (I haven't looked at the details, but it also does not work). And we'll need to
try things out for fish and powershell to see what they need. I can help
out with all this if you want, you just let me know.

But we are making good progress 👍

[marckhouzam]

Could you re-enable this test, it passes.

[fiftin]

Hi @marckhouzam I did. I updated PR yesterday with --force flag

[fiftin]

Hi @marckhouzam
I researched the completion in Bash. I didn't find solution how to complete following expression correctly:

$ helm -ldsV<tab>

Yes, it is possible, but we should print following completion:

-ldsVAL1 description
-ldsVAL2 description

In this case we get partially completed expression:

$ helm -ldsVAL

Otherwise, If we print:

VAL1 description
VAL2 description

We get:

$ helm VAL

I didn't find how to solve it.

I suggest to complete this expression to:

$ helm -lds=V

Why it works with =? Because $COMP_WORDBREAKS contains = (as I understood).

[fiftin]

Hi @marckhouzam , any updates? What do you think about our issue?

[marckhouzam]

Thanks @fiftin for the effort. I will need to take the time to give this proper attention but I will not have time for the next couple of weeks. I'll get back to this as soon as I can. Thanks for your patience

[fiftin]

Hi @marckhouzam ,

Do you have a time to work on the issue?

[marckhouzam]

@fiftin Are you no longer interested in making this contribution? I was about to review it when I saw you had closed it.
I'm sorry it took a long time, but it is summer time and I was on vacation.