-
Notifications
You must be signed in to change notification settings - Fork 12
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
Interrupt reloads when new files change #158
Interrupt reloads when new files change #158
Conversation
DUX-1244 Interrupt reloads when new files change
This would help things feel a little snappier. Canceling tests would also be helpful, e.g. to implement Maybe we could have separate "cancel reloads" and "cancel tests" switches? Cancelling reloads might be a bit tricky — new files don't get loaded automatically, so we'd need to make sure nothing gets dropped. |
) -> miette::Result<T> { | ||
// See: https://github.com/rust-lang/rust/pull/100737#issuecomment-1445257548 | ||
let mut old_signal_mask = SigSet::empty(); | ||
pthread_sigmask( |
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 turns out that this doesn't just stop ghci
from inheriting SIGINT
from ghciwatch
, it also stops ghci
from receiving SIGINT
whatsoever.
This is a little messier, I guess — when users press Ctrl-C the underlying ghci
session will see that as well — but then ghciwatch
will shut down, so it doesn't matter much in the end.
c065514
to
6961855
Compare
This will give a snappier UX in general. Note that now, many hooks can be canceled or mismatched (e.g. the before-reload hooks can run but the after-reload hooks get canceled).
6961855
to
3fea57a
Compare
tokio::select! { | ||
_ = handle.on_shutdown_requested() => { | ||
// Cancel any in-progress reloads. This releases the lock so we don't block here. | ||
task.abort(); | ||
ghci.lock().await.stop().await.wrap_err("Failed to quit ghci")?; | ||
break; | ||
} | ||
Some(new_event) = receiver.recv() => { |
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.
Might be worth explicitly matching on the Reload
constructor of GhciEvent
so that if you later add more constructors this will become a non-exhaustive pattern match.
This will give a snappier UX in general.
Note that now, many hooks can be canceled or mismatched (e.g. the before-reload hooks can run but the after-reload hooks get canceled). I'm not sure if this will cause issues with Honeycomb (e.g. spans that open but don't close).