-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
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. |
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. |
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... |
Fixes #1795, maybe #1493
Proposed Changes
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