search

Powerlevel9k / powerlevel9k

Powerlevel9k was a tool for building a beautiful and highly functional CLI, customized for you. P9k had a substantial impact on CLI UX, and its legacy is now continued by P10k.
https://github.com/romkatv/powerlevel10k
MIT License
13.47k stars 947 forks source link

[kubecontext] Add possibility to on/off segment with commands #1252

Open grzesuav opened 5 years ago

grzesuav commented 5 years ago

It would be great to have possibility to customize kubcontext behavior with on/off switch. It would require changes around https://github.com/bhilburn/powerlevel9k/blob/master/powerlevel9k.zsh-theme#L1694 :

I am aware of https://github.com/bhilburn/powerlevel9k/issues/860 but I thinking about extending current segment.

What do you think about idea ? Does it have any drawbacks (i.e. is it impossible for segment to display "nothing") ?

I can try to implement this idea (need help with variable naming tough).

Cheers

jerome-diver commented 5 years ago

i don't know nothing about kubcontext (what htis is about ?). But i'm thinking that you can do the change and post a ull request for that. I can help you for zsh script if you need. create a variable is easy and it is possible for a segment to display nothing as long as nothing is empty char and also as long as the segment can also be not shown at all. You should create an option named variable to use for that, then implement the doc around this new option to give a true or false value from .zshrc file, then a condition from the theme should be able to implement segment (or not). If you open the theme file (who is in the root directory) you will see how this is happening for many other segments with options. It is easy to do as long as you can read a zsh script.

grzesuav commented 5 years ago

@jerome-diver thanks :) I would need help with review for sure. But first I would like to hear what @bhilburn thinks about the idea I presented.

grzesuav commented 5 years ago

I think similar snippet can be used as in https://github.com/bhilburn/powerlevel9k/pull/1220 but which will test the presence of a variable.

dritter commented 5 years ago

Hi @grzesuav ,

I think this would be a cool addition. If you want to implement such change, please do it on top of the next branch, which is our development branch.

About your idea: If I understand you correctly, you want to add a new variable and if this variable is set, disable/hide the segment, and add aliases for kubeon/kubeoff, to set that variable. While this idea is tempting, IMHO the flow how P9K retrieves data should be passive. So, instead of providing aliases and setting that state actively, P9K should look for other signals to turn that segment off. Implementing this in a more passive way, should look more like this: https://github.com/porcupie/powerlevel9k/commit/769dbf915d579e6721526077a04a068f0a0377e4#diff-ea5a405f0331722fc10a832209ffe20bR1422 (or like the PR in #1220 ).

Are there other things that happen (like a variable being set, or a lockfile being written), if you do a kubeon? These could also be a way to disable the segment in P9K..

Edit: In which way does your suggestion differ from #1220 (besides a different technical approach)?

jerome-diver commented 5 years ago

@grzesuav ok, then look at this commit: https://github.com/bhilburn/powerlevel9k/pull/1241/commits/33207384e64eb9e0cded71614e9d1be9ffce39eb on line 1634 you can see how to test an option you can add iniside .zshrc file. It is as easy as it is: just a condition for a zsh varaible who can then be used from .zshrc file. The result of condition is to construct a segment. just this...

grzesuav commented 5 years ago

@dritter My intention is to do it in active way - I have cluster connectivity set all the time (there is no time when my context is no present like in #1220) - I just do not want to see it all the time, because I do not work with it all the time.

So kubeon/kubeoff (as in many other k8s prompt's) is just shortcut to show/not show context information in terminal, it doesn't do anything else.

Only passive way how it can be done (which I can think of) is just to commit code checking if some variable is present, keeping aliases for setting this variable outside. But then anyone who would like to use this variable would need to create this aliases on its own (which is also ok for me).

Is it ok ?

@jerome-diver thanks, planning to do something similar :)

romkatv commented 5 years ago

Perhaps there should be a general mechanism to disable/enable a prompt segment? E.g., if $(( P9K_DIR_DISABLED )) then dir segment is not shown. This condition needs to be evaluated on every precmd; not by the segments themselves but by the higher-level function that calls the segments.

Then kubeon and kubeoff can be defined as follows:

kubeon() { P9K_KUBECONTEXT_DISABLED=0 }
kubeoff() { P9K_KUBECONTEXT_DISABLED=1 }
grzesuav commented 5 years ago

I cannot say if this would be useful mechanism in general, for sure it fits for working with kubectl when not always you want to see it and you would like to have an easy way to turn it off and on

romkatv commented 5 years ago

@grzesuav My suggestion/question was primarily directed towards @dritter. My role on p9k is limited to observation. However, I do care about the new features that get implemented in p9k as I'm obligated to pull them to Powerlevel10k. Since implementing on/off switch just for kubecontext is the same amount of work and complexity as implementing it for all segments, future and present, I'd very much prefer the generic solution. It'll work for every feature request of similar nature and will keep overall complexity of the implementation, API surface and configuration space down.

grzesuav commented 5 years ago

@romkatv ok, understood ;) I can try to do POC of kubecontext solution.

If it is worth to be generalized, we can try to do it but this is my very beginning working with powerlevel code so I will feel more comfortable to do small change first, and then generalize if needed.

dritter commented 5 years ago

@romkatv I agree. We should generalize that approach. I'm always bugged by single purpose variables.

@grzesuav Here is the main loop @romkatv was talking about (at least for the left prompt; right prompt is here) in the next branch.

grzesuav commented 5 years ago

Ok thanks guys !

Will try to implement something and push next week

Thutm commented 5 years ago

(Sorry if i'm rehashing this)

tl;dr - If your using an alias to unset a env variable to turn off the segment, will you still use kubectl while the segment is off generally?

Is there a benefit to turning this on and off in this way rather than unsetting your $KUBECONFIG itself? PR #1220 will fix the prompt so that if there is no context (or not kubeconfig set) it will hide itself, which I think it the best behavior as the prompt is then reflecting what your "pointing" or configured to talk to. (i.e. Segment hides when kubectl is not configured to talk to anything, but shows up when you are)

I assume this would allow us to do something like unset $KUBECONFIG and it will hide the prompt, then set it again and it would show up. I use aliases/functions to switch it back and forth to different configuration files as needed so i'm usually setting/unsetting KUBECONFIG anyway. Not sure if that is common practice though?

grzesuav commented 5 years ago

@Thutm

  1. you do not need to have KUBECONFIG set to work with k8s (read ~/.kube/config directly)
  2. I do not switch on/off aliases in terminal on demand, so probably not so common ;) (or at least one person is not following it ;))
Thutm commented 5 years ago

Ah, okay I see. So when you interact with k8s you would do something like ‘kubectl —kubeconfig=’ type thing then? Sorry for off topic question, I’m still learning k8s myself.

grzesuav commented 5 years ago

@Thutm No, I don't. By default ./kube/config file is used, you do not need to specify anything.

grzesuav commented 5 years ago

Hi, taken longer that I expected, but I did a POC - https://github.com/Powerlevel9k/powerlevel9k/pull/1359 it seems it is working, I have tested it locally

grzesuav commented 5 years ago

I just realize that I did changes against master branch, I tried to rebase on next, but the file looks totally different. I tried to apply changes in https://github.com/Powerlevel9k/powerlevel9k/blob/next/powerlevel9k.zsh-theme#L202 , but it seems it is executed only once at load. Is there another place when I can put my code ? @dritter @romkatv ?

grzesuav commented 4 years ago

@jerome-diver @romkatv @dritter got time and figured it up . Updated PR against next branch and tested locally.

Can you take a look ?

romkatv commented 4 years ago

Took a look. Should work.

Since this issue has been opened, I've learned of a better solution. Or rather two complementary solutions. The first is to display kubernetes context only when the current command you are typing is kubectl (or matches a configurable pattern in general). The second is a key binding that quickly toggles the display of kubernetes context. The binding can be implemented to reveal kubernetes context in under a millisecond without changing your command buffer.

grzesuav commented 4 years ago

@romkatv nice to know. Could you point to some resource about it ?

I think in general this PR can be useful outside a context of kubectl (as it is generic) but I curious in general :)

romkatv commented 4 years ago

@romkatv nice to know. Could you point to some resource about it ?

I'm afraid this would violate p9k code of conduct.

I think in general this PR can be useful outside a context of kubectl (as it is generic)

Indeed.

As I mentioned earlier, I have no power to reject or accept p9k PRs. I'm just a ghost haunting an abandoned castle.