Async right prompt

[ayyess]

This pull is the same as #262.

This could be a solution to the lag experienced in #244.
An subprocess writes the prompt to a temporary file (unique to each zsh instance) as each element is loaded. A signal is send to the parent zsh process after a write so that the RPROMPT can be set to the new value. The temporary file is deleted when the zsh exits.

[Tritlo]

Alright, me and @andjscott worked together on this to make it work in most settings (though the credit should go to @andjscott). It uses a temporary file to store the rprompt while it is being built, and then outputs that when it is ready. When the shell exits, this is cleaned up. Makes for a much smoother experience, especially when using slow segments (like #244 and docker_machine for me), and removes the annoying lag after a command is run.

It is configureable by POWERLEVEL9K_RPROMPT_RENDER_ASAP, which controls whether the individual components are rendered as they become ready, or if the whole segment is built before it is displayed, (a matter of preference, and possibly slowness of segments). If this gets merged, I'll document that variable on the Styleizing your prompt wiki page.

[wadkar]

May I suggest a small improvement to the mktemp call?

It would be (a) nice to use a prefix for the temporary file to indicate where it comes from, e.g. mktemp -t powerlevel_rprompt_$$, and (b) check if mktemp succeeded and if it failed, make it fail silently, instead of blowing up the prompt e.g. POWERLEVEL9K_SOCKET="$(mktemp -q -t powerlevel9k_rprompt_$$)"

Overall, this block would like this:

if [[ "$POWERLEVEL9K_DISABLE_RPROMPT" != true ]]; then
  POWERLEVEL_SOCKET="$(mktemp -q -t powerlevel_rprompt_$$)"
  if [ $? -eq 0 ]; then
    powerlevel9k_async() {
      : >! $POWERLEVEL_SOCKET
      build_right_prompt
    }
  else
    POWERLEVEL9K_RPROMPT_RENDER_ASAP=false
  fi
fi

I am sorry I couldn't test my suggestion. I hope you will give it some thought.

[ayyess]

Using a prefix for the file is a good idea. I had to add a template parameter to the call so it looks like mktemp -q -t powerlevel_rprompt_$$_XXX. I hadn't considered mktemp failing and haven't written a solution yet.

[wadkar]

Great!
My comment also provides a possible solution. I hope you will consider it. It basically checks if the last command failed or not. Which works if you do it right after calling mktemp -q -t ....
I am not sure how you would want to handle the scenario where mktemp fails, but I would imagine simply disabling the async rprompt feature is okay'ish approach.

[ayyess]

Another solution would be to use zsh-async, if the user has the package installed, to generate the prompt asynchronously. These changes can be seen at next...andjscott:next2.

[Tritlo]

In line 992

[bhilburn]

Yeah, I spotted it right after I made that comment and deleted the comment. You are very quick at responding to Github notifications, hah! :)

[bhilburn]

Hey @andjscott, @Tritlo - Very cool work. We had discussed trying to find a way to make things asynchronous in a number of other bugs / PRs, and never really settled on a good way. This seems like a pretty good solution.

I left a small comment about where the code goes in the diff.

It doesn't look like this changeset gives the user the ability to not use the async behavior if they prefer to just use the current synchronous behavior. Is that accurate?

[Tritlo]

yes, the current implementation is async only. Making it configurable should be very easy. I'll have a look at it this weekend.

[ayyess]

As Tritlo said, there currently isn't a choice. The choice they get, (the _ASAP variable) is to make the prompt load each element indepenently rather than as a group.

Tbh I now think that the zsh-async approach is better as it only adds async support if the user has the async library installed (progressive enhancement) and keeps the workload in zsh rather than creating temporary files. A negative is that the "ASAP" method isn't implemented.

[bhilburn]

Another item is that while this fixes the issue for the RPROMPT, it doesn't address it for the main PROMPT, which is where most folks have their vcs segment and such. Especially with repos with lots of submodules, the vcs segment can really slow things down.

Do you think the approach you took, here, could also apply to the main prompt?

[ayyess]

Yes, PROMPT should be updatable in the same way

[bhilburn]

@andjscott - Can you build on this PR to add this capability to the main PROMPT, and add a setting that allows users to turn off async completely? I think with those two additional options, this will be a really amazing fix for a lot of people.

[ayyess]

To the current PR (using the temporary file) or the zsh-async version?

[Tritlo]

Just a note: the zsh-asymc method did not work for me, so I think it should be further investigated. Maybe use the temp file for this version, and then submit another pr with zsh-async?

[bhilburn]

@andjscott - Your current method using temp files. We can look into zsh-async after we have some experience, here, but I'm very wary of creating additional dependencies for functionality, generally =)

[ayyess]

I've hacked on async PROMPT and made it optional. Being optional, there are a lot of if branches like

if ASYNC then
    make_segment >> $SOCKET 
else
     make_segment
end

which isn't pretty. The other problem is that there is only one TRAPUSR1 so both prompts get updated when one completes.
I don't want to add more to this PR as I think this approach is messy.

[bhilburn]

Instead of conditionalizing it, I wonder

[bhilburn]

/me sighs

I did not mean to post that or close this bug.

[bhilburn]

Agreed re: ugliness of the conditionals. I wonder if there is a way to combine them / break them out. Will spin on it... thoughts, others?

[bhilburn]

Hey @andjscott - I would really like to get this merged. You and @Tritlo did some really awesome work, and this will be really useful to a lot of people.

After working with the two different versions you created, zsh/async and then the temp file version, I actually like the former quite a bit more. In addition, the code is significantly simpler. I was previously interested in non-zsh/async solutions so that we wouldn't need to add dependencies, but I looked at the project in a bit more detail and we could simply re-distribute the code in powerlevel9k. I just need to specifically call out a license for powerlevel9k (which I should have done ages ago, anyway), and handle a few other things.

Can you push your changeset that uses zsh/async to a branch somewhere and submit a PR for it? It looks like it may still be here, but I would prefer to just use a PR.

[bhilburn]

@andjscott @Tritlo -

Okay, so check it out: I created a new branch and merged the work you had in your next2 branch (referenced in my last message) into it. I then integrated zsh-async into powerlevel9k as a git subtree.

Looking at the docs for zsh-async, it seems to indicate that we need to source the async.zsh file and then call async_init. Unfortunately, it seems that sourcing async.zsh freezes my terminal session somehow. I noticed in this PR that you are actually never invoking it - how did you handle this in your original work?

My progress so far is in the zsh-async-integration branch. If you could pull it & have a look, that would be fantastic.

https://github.com/bhilburn/powerlevel9k/tree/zsh-async-integration

[ayyess]

I noticed in this PR that you are actually never invoking it - how did you handle this in your original work?

zsh-async was installed to my fpath meaning that async_init isn't required

[bhilburn]

@andjscott - Ah, I see.

I don't want to require users to install zsh-async themselves, which means necessarily bundling it with powerlevel9k. I'll contact the dev, @mafredri, to chat about it, too.

[mafredri]

Hi guys, first off, glad you're finding use for zsh-async, makes me happy :)!

Also, as @bhilburn contacted me about this I had a look over the code and have some suggestions with regard to how async is handled. First off, I noticed that using this branch causes my shell to go into a endless loop of redoing all the prompt building operations over and over again (visible with debugging output zsh -x). So I took the liberty to implemented some changes here: https://github.com/mafredri/powerlevel9k/commit/c088376725e3808246a07d179a1527b432ed77ee.

Som benefits here:

• Does not override the users zsh-async if found in $fpath.
• Does not require restarting of async worker, the required parameter is passed as arguments to the function(s)

Negatives:

• Needs the additional symlink in the zsh-async folder, generally I'd recommend moving this symlink to a general functions directory for the project thought, one that is added to $fpath. I see that you already have a functions directory, however, these do not seem to be in autoloadable format and should thus not be in the $fpath.
• In the build left/right prompt functions we must use $@ instead of $1 to catch all the arguments passed to the function. This is due to a current limitation of zsh-async where we can't pass a proper array to a function run within a worker.

---

I also have some general comments/concerns about the code, unrelated to this PR, but I'll voice them here anyway. Up to you if you wish to discard them or consider them :)

• Use of zle-line-{init,finish,select} seems a bit dangerous. They might be overriding user defined zle functions. There should be a nice fix for this in the next version of zsh, but I currently don't have a good suggestion. E.g. zsh-syntax-highlighting accomplishes this by wrapping all user defined zle functions in it's own.
• When using built in zle functions (e.g. zle reset-prompt) I recommend prefixing them with .. The command then looks like zle .reset-prompt and ensures that we're executing the native function, not a user-defined function.
• powerlevel9k_prepare_prompts is run on all these different zle events, which means that we're really doing double the work. For example, precmd and zle-line-init will always run when a new prompt is established.
• I see that the default installation method is to source the prompt, curious why not ln -s prompt.zsh /path/to/fpath/prompt_powerlevel9k_setup. Then it can be loaded through prompt powerlevel9k, assuming autoload -Uz promptinit && promptinit
• script_location="$(dirname $filename)" is not a safe method to find the location of the script, I'd recommend using script_location=${(%):-%N} which should be safe in almost all cases.

Anyway, that's it for my quick observations. I can see there's a lot of 💖 behind this theme, keep up the good work!

[mafredri]

Actually, I should've checked the code above before writing down my thoughts on script_location.

To be honest, I think you could replace a lot of the code from there with:

0=${(%):-%N}
dir=${0:A:h}

This makes $0 trustworthy, :A resolves symlinks, etc and :h gives the folder.

[bhilburn]

Hey @mafredri - Wow! Thanks so much for the fantastic input on using zsh-async and your feedback regarding places we can improve the existing codebase. I also really appreciate you pushing a branch with some of the changes applied so that we can work from it.

This is really great information. I haven't gotten a chance to sit down, yet, and incorporate your comments, but I would like to get it done ASAP - perhaps this weekend. Thanks again for spending the time on this =)