gitu icon indicating copy to clipboard operation
gitu copied to clipboard

feat: ctrl-c to quit the app

Open bloznelis opened this issue 1 year ago • 5 comments

Resolves #258.

Firstly, thanks for this - I love magit and command line, so this feels like a perfect tool for me. As a thank you, I figured I will contribute a small ticket for you.

Now ctrl-c will exit the app bypassing any default or custom bindings. If you want to tweak the logic (for example exit only when the amount of screens is 1 i.e. root-level) - add more conditions to is_system_quit function.

bloznelis avatar Dec 26 '24 12:12 bloznelis

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 88.41%. Comparing base (0f02879) to head (a45e9fc). :warning: Report is 181 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
+ Coverage   88.40%   88.41%   +0.01%     
==========================================
  Files          62       62              
  Lines        5578     5583       +5     
==========================================
+ Hits         4931     4936       +5     
  Misses        647      647              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Dec 26 '24 13:12 codecov[bot]

Hi! This is great :)

I wonder if someone would ever want this to be configurable. 🤔

Perhaps it wouldn't hurt, what do you think?

In the case, I believe there's a module "config.rs", as well as a "default_config.toml", where other stuff is configured.

Currently on vacation so could include it in a release when I'm back home (February).

altsem avatar Dec 26 '24 13:12 altsem

I wonder if someone would ever want this to be configurable. 🤔

Unlikely (?).

Now that I gave this a second though, I think that the initial implementation I made is a bit naive. It wouldn't work if for some reason update hangs/takes too long and the user wants to kill the app, because it would need to wait for previous step to finish before handling the ctrl-c.

A better way would be to react on signals. So in the case of SIGINT (produced by ctrl-c), we would interrupt and close the app without waiting for update to resolve.

bloznelis avatar Dec 30 '24 15:12 bloznelis

Probably should happen somewhere around here. If we receive the SIGINT we break the while loop.

bloznelis avatar Dec 30 '24 15:12 bloznelis

Yea that'd make sense :)

If there's a pending external command running, perhaps the signal should be propagated to it.

I can't recall if Ctrl-C produces a signal when terminal raw-mode is enabled, might be a gotcha.

altsem avatar Dec 30 '24 15:12 altsem