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

cli: add completion support for fish shell #11284

Merged
merged 2 commits into from May 3, 2020

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented May 1, 2020

Fixes: #11283

image

Add completion support for fish shell

@sayboras sayboras requested review from a team as code owners May 1, 2020 13:26
@maintainer-s-little-helper
Copy link

Commits 21640e8ae428cace00a4931e2c3d35ef881595ee, 4c957959d84135f9c04ef0ff3d85e87f2cb16233 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 1, 2020
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

1 similar comment
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 1, 2020
@aanm aanm added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label May 1, 2020
Recently, fish completion is supported in cobra v1.0.0.
It would be nice to leverage this feature in cilium.feature/cilium-fish

Fixes: cilium#11283
Signed-off-by: Tam Mach <sayboras@yahoo.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 1, 2020
@qmonnet
Copy link
Member

qmonnet commented May 1, 2020

test-me-please

@qmonnet qmonnet self-assigned this May 1, 2020
@@ -62,7 +62,7 @@ func init() {
flags.StringP("host", "H", "", "URI to server-side API")
viper.BindPFlags(flags)
rootCmd.AddCommand(newCmdCompletion(os.Stdout))
rootCmd.SetOut(os.Stderr)
rootCmd.SetOut(os.Stdout)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change intentional? I wonder if it might be the cause for the CI failure?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is required as internally _cilium _complete command will be called, and it's using the stdout for showing completion options.

I just checked the failure, seems like we are expecting help output in stderr. Let me see if there is easy way to fix this.

@sayboras sayboras requested a review from a team as a code owner May 2, 2020 01:15
This commit is to update the cli test cases for help command

Originally, help command output is printing to stderr instead of stdout.
This commit might be reverted if it's not agreed by maintainer

Signed-off-by: Tam Mach <sayboras@yahoo.com>
@sayboras
Copy link
Member Author

sayboras commented May 2, 2020

test-me-please

1 similar comment
@qmonnet
Copy link
Member

qmonnet commented May 2, 2020

test-me-please

@qmonnet qmonnet merged commit 4a2a873 into cilium:master May 3, 2020
1.8.0 automation moved this from In progress to Merged May 3, 2020
@sayboras sayboras deleted the feature/cilium-fish branch May 4, 2020 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

Fish completion support
4 participants