From c196644191029bc4c378d52603c539d4ef6f79a7 Mon Sep 17 00:00:00 2001
From: Lorenzo Bertin Salvador <lorenzobs@usp.br>
Date: Sat, 15 Mar 2025 18:36:29 -0300
Subject: [PATCH] fix: ensure loading_screen! generated thread joins before
 exiting patch-hub

This commit fixes the cursor becoming invisible when crash occur during loading.
The problem was caused by not joining the thread generated by loading_screen!
before handling the error (e.g exiting patch-hub).
The solution was achieved by the following changed:
1. Made the closure that's passed to the loading_screen! macro return a Result
instead of (), enabling proper handling of the result after joining the loading
thread.
2. Changed the closures to avoid propagating errors using the ? operator, as
in Rust, "?" and "return" are applied to the outer function instead of the
closure itself.

Closes: #74

Signed-off-by: Lorenzo Bertin Salvador <lorenzobs@usp.br>
---
 src/handler.rs            |  4 ++--
 src/handler/bookmarked.rs |  7 +++++--
 src/handler/latest.rs     |  9 ++++++---
 src/handler/mail_list.rs  | 13 +++++++++----
 src/utils.rs              | 15 ++++++++++++---
 5 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/handler.rs b/src/handler.rs
index 97597e58..ef003201 100644
--- a/src/handler.rs
+++ b/src/handler.rs
@@ -72,7 +72,7 @@ where
             if app.mailing_list_selection.mailing_lists.is_empty() {
                 terminal = loading_screen! {
                     terminal, "Fetching mailing lists" => {
-                        app.mailing_list_selection.refresh_available_mailing_lists()?;
+                        app.mailing_list_selection.refresh_available_mailing_lists()
                     }
                 };
             }
@@ -84,7 +84,7 @@ where
                 terminal = loading_screen! {
                     terminal,
                     format!("Fetching patchsets from {}", target_list) => {
-                        patchsets_state.fetch_current_page()?;
+                        patchsets_state.fetch_current_page()
                     }
                 };
 
diff --git a/src/handler/bookmarked.rs b/src/handler/bookmarked.rs
index 2896540e..ab3ec4b8 100644
--- a/src/handler/bookmarked.rs
+++ b/src/handler/bookmarked.rs
@@ -38,8 +38,11 @@ where
             terminal = loading_screen! {
                 terminal,
                 "Loading patchset" => {
-                    app.init_details_actions()?;
-                    app.set_current_screen(CurrentScreen::PatchsetDetails);
+                    let result = app.init_details_actions();
+                    if result.is_ok() {
+                        app.set_current_screen(CurrentScreen::PatchsetDetails);
+                    }
+                    result
                 }
             };
         }
diff --git a/src/handler/latest.rs b/src/handler/latest.rs
index 42102aac..6a3b1e66 100644
--- a/src/handler/latest.rs
+++ b/src/handler/latest.rs
@@ -42,7 +42,7 @@ where
                 terminal,
                 format!("Fetching patchsets from {}", list_name) => {
                     latest_patchsets.increment_page();
-                    latest_patchsets.fetch_current_page()?;
+                    latest_patchsets.fetch_current_page()
                 }
             };
         }
@@ -53,8 +53,11 @@ where
             terminal = loading_screen! {
                 terminal,
                 "Loading patchset" => {
-                    app.init_details_actions()?;
-                    app.set_current_screen(CurrentScreen::PatchsetDetails);
+                    let result = app.init_details_actions();
+                    if result.is_ok() {
+                        app.set_current_screen(CurrentScreen::PatchsetDetails);
+                    }
+                    result
                 }
             };
         }
diff --git a/src/handler/mail_list.rs b/src/handler/mail_list.rs
index ecfd0396..d8797a09 100644
--- a/src/handler/mail_list.rs
+++ b/src/handler/mail_list.rs
@@ -37,9 +37,14 @@ where
                 terminal = loading_screen! {
                     terminal,
                     format!("Fetching patchsets from {}", list_name) => {
-                        app.latest_patchsets.as_mut().unwrap().fetch_current_page()?;
-                        app.mailing_list_selection.clear_target_list();
-                        app.set_current_screen(CurrentScreen::LatestPatchsets);
+                        let result =
+                        app.latest_patchsets.as_mut().unwrap()
+                        .fetch_current_page();
+                        if result.is_ok() {
+                            app.mailing_list_selection.clear_target_list();
+                            app.set_current_screen(CurrentScreen::LatestPatchsets);
+                        }
+                        result
                     }
                 };
             }
@@ -49,7 +54,7 @@ where
                 terminal,
                 "Refreshing lists" => {
                     app.mailing_list_selection
-                        .refresh_available_mailing_lists()?;
+                        .refresh_available_mailing_lists()
                 }
             };
         }
diff --git a/src/utils.rs b/src/utils.rs
index 2497d34f..cb5fb758 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -85,7 +85,10 @@ pub fn binary_exists(binary: &str) -> bool {
 ///
 /// When the execution finishes, the macro will return the terminal.
 ///
-/// Important to notice that the code block will run in the same scope as the rest of the macro
+/// Important to notice that the code block will run in the same scope as the rest of the macro.
+/// Be aware that in Rust, when using `?` or `return` inside a closure, they apply to the outer function,
+/// not the closure itself. This can lead to unexpected behavior if you expect the closure to handle
+/// errors or return values independently of the enclosing function.
 ///
 /// # Example
 /// ```rust norun
@@ -109,11 +112,17 @@ macro_rules! loading_screen {
                 terminal
             });
 
-            $inst;
+            // we have to sleep so the loading thread completes at least one render
+            std::thread::sleep(std::time::Duration::from_millis(200));
+            let inst_result = $inst;
 
             loading.store(false, std::sync::atomic::Ordering::Relaxed);
 
-            handle.join().unwrap()
+            let terminal = handle.join().unwrap();
+
+            inst_result?;
+
+            terminal
         }
     };
 }