Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

vi_mode suddenly stopped working #319

Closed
agsimeonov opened this issue Oct 1, 2016 · 44 comments
Closed

vi_mode suddenly stopped working #319

agsimeonov opened this issue Oct 1, 2016 · 44 comments

Comments

@agsimeonov
Copy link

agsimeonov commented Oct 1, 2016

Running on Mac OS X El Capitain 10.11.6 and vi_mode stopped working on both my machines all of a sudden today. I haven't done any updates on one my machine recently (that I recall). The other one I did an update through brew, but I can confirm that I haven't done the same on the other which is exhibiting the same issue. Everything else seems to work fine.

Edit:
Just logged in to a remote Linux (Ubuntu) machine I haven't updated or used in many many months, and vi_mode is gone on there too. What is happening?

@agsimeonov agsimeonov changed the title vi_mode doesn't work with readline 7.0 vi_mode suddenly stopped working Oct 1, 2016
@agsimeonov
Copy link
Author

agsimeonov commented Oct 1, 2016

So I have my zsh dot files update my theme every once in a while. It seems that ever since commit 520eed1 vi_mode is no longer working. Commits before that exhibit the correct behavior.

@agsimeonov
Copy link
Author

agsimeonov commented Oct 1, 2016

Okay so if I run manually:

function zle-line-init {
  powerlevel9k_prepare_prompts
  if (( ${+terminfo[smkx]} )); then
    printf '%s' ${terminfo[smkx]}
  fi
  zle reset-prompt
  zle -R
}

function zle-line-finish {
  powerlevel9k_prepare_prompts
  if (( ${+terminfo[rmkx]} )); then
    printf '%s' ${terminfo[rmkx]}
  fi
  zle reset-prompt
  zle -R
}

function zle-keymap-select {
  powerlevel9k_prepare_prompts
  zle reset-prompt
  zle -R
}

zle -N zle-line-init
zle -N ale-line-finish
zle -N zle-keymap-select

vi_mode will work once more.

I am running on zsh version 5.2 which is the latest so it seems those overrides are still needed, so perhaps it would be wise to bring them back.

@machinshin
Copy link

i am also seeing this behavior. @agsimeonov , did you add this logic to your ~/.zshrc, or what?

@agsimeonov
Copy link
Author

agsimeonov commented Oct 1, 2016

Yeah @machinshin, after adding the above snippet to my ~/.zshrc vi_mode works fine.

@qifei9
Copy link

qifei9 commented Oct 1, 2016

Same issue here. I just enabled vi_mode yesterday and found that it does not work.

@agsimeonov
Copy link
Author

agsimeonov commented Oct 1, 2016

I created a pull request #320 which reverts the commit that causes this issue.

@ret2src
Copy link

ret2src commented Oct 1, 2016

I can confirm this issue on macOS Sierra.

@bhilburn
Copy link
Member

bhilburn commented Oct 3, 2016

Hi all -

Thanks for the bug report, verifications, and the PR, @agsimeonov!

The issue with reverting that commit is that it will slow down performance in git repositories pretty tremendously. Removing that code fixed some slowness in git repos - I had a couple of other users test that their VI prompts still worked before merging, but it looks like we missed something.

I'll have to play with the code and see if there is a way to fix the VI mode prompt while not re-introducing the latency in VCS repos. If anyone else wants to help by taking a look at this, I would really appreciate it! The more eyes on it, the better :)

@qifei9
Copy link

qifei9 commented Oct 3, 2016

The PR is not from me. The thanks should go to @agsimeonov :)

@bhilburn
Copy link
Member

bhilburn commented Oct 3, 2016

Fixed! Thanks, @qifei9!

@slester
Copy link

slester commented Oct 11, 2016

I'm still seeing this behavior (ubuntu using prezto). I just cloned the repository.

@bhilburn
Copy link
Member

Hi @slester - Yeah, I haven't had a chance to figure out how to fix this while also not dramatically slowing down performance in git repos (see my comment above).

I'm hoping to get time this week to take a look at it.

@slester
Copy link

slester commented Oct 11, 2016

No worries! If need be, I can have a look later today as well if you're open to a PR if I find a nice solution.

@bhilburn
Copy link
Member

@slester - I'm always open to PRs & contributions =)

@bhilburn
Copy link
Member

bhilburn commented Nov 9, 2016

@agsimeonov @slester @christian-titze @qifei9 @machinshin

Are you using the vi-mode plugin distributed with Oh-My-Zsh, or are you just defining your own mode indicators and using vi-mode as built-in to ZSH?

Trying to sort out what's going wrong, and I'm having trouble fixing this without negatively impacting the performance of powerlevel9k.

@ret2src
Copy link

ret2src commented Nov 9, 2016

@bhilburn I'm using the vi-mode plugin distributed with Oh-My-Zsh.

@qifei9
Copy link

qifei9 commented Nov 10, 2016

@bhilburn I am also using the vi-mode plugin from Oh-My-Zsh.

@thiagokokada
Copy link

Just to report, I am suffering from this bug too, tried both vi-mode from Oh-My-Zsh and prezto. zsh 5.2.

@agsimeonov
Copy link
Author

agsimeonov commented Nov 16, 2016

@bhilburn I am not using vi-mode distributed with Oh-My-Zsh I have my own personal ZSH dotfile configs which can be found here:
https://github.com/agsimeonov/dotfiles

I am using vi-mode distributed with ZSH directly by setting:

bindkey -v
export KEYTIMEOUT=1

I then enable vi-mode in powerlevel9k using:

POWERLEVEL9K_RIGHT_PROMPT_ELEMENTS=(status background_jobs vcs rbenv virtualenv aws_eb_env vi_mode)
POWERLEVEL9K_VI_INSERT_MODE_STRING="\ue285"
POWERLEVEL9K_VI_COMMAND_MODE_STRING="\ue20c"

@bhilburn
Copy link
Member

Thanks, @agsimeonov. So both the OMZ and built-in ZSH vi_mode are affected.

The only reason I haven't merged #320 is because it significantly slows down the prompt, and I haven't figured out a way to rectify that. If you look at the changeset, it basically requires regenerating the prompt three times for each actual rendering. Especially if you are in a git repository, this can be painfully long.

Would folks be interested in using a special branch that has #320 merged? vi_mode would then work at the expense of draw speed. This might be a good hold-over until we figure out how to fix it permanently.

@polarathene
Copy link

@bhilburn Unrelated to the issue, but related to the commit. That commit was referenced as a fix by removing the methods for the bug experienced here. Would that not also reintroduce that problem?

That said, the issue I linked to I've read can be avoided by including ZSH_AUTOSUGGEST_CLEAR_WIDGETS+=reset-prompt into the .zshrc after sourcing autosuggest. Just thought it was worth mentioning as those users may wonder why their prompt/theme appears to have regressed if that commit gets reverted.

@dritter dritter mentioned this issue Nov 23, 2016
27 tasks
@Ferenc-
Copy link

Ferenc- commented Nov 24, 2016

@bhilburn "Would folks be interested in using a special branch that has #320 merged?"

  • I personally went back back to v0.4.0, and I think most of the users can do the same or revert until the permanent fix.

@FliiFe
Copy link

FliiFe commented Dec 18, 2016

So, what is the status of this issue ? And, what is the preferred fix ? Revert to v0.4.0, or use @agsimeonov's snippet ?

Is there any fix in sight ?

@dritter
Copy link
Member

dritter commented Dec 18, 2016

@FliiFe I am looking forward to finish #344 in the near future. There is a fix included for the vi_mode segment.

@FliiFe
Copy link

FliiFe commented Dec 18, 2016

@dritter Great ! I used the above snippet (mixed it with what I already had). Works fine, I did not notice any slowdown, but looks kinda hacky.

@eullerborges
Copy link

I've tried the fix, but it doesn't work on my oh-my-zsh installation.

@dritter
Copy link
Member

dritter commented Jan 12, 2017

@eullerborges Do you mean, you've tried the PR #344 ? If yes, what exactly did not work? Just the vi_mode segment, or the whole prompt?

@eullerborges
Copy link

@dritter Sorry, I hadn't tested the push request (I had to learn how to pull it). Vi_mode works perfectly on that push request. Sorry for the long response time. It's looking great so far, although I get some flickering on new lines. Thanks a lot!

@IQubic
Copy link

IQubic commented Feb 27, 2017

Is there a way that I can download and test this PR?

@dritter
Copy link
Member

dritter commented Feb 27, 2017

@IQubic It depends how you installed powerlevel9k. If you did install it via git, then you need to go into that installation dir and do:

  1. git remote add dritter https://github.com/dritter/powerlevel9k.git or git remote add dritter [email protected]:dritter/powerlevel9k.git, if you use SSH
  2. git fetch dritter
  3. git checkout -t dritter/async_all_the_segments
  4. source ~/.zshrc

That should do it.

To provide a little more background information:
The vi_mode segment redraws the prompt every time you switch the input mode. Before #344 that means that every segment gets re-evaluated. In #344 all segments are rendered asynchronously, which means they write a cache file with their information. Now, when you change the input mode, only the vi_mode segment gets re-evaluated and renews its cache file, all other segments get drawn by the information of their cache files.

@IQubic
Copy link

IQubic commented Feb 27, 2017

@dritter: When I try to pull that remote branch of yours, I get this error:

fatal: Cannot update paths and switch to branch 'async_all_the_segments' at the same time.
Did you intend to checkout 'dritter/async_all_the_segments' which can not be resolved as commit?

You know what's up with that?

@dritter
Copy link
Member

dritter commented Feb 27, 2017

@IQubic Yes, sorry. I forgot one step. Before you can switch to that branch, you need to fetch the latest stuff from my remote.. I updated the tutorial.

@IQubic
Copy link

IQubic commented Feb 28, 2017

Ah, now it works. And it's quite snappy at that.

@gone
Copy link

gone commented Mar 3, 2017

hey @dritter as a heads up w/ that branch and zpresto powerline breaks :(

@dritter
Copy link
Member

dritter commented Mar 3, 2017

Thx for letting me know @gone
This requires some more conversation. Could you open a new Issue, and describe there your environment and what exactly does not work (eventually with a screenshot). That would be helpful!

@voiprodrigo
Copy link

voiprodrigo commented Mar 9, 2017

I tried the branch mentioned above due, and my experience was that it became noticeably slower and flickery. Just FYI.

(thanks for powerlevel9k)

@dritter
Copy link
Member

dritter commented Mar 9, 2017

@voiprodrigo Thanks for letting me know. As performance is always a matter of perception, could you be more specific about what is slow and your setup?

Regarding your setup

  • What is your ZSH version?
  • Do you have an SSD?

Regarding the performance

  • Is it noticeably slower until the first segment gets rendered?
  • Or do you mean that it is noticeably slower until the whole theme finishes rendering?

Btw. I will add a debug script that can visualize the rendering times in the next couple of days. It would be super cool if you could run that. :)

About the flickering

Yes, this is the trade-off we have to do. Because there are some segments that might take some time until they gathered all necessary information to get rendered, we had to split up the rendering process so that each segment gets rendered individually. This process has some overhead (e.g. more disk I/O) That causes the flickering..

@voiprodrigo
Copy link

zsh 5.2 (x86_64-apple-darwin16.4.0) (installed with brew)
Macbook Pro Mid-2015 2,8Ghz i7, 16GB RAM/512GB SSD
iTerm 2 latest

I meant until the whole theme finishes rendering. After some more testing, I would revise the "noticeably" to "slightly", to be honest. And it depends where you are (or in other words, which segments are active in any given path). For example It actually seems to help a bit when git segment is active, but slightly slower otherwise.

I'll run that debug script, no problem!

@dritter
Copy link
Member

dritter commented Mar 9, 2017

For example It actually seems to help a bit when git segment is active, but slightly slower otherwise.

Yep. As said, compared to the current 0.6.0 version, it does significantly more. But it shows its advantage in the extremes, e.g. when single segments are very slow. For instance, I have a git repo with 500.000 untracked files for testing purposes. On 0.6.0 what happens is that the entire prompt is not usable and the theme gets not rendered for almost a minute. On this branch, the prompt is usable, the theme gets rendered almost complete (except for the VCS segment that gets rendered when all informations are available).
See my screencasts in #344 (comment)

@dritter
Copy link
Member

dritter commented Mar 14, 2017

@voiprodrigo If you update your checkout, you'll get segment timings as promised. And I added debug/segment_timings.zsh, so that you can view how long it took to render the segments.

The best way to use it:

  1. Go into a directory, where the prompt is especially slow
  2. Do an echo $$ to get the PID
  3. Open a new shell, and navigate to the powerlevel9k directory
  4. Execute debug/segment_timings.zsh with the PID

(Sorry for the bad usability)

In my case (in my huge repo) it looks like this (ignore the wrong name):
bildschirmfoto 2017-03-14 um 22 57 26
You can see, that it took almost 25 seconds to render my VCS segment. But that is all right, because the prompt is usable in the meantime. ;)

@bhilburn
Copy link
Member

bhilburn commented Apr 4, 2017

Nice work, @dritter! 👍

@jfmercer
Copy link

I can confirm this issue on macOS Sierra 10.12.6, zsh version 5.3.1. I am not using a zsh framework. Let me know if I can provide any further debugging info.

@twbecker
Copy link

twbecker commented Jan 8, 2018

AFAICT this is just due to P9K removing the zle-keymap-select widget. I was able to make the indicator work by adding the following:

zle-keymap-select () {
	zle reset-prompt
	zle -R
}

zle -N zle-keymap-select

From what I understand, this only incurs the extra prompt evaluation when the keymap changes. Is there a reason we can't add just this part back?

@bhilburn
Copy link
Member

bhilburn commented Oct 8, 2018

This has been fixed since the merge of #740, and is continuing to get more changes in the next branch as part of the v0.7.0 codebase. Going to close this issue out, as it's become one of those long-lived zombie issues that no longer really reflects the current state of the code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests