Skip to content
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

Support different bash completion options #1509

Merged
merged 1 commit into from Dec 7, 2021

Conversation

marckhouzam
Copy link
Collaborator

@marckhouzam marckhouzam commented Oct 19, 2021

Fixes #1508

Bash completion V2 adds descriptions to completions. However, with certain options, the description must be removed. For example the menu-complete/menu-complete-backward bash option will immediately insert on the command-line the first completion returned, therefore we must remove the description to avoid it being added on the command-line also.

Based on the documentation found here
https://www.gnu.org/software/bash/manual/html_node/Commands-For-Completion.html
we remove descriptions for the cases:

  • menu-complete
  • menu-complete-backward
  • insert-completions

Copy link
Contributor

@Luap99 Luap99 left a comment

Thanks for the quick fix @marckhouzam

I think we should handle insert-completion like menu-complete,
insert-completion is documented to return all current completions.

One cool usecase I could think of is podman rm a[press key for insert-completions]. This would automatically insert all container names starting with an a.

@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Oct 19, 2021

I think we should handle insert-completion like menu-complete, insert-completion is documented to return all current completions.

One cool usecase I could think of is podman rm a[press key for insert-completions]. This would automatically insert all container names starting with an a.

Brilliant!
I was confused because such a behaviour would break completion for commands; but I now realize different keys are bound to different completion types. So, on my Mac, TAB triggers normal completion, while ESC-* triggers the insert-completion type. I love it.

I will update the PR.

Thanks @Luap99!

@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Oct 19, 2021

I think we should handle insert-completion like menu-complete

PR updated

Luap99
Luap99 approved these changes Oct 19, 2021
Copy link
Contributor

@Luap99 Luap99 left a comment

LGTM

@Luap99
Copy link
Contributor

Luap99 commented Oct 19, 2021

Do you think it is possible to add a test in your completion-testing repo?

@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Oct 19, 2021

Do you think it is possible to add a test in your completion-testing repo?

Should be possible, and we should do it.
Feel free to do a PR if you have some time, if not, I'll eventually get to it 🤞

spf13#1508

Based on the documentation found here
https://www.gnu.org/software/bash/manual/html_node/Commands-For-Completion.html
we remove descriptions for the following completion types:
- menu-complete
- menu-complete-backward
- insert-completions

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
marckhouzam added a commit to marckhouzam/cobra-completion-testing that referenced this issue Oct 21, 2021
These tests cover the issue: spf13/cobra#1508
and the corresponding fix: spf13/cobra#1509

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Oct 21, 2021

Do you think it is possible to add a test in your completion-testing repo?

I've done a PR to add tests: marckhouzam/cobra-completion-testing#9

marckhouzam added a commit to marckhouzam/cobra-completion-testing that referenced this issue Oct 21, 2021
These tests cover the issue: spf13/cobra#1508
and the corresponding fix: spf13/cobra#1509

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Oct 21, 2021

Looks like I confused GitHub 🙄

@marckhouzam marckhouzam reopened this Oct 21, 2021
umarcor pushed a commit to umarcor/cobra that referenced this issue Nov 5, 2021
spf13#1508

Based on the documentation found here
https://www.gnu.org/software/bash/manual/html_node/Commands-For-Completion.html
we remove descriptions for the following completion types:
- menu-complete
- menu-complete-backward
- insert-completions

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
umarcor pushed a commit to umarcor/cobra that referenced this issue Nov 15, 2021
spf13#1508

Based on the documentation found here
https://www.gnu.org/software/bash/manual/html_node/Commands-For-Completion.html
we remove descriptions for the following completion types:
- menu-complete
- menu-complete-backward
- insert-completions

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
umarcor pushed a commit to umarcor/cobra that referenced this issue Nov 16, 2021
spf13#1508

Based on the documentation found here
https://www.gnu.org/software/bash/manual/html_node/Commands-For-Completion.html
we remove descriptions for the following completion types:
- menu-complete
- menu-complete-backward
- insert-completions

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
umarcor pushed a commit to umarcor/cobra that referenced this issue Nov 25, 2021
spf13#1508

Based on the documentation found here
https://www.gnu.org/software/bash/manual/html_node/Commands-For-Completion.html
we remove descriptions for the following completion types:
- menu-complete
- menu-complete-backward
- insert-completions

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
jpmcb
jpmcb approved these changes Dec 7, 2021
Copy link
Collaborator

@jpmcb jpmcb left a comment

Seems good! @marckhouzam - I'm guessing in marckhouzam/cobra-completion-testing we need this merged before we test?

@jpmcb jpmcb merged commit 3fed3ef into spf13:master Dec 7, 2021
21 checks passed
@jpmcb jpmcb added this to the 1.3.0 milestone Dec 7, 2021
umarcor pushed a commit to umarcor/cobra that referenced this issue Dec 7, 2021
spf13#1508

Based on the documentation found here
https://www.gnu.org/software/bash/manual/html_node/Commands-For-Completion.html
we remove descriptions for the following completion types:
- menu-complete
- menu-complete-backward
- insert-completions

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
@marckhouzam marckhouzam deleted the fix/bashV2CompType branch Dec 8, 2021
@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Dec 8, 2021

Seems good! @marckhouzam - I'm guessing in marckhouzam/cobra-completion-testing we need this merged before we test?

Yes the cobra-completion-testing already had a test for this which was failing until this merge.

Thanks for merging @jpmcb !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants