Powerlevel9k: [Regression: 0.6.3] Visual artefacts with quick consecutive commands

[protist]

I just upgraded from v0.6.2 to v0.6.3, and I've noticed some visual artefacts when I type rapidly after the previous command. For example, in v0.6.3, I type l (an alias for ls) twice quickly.

 /path > l
foo bar
l /path >l

You can see the extra l at the beginning of the second prompt, and the shift right of the prompt. This is a fairly facile example, but the artefacts sometimes result in very garbled text, forcing me to cancel the current command.

I can make the above occur ~50% of the time if I alternate between l and Enter as fast as possible. In contrast, I downgraded to v0.6.2, and can no longer replicate this. Perhaps 20% of the time I can type it fast enough that the prompt isn't drawn yet, but it writes on its own line, and doesn't cause artefacts.

 /path > l
foo bar
l
 /path > l

If you cannot replicate this issue, I'm happy to test a few specific commits, if you can point me in the right direction. Cheers.

[protist]

I've just found a much more easily reproducible example, which is presumably related. I can replicate it by creating an output that lacks a trailing newline, e.g. printf foo.

In powerlevel9k v0.6.2:

This is similar to how vanilla zsh prints out strings with no trailing newlines, with a colour-inverted %, followed by a literal newline. However, in powerlevel9k v0.6.3:

Note the final character of the right prompt moving to the next line, and the cursor sitting on top of this character. If you then type something, and then delete some of it, you can get artefacts with the cursor not synchronising with what it's actually editing.

[protist]

I decided to learn git bisect to find the offending commit. (Wow, that's a great tool!)

I managed to trace it down to f46c1bc. With this commit, I get the erroneous behaviour above, both with typing commands quickly, and with printf. With the previous commit (6f4c09a), both situations are fine. FWIW I ran into a bit of problem with numerous commits giving me the following literal prompt.

$(build_left_prompt)               $RPROMPT_PREFIX$(build_right_prompt)$reset_color$RPROMPT_SUFFIX

I also tested removing the added lines in the offending commit from powerlevel9k v0.6.3, but I just ended up with the same odd literal prompt as above.

[Gee19]

I am also seeing strange output in my terminal as of recently.

^C $RPROMPT_PREFIX$(build_right_prompt)

[bhilburn]

@Gee19 - If you check out 6f4c09ab99d13db9e48959ef0ed37c83c845b1f0, per @protist's comment above, do you still see the issue?

@protist - Nice debugging work with bisect! That's really helpful.

I was able to reproduce what you are seeing with the simple test you provided, printf foo, and if I go back to v0.6.2 I also see the problem go away.

So f46c1bc11b4bd41be1f6feac61d1944891aa6830 is part of the fix for the pw3nage exploit, so just backing it out isn't an option. It would probably be good to get @belak to weigh-in with his expertise.

@dritter @wadkar - in case they have guidance, as well.

[bhilburn]

@protist - Can you try out #513?

[bhilburn]

Thanks to @belak's quick coding, this is now fixed at the HEAD of master and next =)

[wadkar]

Awesome! One of the many reasons I love PL9K and use it daily - issues get fixed before you find time to open them on GH! 😆

[protist]

Brilliant. Thanks for the blazingly quick fix. Works perfectly!

[Gee19]

I can still reproduce this if I am within a virtualenv.

$RPROMPT_PREFIX$(build_right_prompt)

[bhilburn]

@Gee19 - Hrm, okay, that seems strange. To be clear, you don't see it if you aren't in a virtualenv?

[belak]

I'm not able to reproduce when inside a virtualenv. Can you provide any more information about your setup? Also, are you using normal virtualenv or virtualenvwrapper?

[Gee19]

@bhilburn works flawless if no virtualenv is activated

@belak zshrc/dotfile here: https://github.com/Gee19/dotfiles/blob/master/zshrc

using antibody & virtualenvwrapper.

Thanks for the quick responses & help guys.

[bhilburn]

Hm, I can't imagine this is an antibody thing, but I suppose I could be wrong about that.

I'm not a virtualenv user, so I'm not knowledgeable about it. That said, I noticed that you have this in your zshrc:

# virtualenv environments
if [ -f /usr/local/bin/virtualenvwrapper.sh ]; then
    export WORKON_HOME=~/Envs
    mkdir -p $WORKON_HOME
    source /usr/local/bin/virtualenvwrapper.sh
fi

Whereas @belak does not. Any chance that is impacting things? What does that wrapper do?

[belak]

That portion of code is activated by prezto for me, but I do have it.

[belak]

I can't reproduce it by using your dotfiles directly. Are you sure powerlevel9k is up to date?

[Gee19]

Only seems to happen in a git repo, and p9k is up to date :(

http://i.imgur.com/4vEkrZ3.png

[bhilburn]

@Gee19 - Just to be sure, if you checkout v0.6.2, you don't see this problem?

Really strange that this is affected by whether or not you are in a VCS repo.

[dritter]

Okay. I can reproduce this issue on the current master (ecd05e1b679964b216f731db0ef9ce5804dd20d1). It does not appear on v6.0.2 and I neither have to use virtualenv nor be in a git repo. I just keep pressing CTRL-C and it happens.
ZSH settings are identical between master and v6.0.2:

autocd
autopushd
completeinword
extendedhistory
noflowcontrol
histexpiredupsfirst
histignoredups
histignorespace
histverify
incappendhistory
interactive
interactivecomments
longlistjobs
monitor
promptsubst
pushdignoredups
pushdminus
sharehistory
shinstdin
zle

My best guess is that on v6.0.2 the prompt got expanded, before the interrupt can cancel it, and the single quotes in master delay the expansion far enough so that the interrupt cancels it.
The ZSH documentation about 14.3.2 Expansion Rules and Prompt Expansion give no hint about that.

[belak]

I can try to take a closer look in the next few days... That seems like an odd (but still possible) reason so I'm hoping there's a more logical explanation.

[dritter]

Thanks @belak . I am curious about what you will come up with. :)

[belak]

This is definitely an odd issue. I haven't gotten the chance to completely debug it, but I have discovered some oddities.

Firstly: most prompts set PROMPT and RPROMPT in the prompt_setup function because there is no need for them to be set multiple times. However, with powerlevel9k, we allow users to change these values at any time and it is expected that the prompt will update. This was what I initially looked into, but it does not appear to be what is causing this.

So, I believe what @dritter mentioned before makes sense, I just didn't understand it properly. The issue comes because of build_right_prompt being a function, so when you hold ^C (which is how I was reproducing it) the function never completes and therefore zsh just doesn't expand the whole thing. This can be fixed by calculating the return value from build_right_prompt elsewhere and only using it as a variable in RPROMPT.

This shows up more in git repos because build_right_prompt takes longer to run (as it has the additional git checks) and therefore there's a larger window for that function to be killed by the ^C.

I think that if something is added to the docs, it should be added here: http://zsh.sourceforge.net/Doc/Release/Expansion.html#Command-Substitution Maybe a clarification as to what happens when one of the commands in a command substitution gets killed... but that seems like a bit of an edge case so I can understand why it wasn't included in the first place.

tl;dr: This would require a relatively large rewrite to fix properly, but it might be fixed by #344 if PROMPT and RPROMPT only contain variables and async is used to update and display them.

[bhilburn]

Thanks so much for the thorough investigation & report, @belak!

Based on all of the discussion above, I don't think that, realistically, there is anything further for us to do, here. Unless we want to try and catch signals and process them, which would likely interfere with normal emulator operation, I think this is just a case of "if you kill something mid-process, it's not going to finish properly". @dritter`s work will probably improve things, but only because it will protect P9k's logic a bit more from the signals.

Thoughts from others?

[belak]

I think it may make sense to leave this open and take another look after @dritter 's async PR goes in (I'm not sure when that will be, haven't been following it... and the PR is massive)... it would be a bit of churn, but may be worthwhile if it avoids this bug.

This can be avoided by making the PROMPT and RPROMPT made of only variables and setting them in precmd and other hooks. It'll just take a bit of work.