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

[Enhancement] Adds option to periodically update time segment #1193

Open Syphdias opened 5 years ago

Syphdias commented 5 years ago

This implements #1190

Hey @romkatv, porting your code was easy. Thank you! I do have some questions though.

  1. You mentioned big git repos which would take time. I don't think this is the case on next. Please correct my if you think I'm wrong. PROMPT gets evaluated every time you redraw the prompt. On master PROMPT is something like %f%b%k$(__p9k_build_left_prompt), on next it is __p9k_unsafe_PROMPT. This variable get filled in the precmd hook which is not called on prompt-reset. This was done to fix #1119 another problem but come is very handy.
  2. Any ideas for how to test this?
  3. I ran into some issues. Which lead to the notice in the README and a default delay of 60 seconds. The issues are with going through the history and tab completion/suggestions. Example for weirdness: This is how it should look: image This is what happens: image I even had a segfault once doing this. I can reproduce the segfault by ls <tab><arrow-keys-especially-up>
  4. Changing keybinds You mentioned that zle-keymap-select was a rude practice in #1190(?) Would you say the same about an option that would do this?
    renew-prompt-accept-line() {
    zle .reset-prompt
    zle accept-line
    }
    zle -N renew-prompt-accept-line
    bindkey "^M" renew-prompt-accept-line
    ZSH_AUTOSUGGEST_CLEAR_WIDGETS+=renew-prompt-accept-line
Syphdias commented 5 years ago

@romkatv wtf? How did you comment before I even created the PR... Well, I think I answer your question in 1

Edit: You commented on the commit, not the PR, I see... :man_facepalming:

romkatv commented 5 years ago

[...] This was done to fix #1119 another problem but come is very handy.

Interesting. So does it mean background_jobs broke? Just checked -- yes, it is broken in next. I opened https://github.com/bhilburn/powerlevel9k/issues/1196.

  1. Any ideas for how to test this?

None.

  1. I ran into some issues...

Ha-ha, this is funny. I mailed https://www.zsh.org/mla/workers//2019/msg00161.html. ZSH seems kinda buggy. This is the 4th bug I've bumped into and I haven't been using ZSH even for a month.

  1. Changing keybinds You mentioned that zle-keymap-select was a rude practice in #1190(?) Would you say the same about an option that would do this?
renew-prompt-accept-line() {
    zle .reset-prompt
    zle accept-line
}
zle -N renew-prompt-accept-line
bindkey "^M" renew-prompt-accept-line
ZSH_AUTOSUGGEST_CLEAR_WIDGETS+=renew-prompt-accept-line

Is this related? I'd say this code is fine in .zshrc and not fine anywhere else. If your plugin thinks it's OK to override renew-prompt-accept-line, it should be OK for other plugins too. And it obviously won't work. Great post on this topic, highly recommended: https://devblogs.microsoft.com/oldnewthing/?p=35413.

Syphdias commented 5 years ago

Interesting. So does it mean background_jobs broke? Just checked -- yes, it is broken in next. I opened #1196.

Good catch! Interestingly I did run into trouble with background jobs but did not notice at the time.

  1. I ran into some issues...

Ha-ha, this is funny. I mailed https://www.zsh.org/mla/workers//2019/msg00161.html. ZSH seems kinda buggy. This is the 4th bug I've bumped into and I haven't been using ZSH even for a month.

😆

Is this related? I'd say this code is fine in .zshrc and not fine anywhere else. If your plugin thinks it's OK to override renew-prompt-accept-line, it should be OK for other plugins too. And it obviously won't work. Great post on this topic, highly recommended: https://devblogs.microsoft.com/oldnewthing/?p=35413.

Actually, this is straight from my .zshrc I don't think I have overridden renew-prompt-accept-line since I named it. I should have gone with the p9k naming conventions in my example. But it does override the keybind for ^M (Enter) from accept-line to the new function. Better example:

__p9k_renew-prompt-accept-line() {
    zle .reset-prompt
    zle accept-line
}
zle -N __p9k_renew-prompt-accept-line
bindkey "^M" __p9k_renew-prompt-accept-line
ZSH_AUTOSUGGEST_CLEAR_WIDGETS+=__p9k_renew-prompt-accept-line

So I'm not asking for overriding the function but overriding the keybind.

Syphdias commented 5 years ago

Is this related? I'd say this code is fine in .zshrc and not fine anywhere else. If your plugin thinks it's OK to override renew-prompt-accept-line, it should be OK for other plugins too. And it obviously won't work. Great post on this topic, highly recommended: https://devblogs.microsoft.com/oldnewthing/?p=35413.

I don't quite get what you are playing at. If I'd change a bindkey setting twice it will simply be overridden there is no conflict or logical problem besides the fact that you might not want to to be overridden. Just to clarify: I'd offer this as an option, not by default.

romkatv commented 5 years ago

I'm not sure where the example with key binding is coming from. My comment was abut TMOUT. The user isn't explicitly asking TMOUT to be hogged, they just want thge clock to tick. Many plugins would like to claim TMOUT for themselves to implemented this and that but it's not cool because there is only one TMOUT. Likewise with zle overrides. The user isn't asking for zle-keymap-select to be overridden, they just want vi mode in their prompt. If you claim zle-keymap-select to implement vi_mode segment, then the user cannot use any other plugin that also wants to hook into zle-keymap-select.

This is a minor point. I suggest we drop this subject to save time for something more important.

Syphdias commented 5 years ago

This is a minor point. I suggest we drop this subject to save time for something more important.

Agreed. We are arguing about different things.

I still like the feature, but since it is a bit hacky and buggy, I'd like to have another option. I'll implement __p9k_renew-prompt-accept-line to see if it is feasible to achieve something similar.