Fix some issues in bash autocompletion#1674
Fix some issues in bash autocompletion#1674dearchap merged 2 commits intourfave:mainfrom MrNaif2018:fix-bash-autocomplete
Conversation
|
@MrNaif2018 Can you add a test case(s) for this ? I will attempt to backport to v2.x after that. Thanks |
|
Sure! Could you guide me where to check? Because bash behavior I tested was via tab, so not sure how to test it non-interactively |
|
@MrNaif2018 You could use this test as a starting point. You can either add additional cases for this test or add a new test Line 1177 in f983141 |
|
@dearchap I've added a simple test, but the issue is, it won't catch the bug anyway :D No go-based code can catch the bug |
|
@MrNaif2018 Just to confirm you want to merge this in v2 or v3 ? |
|
I am currently using v2 version, so I would like if it would land in v2 too. v3 probably needs this too if it uses same setup |
|
@MrNaif2018 Can you re-submit a PR for v2 ? I will merge this into main for now |
|
Is it v2-maint? |
|
Yes |
What type of PR is this?
What this PR does / why we need it:
With current version of autocomplete script, shell quotes gets expanded incorrectly
Consider the following example:
my-cli --url http://localhost:8000 he(pressing tab should auto-complete to help, but it doesn't)Why doesn't it auto-complete?
Because it calls the following command:
my-cli --url http : //localhost : 8000 --generate-bash-completionAnd if we put it in quotes my-cli --url "http://localhost:8000" he
It calls:
my-cli --url '"http://localhost:8000"' --generate-bash-completionBoth fail, so CLI fails to call correct URL and return a list of completions.
With this PR it sends correct data
Actually I am not sure if that's the best solution, I checked how cobra does this and converted to our use case
It looks like the
_init_completionfunction does all the magic and makes it work, I don't understand why unfortunatelyP.S. I hope it'll be backported to 2.x series too (:
Testing
I tested multiple cases, i.e. sending one positional argument, 2 of them, or argument + flag, the CLI receives same arguments as before, but now more cases work
Release Notes