show error instead of app close when staging fails (#108)

update changelog
extrawurst · Jun 3, 2020 · bec1fb9 · bec1fb9
1 parent 415d511
commit bec1fb9
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `arrow-up`/`down` on bottom/top of status file list switches focus ([#105](https://github.com/extrawurst/gitui/issues/105))
 - New `Stage all [a]`/`Unstage all [a]` in changes lists ([#82](https://github.com/extrawurst/gitui/issues/82))
 
+### Fixed
+- app closes when staging invalid file/path ([#108](https://github.com/extrawurst/gitui/issues/108))
+
 ## [0.5.0] - 2020-06-01
 
 ### Changed

diff --git a/asyncgit/src/sync/utils.rs b/asyncgit/src/sync/utils.rs
@@ -130,7 +130,9 @@ mod tests {
     use super::*;
     use crate::sync::{
         status::{get_status, StatusType},
-        tests::{get_statuses, repo_init, repo_init_empty},
+        tests::{
+            debug_cmd_print, get_statuses, repo_init, repo_init_empty,
+        },
     };
     use std::{
         fs::{self, remove_file, File},
@@ -284,4 +286,34 @@ mod tests {
         assert_eq!(status_count(StatusType::WorkingDir), 0);
         assert_eq!(status_count(StatusType::Stage), 1);
     }
+
+    // see https://github.com/extrawurst/gitui/issues/108
+    #[test]
+    fn test_staging_sub_git_folder() -> Result<()> {
+        let (_td, repo) = repo_init().unwrap();
+        let root = repo.path().parent().unwrap();
+        let repo_path = root.as_os_str().to_str().unwrap();
+
+        let status_count = |s: StatusType| -> usize {
+            get_status(repo_path, s).unwrap().len()
+        };
+
+        let sub = &root.join("sub");
+
+        fs::create_dir_all(sub)?;
+
+        debug_cmd_print(sub.to_str().unwrap(), "git init subgit");
+
+        File::create(sub.join("subgit/foo.txt"))
+            .unwrap()
+            .write_all(b"content")
+            .unwrap();
+
+        assert_eq!(status_count(StatusType::WorkingDir), 1);
+
+        //expect to fail
+        assert!(stage_add_all(repo_path, "sub").is_err());
+
+        Ok(())
+    }
 }
diff --git a/src/components/changes.rs b/src/components/changes.rs
@@ -17,6 +17,22 @@ use std::path::Path;
 use strings::commands;
 use tui::{backend::Backend, layout::Rect, Frame};
 
+/// macro to simplify running code that might return Err.
+/// It will show a popup in that case
+#[macro_export]
+macro_rules! try_or_popup {
+    ($self:ident, $msg:literal, $e:expr) => {
+        if let Err(err) = $e {
+            $self.queue.borrow_mut().push_back(
+                InternalEvent::ShowErrorMsg(format!(
+                    "{}\n{}",
+                    $msg, err
+                )),
+            );
+        }
+    };
+}
+
 ///
 pub struct ChangesComponent {
     files: FileTreeComponent,
@@ -241,19 +257,26 @@ impl Component for ChangesComponent {
                         Ok(true)
                     }
                     keys::STATUS_STAGE_FILE => {
-                        if self.index_add_remove()? {
-                            self.queue.borrow_mut().push_back(
-                                InternalEvent::Update(
-                                    NeedsUpdate::ALL,
-                                ),
-                            );
-                        }
+                        try_or_popup!(
+                            self,
+                            "staging error:",
+                            self.index_add_remove()
+                        );
+
+                        self.queue.borrow_mut().push_back(
+                            InternalEvent::Update(NeedsUpdate::ALL),
+                        );
+
                         Ok(true)
                     }
 
                     keys::STATUS_STAGE_ALL if !self.is_empty() => {
                         if self.is_working_dir {
-                            self.index_add_all()?;
+                            try_or_popup!(
+                                self,
+                                "staging error:",
+                                self.index_add_all()
+                            );
                         } else {
                             self.stage_remove_all()?;
                         }