Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to an alternative method of async git in the sorin prompt #1805

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

belak
Copy link
Collaborator

@belak belak commented Mar 19, 2020

Fixes #1795, maybe #1493

Proposed Changes

  • Update sorin prompt to use zle callbacks rather than zsh-async

This should fix a number of random, hard to debug issues we've had with zsh-async and may improve Windows support. It doesn't get updates super often either so it may be worth investigating removing it completely if we can.

The code here is based off a simplified version of the code in autosuggestions mentioned here: mafredri/zsh-async#24

Copy link
Collaborator

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't get updates super often either so it may be worth investigating removing it completely if we can.

Which doesn't get updates? zsh-async or zle?

This isn't an area I know much (anything?) about, so not really qualified to review it... all I can do is ask questions...

I vote we hold off on merging this until we get confirmation feedback in those particular issues... the comments so far are not promising, but I'm unclear if that's due to a simple bug in the porting or something deeper in the framework.

But if it does start solving existing problems, then no objections to trying it... worst case is we just revert.

else
# Git target detected, update the git fragment and redisplay the prompt.
_prompt_sorin_git="${_git_target}${_git_post_target}"
zle && zle reset-prompt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So are we already using both zle and zsh-async? It does seem weird that we'd be using two async frameworks...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zle is a zsh built-in and does a ton of stuff related to line editing. For some reason there's also some functionality in there around managing file descriptors for sub-shells.

@belak
Copy link
Collaborator Author

belak commented Mar 23, 2020

Which doesn't get updates? zsh-async or zle

zsh-async. There are people saying reverting to 1.1 fixes the issue but that’s a really old release and there have been a ton of fixes since then.

Yeah, there are obviously some bugs... it was a quick implementation thrown together in an hour and it’s hard for me to debug this because I don’t have a windows machine available at the moment.

@jeffwidman
Copy link
Collaborator

@belak Is this still relevant given that #1810 appears to fix the root issue? Or were there other problems with zsh-async such that you want to continue pursuing this?

@belak
Copy link
Collaborator Author

belak commented Apr 1, 2020

As far as I know, the fix for zsh-async still doesn't fix the zsh hang on exit with windows because (by design) there's a process still running to handle git information. I'm not sure if this is worth pursuing or not.

@jeffwidman
Copy link
Collaborator

Gotcha... yeah, Idk either... Microsoft/windows has been changing a lot to be more linux-friendly, I would probably vote to let this sit a bit and see if they provide easier hooks to do that. This would be years in the future, so if someone urgently wanted it and came up with a way, I wouldn't be opposed, but otherwise not something I'd pursue...

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

Successfully merging this pull request may close these issues.

async_job:zpty:12 no such pty command: prompt_sorin
3 participants