-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(bash): Fix bash completion for suggestions that contain special characters. #2126
base: main
Are you sure you want to change the base?
Conversation
It would be nice to have this PR merged - many rclone users have noticed that we can't complete file names with spaces in :-( |
Sorry for the delay. I will be reviewing this next. |
That would be great! |
I think there may be some confusion here. |
…haracters. Special characters include whitepace, so this is more general than spf13#1743. This should also fix spf13#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 Signed-off-by: Jeffrey Faer <jeffrey.faer@gmail.com>
…uoting. Signed-off-by: Jeffrey Faer <jeffrey.faer@gmail.com>
They're not going through compgen so they don't need it. Signed-off-by: Jeffrey Faer <jeffrey.faer@gmail.com>
Signed-off-by: Jeffrey Faer <jeffrey.faer@gmail.com>
ec43e79
to
9771cb0
Compare
I've updated these changes to reflect the behavior you're looking for. It automatically escapes the last completion suggestion. |
Hmm there's actually a bad interaction here with The bash completion script is handling a couple wordbreaks ( |
Signed-off-by: Jeffrey Faer <jeffrey.faer@gmail.com>
I took a pass at a way to address the |
Signed-off-by: Jeffrey Faer <jeffrey.faer@gmail.com>
The other ones should already have been quoted by the completion script. Signed-off-by: Jeffrey Faer <jeffrey.faer@gmail.com>
398ef9f
to
7a9725b
Compare
@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.
In the above case, when the user presses If the user where to escape the
Comparing it to
The same approach should apply to the other special characters: |
…ld print them to the command line (COMP_TYPE=9)
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 |
|
||
63) | ||
# Type: Listing completions after successive tabs | ||
__%[1]s_handle_standard_completion_case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you give an example of the behaviour you are referring too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
)
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.