Allow deleting multiple contexts at once #46
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
|
I signed it! |
|
CLAs look good, thanks! |
9b0fd3b to
7428144
Compare
|
Thanks so much for this PR. I'll review now. |
kubectx
Outdated
| @@ -32,6 +32,7 @@ USAGE: | |||
| kubectx <NEW_NAME>=<NAME> : rename context <NAME> to <NEW_NAME> | |||
| kubectx <NEW_NAME>=. : rename current-context to <NEW_NAME> | |||
| kubectx -d <NAME> : delete context <NAME> ('.' for current-context) | |||
| accepts multiple context names separated by a space | |||
There was a problem hiding this comment.
I think the right notation is to do:
kubectx -d <NAME> [<NAME...>]
then you don't need this line.
| @@ -148,6 +149,13 @@ rename_context() { | |||
| kubectl config rename-context "${old_name}" "${new_name}" | |||
| } | |||
|
|
|||
| delete_contexts() { | |||
| IFS=' ' read -ra CTXS <<< "${1}" | |||
There was a problem hiding this comment.
I don't think you need this.
See:
#!/bin/bash
f() {
for v in "${@}"; do
echo "--> ${v}"
done
}
f "${@:2}"this still iterates correctly.
There was a problem hiding this comment.
Actually, I did some tests here and in this function ${1} equals "ctx1 ctx2" (a string, not an array). It seems that ${@:2} mashes the arguments into a string, so I do need to split it afterwards.
After some research I found this, which seems to explain what's happening here.
When in scalar context the list of elements is joined with space characters with
bashandksh93and with the first character of$IFS(or nothing if$IFSis empty or space if it is unset) withzsh.
Would you be fine with keeping the current solution as it is? I didn't manage to come up with a solution to overcome this issue.
There was a problem hiding this comment.
This might be ok. I've been using mapfile -t to turn strings into arrays:
mapfile -t my_arr < <(kubectl get pods -o=name)
This only works on bash 4.0+. So most macOS users still can't use this with /bin/bash.
read solution is slower, but works, so we can keep it.
There was a problem hiding this comment.
That's nice, didn't know about this. Let's keep this one then :) the rest of the feedback is already addressed.
|
One of my hopes is to get the bash/zsh completion to work with this command, but so far I have not been able to find someone who knows about this topic to fix that. I think having tab completion would totally be useful with this command but I won't require you to fix the scripts. |
fixes ahmetb#39 Example: `kubectx -d ctx1 ctx2`
7428144 to
4a1d73d
Compare
|
@ahmetb I'm not too familiar with how completion works for bash/zsh, but after this is merged I'd love to have a look into it, could also be a good learning experience :) |
fixes #39
Example:
kubectx -d ctx1 ctx2This can be useful when doing things like
kubectx -d $(kubectx | grep test)