fix(bash): Fix bash completion for suggestions that contain special characters.

[JeffFaer]

Special characters include whitepace, so this is more general than
#1743. This should also fix #1740.

I added some test cases to cobra-completion-testing. This PR makes them
pass. It also doesn't trip any of the performance regression tests. I'm
happy to submit those tests as a PR as well.

• https://github.com/JeffFaer/cobra-completion-testing/tree/special_characters
• JeffFaer/cobra-completion-testing@52254c1

[CLAassistant]

All committers have signed the CLA.

[ncw]

It would be nice to have this PR merged - many rclone users have noticed that we can't complete file names with spaces in :-(

[marckhouzam]

Sorry for the delay. I will be reviewing this next.

[marckhouzam]

@JeffFaer

I'm happy to submit those tests as a PR as well.
https://github.com/JeffFaer/cobra-completion-testing/tree/special_characters
JeffFaer/cobra-completion-testing@52254c1

That would be great!

[JeffFaer]

marckhouzam/cobra-completion-testing#25

[marckhouzam]

I think there may be some confusion here.
When doing completion for special characters, the characters should be automatically escaped.
So if the completion choice is a>b, doing prog [tab][tab] should yield prog a\>b.
You can rebase #1743 and try it out as it behaves like this.
You can also try doing such completion with zsh and see how it should behave.

[JeffFaer]

I've updated these changes to reflect the behavior you're looking for. It automatically escapes the last completion suggestion.

[JeffFaer]

Hmm there's actually a bad interaction here with COMP_WORDBREAKS. The default COMP_WORDBREAKS seems to include many special characters by default.

The bash completion script is handling a couple wordbreaks (= and :) "correctly" by removing everything up to and including the wordbreak from proposed COMPREPLYs so that readline doesn't duplicate the same prefix again. That's problematic for special characters that need to be escaped like > because that means the completion script won't be able to escape them anymore...

[JeffFaer]

I took a pass at a way to address the COMP_WORDBREAKS interaction (tl;dr: remove all wordbreaks that would need to be escaped)

[marckhouzam]

@JeffFaer Thanks for the update! It looks really good. However, while testing with the updated program of marckhouzam/cobra-completion-testing#25 I'm noticing a subtle behaviour, that I feel is incorrect.

bash-5.2$ ./testprog prefix special-chars bash4>[tab]
bash-5.2$ ./testprog prefix special-chars bash4\>redirect

In the above case, when the user presses [tab] after the redirect symbol > they should get some file completion and not the > being escaped. In the above case, since the user did not escape the > themselves, it means they want to use it as a redirect command, so Cobra should not escape it.

If the user where to escape the > themselves, that is when Cobra should continue the shell completion:

bash-5.2$ ./testprog prefix special-chars bash4\>[tab]
# Below is what I would expect, but this is not how the PR behaves
bash-5.2$ ./testprog prefix special-chars bash4\>redirect

Comparing it to zsh completion helps see the expected behaviour.
So, to summarize, this is what we need:

bash-5.2$ ./testprog prefix special-chars bash4[tab]
bash-5.2$ ./testprog prefix special-chars bash4\>redirect

bash-5.2$ ./testprog prefix special-chars bash4\>[tab]
bash-5.2$ ./testprog prefix special-chars bash4\>redirect

bash-5.2$ ./testprog prefix special-chars bash4>[tab]
# Normal file completion should happen here

The same approach should apply to the other special characters: , |, $, ;.
For #, zsh requires it to be escaped to continue completion (bash5\#<TAB>), but I'm not sure why, but let's do the same.

[JeffFaer]

It sounds like you're looking to have special characters escaped as soon as they end up on the command line instead of after the full word has been completed. I've updated the PR to do that. PTAL. Escaping special characters as soon as they end up on the command line lets us simplify some of the wordbreak complexity, too

[JeffFaer]

I'm not escaping here so that the list of results on multiple tabs doesn't include any extra escape characters...is that the behavior you're expecting?

[marckhouzam]

Could you give an example of the behaviour you are referring too?

[JeffFaer]

testprog prefix special-chars bash<tab><tab>

The list of options it reports is not escaped (i.e. bash4>redirect), but if you were to do bash4<tab> you'd get the escaped version (bash4\>redirect)

[marckhouzam]

I think we should escape it in the list. I will do that in #1743

[marckhouzam]

Hmm, I notice that zsh and fish don't show the escaping in their list. I'll think about it...

[marckhouzam]

@JeffFaer I worked many hours trying to make sure this was safe to merge, and in the end I felt there were too many changes I didn't understand well enough so I tried to simplify the change as much as I could. By the time I was done and had something working, it looked a lot like #1743. So, I feel the fair thing to do is to go back to #1743 and make it work.

I was able to do sort all this out thanks to the great tests you wrote in marckhouzam/cobra-completion-testing#25, so I want to merge that one also.

I'm going to pause working on this one and see if we can get #1743 working.
Thanks for your continue efforts.