Skip to content

Show cursor on panic #2620

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

Merged
merged 1 commit into from
Apr 18, 2025
Merged

Conversation

naseschwarz
Copy link
Contributor

ratatui::Terminal starts by hiding the cursor. If we panic and abort, that Terminal instance is not dropped, which leaves restoring the cursor state to us.

It changes the following:

  • Shows the terminal cursor if gitui panics

I followed the checklist:

  • I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I added an appropriate item to the changelog
    • left out intentionally for such a minor item

ratatui::Terminal starts by hiding the cursor. If we panic and abort,
that Terminal instance is not dropped, which leaves restoring the cursor
state to us.
@extrawurst
Copy link
Collaborator

I think we should revisit the whole panic handlings. this is a bandaid over the fact that we are not having propper dropping happen which is only the case if we abort instead of panic.

ideally we find out how to handle the rayon panic in a way that we still end the program but run all the drops properly

@extrawurst extrawurst merged commit 9056e5e into gitui-org:master Apr 18, 2025
22 checks passed
extrawurst added a commit that referenced this pull request Apr 18, 2025
@extrawurst
Copy link
Collaborator

@naseschwarz reverted this now again after pushing 9271b41 - i could not produce a case where the cursor was not shown again since we now should be running drops for everything (including terminal) but please have another look

@naseschwarz
Copy link
Contributor Author

naseschwarz commented Apr 18, 2025

Thanks. I actually tested this PR without the rayon handler too, as I was expecting this to be merged sooner or later. On my terminal, alacritty on Linux, the cursor is still hidden, when I do the following:

  1. checkout gitui-org/master (9271b41 is included in my test)
  2. apply patch from below
  3. cargo run
  4. go to tab 2, revlog
  5. look at the diff for the just applied pach

-> panic

-> cursor still hidden

From d8deeb2125b51efa02cde590bfcc9605a5d684c8 Mon Sep 17 00:00:00 2001
From: Naseschwarz <[email protected]>
Date: Wed, 16 Apr 2025 20:22:33 +0200
Subject: [PATCH] Panic on diff view

---
 asyncgit/src/diff.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/asyncgit/src/diff.rs b/asyncgit/src/diff.rs
index 5316e67b..1d98ea00 100644
--- a/asyncgit/src/diff.rs
+++ b/asyncgit/src/diff.rs
@@ -122,6 +122,7 @@ impl AsyncDiff {
 		self.pending.fetch_add(1, Ordering::Relaxed);

 		rayon_core::spawn(move || {
+			panic!();
 			let notify = Self::get_diff_helper(
 				&repo,
 				params,
--
2.49.0

@naseschwarz
Copy link
Contributor Author

Also, grepping through a capture of everything that's written to the output, at least CSI 25h, which would show the cursor, is never printed in master on panic right now:

% grep 25h terminal-log | xxd | wc -l
0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants