-
-
Notifications
You must be signed in to change notification settings - Fork 413
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
Calispel: Proper concurrency in Nyxt at last! #1047
Conversation
Seems that running the startup-function in a separate thread is a remnent from an draft version and it's no longer needed.
ChanL's SELECT statement is non-blocking so it was unusable.
This branch starts properly for me. |
Yay!
|
Branch appears to work for me too! woohoo! First bug report: Reduce to buffer command will activate, but will not show the minibuffer. I have to cancel the command, try to reactivate it, and then sometimes it shows. I am not sure what the sequence of events are to get it work. |
It's the same issue with all chained minibuffers.
Try:
…--8<---------------cut here---------------start------------->8---
- M-x open-file RET
--8<---------------cut here---------------end--------------->8---
The current workaround: Bind the command ;)
This issue is also on master, but the race condition happens way less
often.
It's actually good we have Calispel now so we can more reliable test if
we've fixed this issue properly.
My guess is that `hide` is run too late and hides the new minibuffer.
It's probably easy to fix this, I'll work on it later.
|
Ah, yes! I remember we had discussed a similar bug at some point in the recent past. |
Perhaps it was the very same bug. |
To clarify, this race condition only occurs _frequently_ (about 3 times
out of 4 for me) with Calispel.
|
Before merging:
|
|
There was a race condition between prompt-minibuffer and hide. But it seems that we can't use locks in prompt-minibuffer, possibly because it runs on the GTK thread. Anyways, the fix is easy: always hide the minibuffer in return-selection _before_ returning the result.
|
All done. Has anyone experienced any other issues with this patch? |
Any other review? Did anyone have the chance to look at the code? |
@jmercouris: Can you test this on macOS? Thanks! |
Works on macOS! Merge when ready! |
Merged with c0b6b9e |
I've replaced the ChanL library with Calispel which seems much, much better!
See CodyReichert/awesome-cl#290 (comment) for a discussion.
I took the opportunity to (hopefully) fix the startup race condition. @aartaka Can you make sure this branch starts properly for you?
Finally, I've made use of
fair-alt
instead of the interrupt-channel juggling business which was ugly and hard to understand.This
fair-alt
will become essential in the new minibuffer library.