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

SPACESHIP_KUBECONTEXT_NAMESPACE_SHOW should be empty #588

Merged
merged 2 commits into from Dec 13, 2018
Merged

SPACESHIP_KUBECONTEXT_NAMESPACE_SHOW should be empty #588

merged 2 commits into from Dec 13, 2018

Conversation

nomaed
Copy link
Contributor

@nomaed nomaed commented Dec 11, 2018

Correctly initializes unset SPACESHIP_KUBECONTEXT_NAMESPACE_SHOW to be empty instead of a "()" string

Fix #587

@salmanulfarzy salmanulfarzy added improvement A PR that make small changes for improving UX, performance, readability, etc needs doc update labels Dec 11, 2018
@salmanulfarzy
Copy link
Member

salmanulfarzy commented Dec 11, 2018

Thank you for the quick fix 👍

kubecontext color groups empty fix

Please also update the relevant documentation.

@nomaed
Copy link
Contributor Author

nomaed commented Dec 11, 2018

How should an empty default value be rendered in the docs, just an empty cell, ` ` or ``?

@salmanulfarzy
Copy link
Member

salmanulfarzy commented Dec 11, 2018

Since this value is specified as empty in the next cell, Using the middot symbol · might be okay, Which is also used for white space in documentation.

Leading and trailing whitespaces in inline code blocks will be stripped, to indicate that a white space is present, use the middot symbol ·

CONTRIBUTING.md#documentation

@nomaed
Copy link
Contributor Author

nomaed commented Dec 11, 2018

Yeah, I'm familiar with this section, but there is no whitespace there. Just an empty value.
That's why I'm unsure.

sections/kubecontext.zsh Outdated Show resolved Hide resolved
Iteration over SPACESHIP_KUBECONTEXT_COLOR_GROUPS list will be done only
on matched pairs. It will eliminate dangling unmatched value, or if the
configuration has been set incorrectly to a string value (of list length 1)
@nomaed
Copy link
Contributor Author

nomaed commented Dec 13, 2018

@denysdovhan could you review this fix please?

I started converting my co-workers to spaceship-prompt and two of them happened to install the latest version with the bug. I got laughed at when I arrived to work this morning, for creating this bug in the prompt :P

@denysdovhan denysdovhan merged commit 5bf337e into spaceship-prompt:master Dec 13, 2018
@denysdovhan
Copy link
Member

@salmanulfarzy could you release a new version? 🙏

@nomaed nomaed deleted the patch-1 branch December 13, 2018 17:13
@salmanulfarzy
Copy link
Member

Done @denysdovhan, Published as part of v3.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement A PR that make small changes for improving UX, performance, readability, etc
Development

Successfully merging this pull request may close these issues.

None yet

4 participants