-
Notifications
You must be signed in to change notification settings - Fork 87
Adding bash completion, for now only "-n" completes with available namespaces #371
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
Adding bash completion, for now only "-n" completes with available namespaces #371
Conversation
| const ( | ||
| bash_completion_func = `# $1 is the name of resource (required) | ||
| # $2 is template string for kubectl get (optional) | ||
| __kubectl_parse_get() |
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.
What should the assumption be in an openshift environment (e.g. oc might be present but not kubectl)?
Would adding list to pkg/kube/namespace be an alternative?
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.
basically to auto-complete, bash executes "some executable", it may be anything, in this case we are executing "kubectl -n ..." we could execute "skupper -something" or build a particular new skupper executable like "skupper-completer" orrr whatever option we want.
Will take a look now if from bash I can "try" with "oc" and if fails, then try with "kubectl", which I think it is totally possible.
| rootCmd = &cobra.Command{Use: "skupper"} | ||
| rootCmd = &cobra.Command{ | ||
| Use: "skupper", | ||
| BashCompletionFunction: bash_completion_func, |
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.
@nicob87 I would suggest writing the bash_completion in a separate .sh file,
which would make it way easier to maintain as we go.
Then we could tweak the build command for the skupper binary to include a
build-time variable that points to the bash_completion.sh file, like:
go build -ldflags "-X main.BashCompletionEncoded=`cat bash_completion.sh | base64 -w 0`" ...
Then inside skupper.go, you could decode it as:
bashCompletionFunction, err := base64.StdEncoding.DecodeString(BashCompletionEncoded)
Not sure if shell will impose some limitation, but if it works, might be worth trying.
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 tested it locally using a 65k shell script and it worked just fine.
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.
mmmm and that works for windows and macos? (windows does not have a cat, to begin) will investigate options anyway...
awesome review!
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.
maybe just the "bash_completion_func" variable definition to a dedicated *.go file is a simple solution, if you want that file to be a ".sh" I think we should "read" that file from our go code, but, this makes a dependence and a possible runtime error, on a .sh file, which I do not like so much ...
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.
The dependency exists only at build-time, after that it no longer exists, but a separate go file just with the constant
might also work.
7d9f270 to
4c4af01
Compare
|
Closing as issue #574 has been raised to track this proposal. |
Closes #367
based/stolen from:
https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/cmd.go#L293
It requires "kubectl" to be installed.
To give it a try just: