Closed altsem closed 4 months ago
Attention: Patch coverage is 87.12871%
with 26 lines
in your changes are missing coverage. Please review.
Project coverage is 88.21%. Comparing base (
6fa457f
) to head (23eddef
).
Files | Patch % | Lines |
---|---|---|
src/state.rs | 85.27% | 19 Missing :warning: |
src/lib.rs | 0.00% | 7 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This is looking pretty good! I slammed my ping horribly high with a fun command I just discovered: sudo tc qdisc add dev wlan0 root netem delay 500ms
(https://www.baeldung.com/linux/network-failures-simulation) and it behaved as expected, with the one "running" message at the bottom, and it added that error message when I tried to run another command whilst that one was in progress!
There are some crazy things that would maybe work, like trying to stage a file whilst waiting for a fetch, but I don't know if I want to discover weird race conditions in git right now... I think this is pretty good with nothing freezing on those network commands anymore!
Even if I can't stage files or patches whilst waiting on a network command, I now get a nice message why (a command is already running) and the UI stays responsive! Great (and quick) work!
From the code perspective, I think it's probably not a real issue, but if you want to avoid that polling every 100ms, you might be able to use something like this: https://crates.io/crates/async-process
I've not actually used it myself before, but it looks super popular on crates.io and will use Rust's async await that I think in most cases avoids polling and sets up event listeners in the background!
If that looks like a mess to integrate though, then I think this is more than fine as is!
(Looking closer, I think you'd need to also switch to listening to those crossterm events from an async EventStream
, then join
the event-stream and pending command futures — essentially waiting for whichever of the two comes first. But it looks like using that EventStream
also means pulling in something like async-std
or tokio
, and that might be more work than is worth it!)
Git does keep some sort of lock-file I discovered. Perhaps its fine. Would need to keep track of multiple running processes. Another caveat I found now, is that you can't open a hunk with a text editor while it is e.g. fetching.
I had a go at turning things async before. Listening to those crossterm events async and such. I think my PoC turned garbage, it's probably doable and nice. Might try it again in the future :+1:
I'll go ahead and merge it and close the issue. Ty for the help testing and researching! :)
closes: #97
Now fetch/pull/push can run async. Still only one git command (non-reads) can be run at a time. The popup will linger as long as its running, and show on-top of any menu too.
Running fetch as well as viewing the log menu:![image](https://github.com/altsem/gitu/assets/3618477/1b5ae7c6-445c-4750-bb46-bee6e0f05106)
Attempting to run a second![image](https://github.com/altsem/gitu/assets/3618477/ee8078cb-e091-4ec3-8b82-96effe4bc364)
git fetch --all
: