Add kubecontext section to show currently active kubectl context #76

owais · 2018-10-01T22:58:30Z

Issue: #73

Add kubecontext section

Description

Shows currently active kubectl context

Screenshots (if appropriate):

How Has This Been Tested?

I have tested using MacOS
I have tested using Linux

Checklist:

I have checked that no other PR duplicates mine
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have updated the tests accordingly.

matchai

Hey owais!

Thank you for your contribution! 😄
A few small comments here and there, but otherwise your PR looks good! 👍

Cheers!

matchai · 2018-10-01T23:40:31Z

functions/__sf_section_kubecontext.fish

+# ------------------------------------------------------------------------------
+function __sf_section_kubecontext -d "Display the kubernetes context"
+	__sf_util_set_default SPACEFISH_KUBECONTEXT_SHOW true
+	__sf_util_set_default SPACEFISH_KUBECONTEXT_PREFIX " "


Looking at the spaceship implementation, the default prefix is "at ": https://github.com/denysdovhan/spaceship-prompt/blob/master/sections/kubecontext.zsh#L13

matchai · 2018-10-01T23:40:53Z

functions/__sf_section_kubecontext.fish

+	__sf_util_set_default SPACEFISH_KUBECONTEXT_SHOW true
+	__sf_util_set_default SPACEFISH_KUBECONTEXT_PREFIX " "
+  __sf_util_set_default SPACEFISH_KUBECONTEXT_SUFFIX $SPACEFISH_PROMPT_DEFAULT_SUFFIX
+	__sf_util_set_default SPACEFISH_KUBECONTEXT_SYMBOL "☸️"


It also appears the original implementation has a space following the ☸️ emoji. I would suggest adding it 😃

matchai · 2018-10-01T23:41:53Z

docs/Options.md

+| Variable | Default | Meaning |
+| :------- | :-----: | ------- |
+| `SPACEFISH_KUBECONTEXT_SHOW` | `true` | Show current kubectl context |
+| `SPACEFISH_KUBECONTEXT_PREFIX` | `$SPACEFISH_PROMPT_DEFAULT_PREFIX` | Prefix before the kubectl section |


I would suggest also changing the prefix here to at.

matchai · 2018-10-01T23:42:32Z

docs/Options.md

@@ -207,11 +207,24 @@ Go section is shown only in directories that contain `Godeps`, `glide.yaml`, any
 | Variable | Default | Meaning |
 | :------- | :-----: | ------- |
 | `SPACEFISH_GOLANG_SHOW` | `true` | Show current Go version or not |
-| `SPACEFISH_GOLANG_PREFIX` | `$SPACEFISH_PROMPT_DEFAULT_PREFIX` | Prefix before the Go section |
+| `SPACEFISH_GOLANG_PREFIX` | ` ` | Prefix before the Go section |


I think you might've unintentionally changed the value here.
The Go prompt prefix is correct to be $SPACEFISH_PROMPT_DEFAULT_PREFIX.

matchai · 2018-10-01T23:44:01Z

functions/__sf_section_kubecontext.fish

+		return
+	end
+
+	set -l sf_kube_context (kubectl config current-context 2>/dev/null)


To redirect to stderr in Fish, you would replace 2> with ^ 😄

Since this variable is local, I wouldn't use the sf_ prefix. That has only been used in the past for global variables so that we don't occupy the namespace.

matchai · 2018-10-01T23:46:19Z

functions/__sf_section_kubecontext.fish

+	__sf_lib_section \
+		$SPACEFISH_KUBECONTEXT_COLOR \
+		$SPACEFISH_KUBECONTEXT_PREFIX \
+		"$SPACEFISH_KUBECONTEXT_SYMBOL $sf_kube_context" \


If we're going to add the space directly to the symbol (mentioned in the above comment), I would suggest then removing the space here.

matchai · 2018-10-01T23:49:53Z

docs/README.md

@@ -17,6 +17,7 @@
  * [Haskell (haskell)](./docs/Options.md#haskell-haskell)
  * [Pyenv (pyenv)](./docs/Options.md#pyenv-pyenv)
  * [Go (golang)](./docs/Options.md#go-golang)
+  * [Kubectl (kubectl)](./docs/Options.md#kubectl)


Like in the docs of spaceship, I would change the text here to Kubectl context (kubecontext). You will need to then change the anchor to #kubectl-context-kubecontext. 🔗

matchai · 2018-10-01T23:50:21Z

docs/Options.md

 | `SPACEFISH_GOLANG_SUFFIX` | `$SPACEFISH_PROMPT_DEFAULT_SUFFIX` | Suffix after the Go section |
 | `SPACEFISH_GOLANG_SYMBOL` | `🐹·` | Character to be shown before Go version |
 | `SPACEFISH_GOLANG_COLOR` | `cyan` | Color of Go section |

+
+### Kubectl \(`kubectl`\)


Like in the docs of spaceship, I would change the text here to Kubectl context (kubecontext) 😄

owais · 2018-10-02T00:17:16Z

@matchai ^

matchai · 2018-10-02T00:38:10Z

docs/README.md

@@ -17,6 +17,7 @@
  * [Haskell (haskell)](./docs/Options.md#haskell-haskell)
  * [Pyenv (pyenv)](./docs/Options.md#pyenv-pyenv)
  * [Go (golang)](./docs/Options.md#go-golang)
+  * [Kubectl (kubectl)](./docs/Options.md#kubectl-context-kubecontext)


One last nitpick:
I would change the text of the link to Kubectl context $kubecontext$ 👍

Issue: matchai#73

matchai · 2018-10-02T01:43:45Z

Looks good! Thanks again for your contribution! 😄

MindTooth · 2018-11-23T07:17:56Z

This broke with the latest Docker update I believe. Now that Docker bundles kubectl, I have the snow flake all the time.

matchai · 2018-11-23T14:42:53Z

Hmm... It seems you may be right 🤔
@MindTooth Would you mind creating an issue for this? I'll try to address it in the next few days if no one else can.

MindTooth · 2018-11-23T15:40:14Z

Done. 😄

owais force-pushed the kubecontext branch 2 times, most recently from ac6265e to fcd91ea Compare October 1, 2018 23:15

owais mentioned this pull request Oct 1, 2018

Implement a prompt section for Kubectl version (☸️) #73

Closed

matchai approved these changes Oct 1, 2018

View reviewed changes

matchai assigned owais Oct 1, 2018

matchai reviewed Oct 2, 2018

View reviewed changes

Add kubecontext section to show currently active kubectl context

afc14c9

Issue: matchai#73

owais force-pushed the kubecontext branch from 880081f to afc14c9 Compare October 2, 2018 00:53

matchai merged commit 8108b6b into matchai:master Oct 2, 2018

MindTooth mentioned this pull request Nov 23, 2018

Kubernetes included with Docker 18.09 sets the kubectl-section to always be enabled #111

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add kubecontext section to show currently active kubectl context #76

Add kubecontext section to show currently active kubectl context #76

owais commented Oct 1, 2018 •

edited

Loading

matchai left a comment

matchai Oct 1, 2018

matchai Oct 1, 2018

matchai Oct 1, 2018

matchai Oct 1, 2018

matchai Oct 1, 2018

matchai Oct 1, 2018

matchai Oct 1, 2018

matchai Oct 1, 2018

matchai Oct 1, 2018

owais commented Oct 2, 2018

matchai Oct 2, 2018 •

edited

Loading

owais Oct 2, 2018

matchai commented Oct 2, 2018

MindTooth commented Nov 23, 2018

matchai commented Nov 23, 2018

MindTooth commented Nov 23, 2018

Add kubecontext section to show currently active kubectl context #76

Add kubecontext section to show currently active kubectl context #76

Conversation

owais commented Oct 1, 2018 • edited Loading

Description

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

matchai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owais commented Oct 2, 2018

matchai Oct 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matchai commented Oct 2, 2018

MindTooth commented Nov 23, 2018

matchai commented Nov 23, 2018

MindTooth commented Nov 23, 2018

owais commented Oct 1, 2018 •

edited

Loading

matchai Oct 2, 2018 •

edited

Loading