git: No-op when trying to commit with nothing staged or no commit message (#23491)

Cole Miller created 11 months ago

The buttons are disabled in this case, but users can still trigger a
commit via the actions directly, and an error toast seems a bit loud for
this.

cc @iamnbutler 

Release Notes:

- N/A

Change summary

crates/git_ui/src/git_panel.rs | 36 ++++++++++------------
crates/project/src/git.rs      | 57 ++++++++++++++++++-----------------
2 files changed, 47 insertions(+), 46 deletions(-)

Detailed changes

crates/git_ui/src/git_panel.rs

@@ -549,11 +549,10 @@ impl GitPanel {
         let Some(active_repository) = self.active_repository.as_ref() else {
             return;
         };
-        if let Err(e) = active_repository.commit(self.err_sender.clone(), cx) {
-            self.show_err_toast("commit error", e, cx);
-        };
-        self.commit_editor
-            .update(cx, |editor, cx| editor.set_text("", cx));
+        if !active_repository.can_commit(false, cx) {
+            return;
+        }
+        active_repository.commit(self.err_sender.clone(), cx);
     }
 
     /// Commit all changes, regardless of whether they are staged or not
@@ -561,11 +560,10 @@ impl GitPanel {
         let Some(active_repository) = self.active_repository.as_ref() else {
             return;
         };
-        if let Err(e) = active_repository.commit_all(self.err_sender.clone(), cx) {
-            self.show_err_toast("commit all error", e, cx);
-        };
-        self.commit_editor
-            .update(cx, |editor, cx| editor.set_text("", cx));
+        if !active_repository.can_commit(true, cx) {
+            return;
+        }
+        active_repository.commit_all(self.err_sender.clone(), cx);
     }
 
     fn fill_co_authors(&mut self, _: &FillCoAuthors, cx: &mut ViewContext<Self>) {
@@ -906,15 +904,15 @@ impl GitPanel {
     pub fn render_commit_editor(&self, cx: &ViewContext<Self>) -> impl IntoElement {
         let editor = self.commit_editor.clone();
         let editor_focus_handle = editor.read(cx).focus_handle(cx).clone();
-        let (can_commit, can_commit_all) = self.active_repository.as_ref().map_or_else(
-            || (false, false),
-            |active_repository| {
-                (
-                    active_repository.can_commit(false, cx),
-                    active_repository.can_commit(true, cx),
-                )
-            },
-        );
+        let (can_commit, can_commit_all) =
+            self.active_repository
+                .as_ref()
+                .map_or((false, false), |active_repository| {
+                    (
+                        active_repository.can_commit(false, cx),
+                        active_repository.can_commit(true, cx),
+                    )
+                });
 
         let focus_handle_1 = self.focus_handle(cx).clone();
         let focus_handle_2 = self.focus_handle(cx).clone();

crates/project/src/git.rs

@@ -314,32 +314,28 @@ impl RepositoryHandle {
             && (commit_all || self.have_staged_changes());
     }
 
-    pub fn commit(
-        &self,
-        err_sender: mpsc::Sender<anyhow::Error>,
-        cx: &mut AppContext,
-    ) -> anyhow::Result<()> {
-        if !self.can_commit(false, cx) {
-            return Err(anyhow!("Unable to commit"));
-        }
+    pub fn commit(&self, mut err_sender: mpsc::Sender<anyhow::Error>, cx: &mut AppContext) {
         let message = self.commit_message.read(cx).as_rope().clone();
-        self.update_sender
-            .unbounded_send((Message::Commit(self.git_repo.clone(), message), err_sender))
-            .map_err(|_| anyhow!("Failed to submit commit operation"))?;
+        let result = self.update_sender.unbounded_send((
+            Message::Commit(self.git_repo.clone(), message),
+            err_sender.clone(),
+        ));
+        if result.is_err() {
+            cx.spawn(|_| async move {
+                err_sender
+                    .send(anyhow!("Failed to submit commit operation"))
+                    .await
+                    .ok();
+            })
+            .detach();
+            return;
+        }
         self.commit_message.update(cx, |commit_message, cx| {
             commit_message.set_text("", cx);
         });
-        Ok(())
     }
 
-    pub fn commit_all(
-        &self,
-        err_sender: mpsc::Sender<anyhow::Error>,
-        cx: &mut AppContext,
-    ) -> anyhow::Result<()> {
-        if !self.can_commit(true, cx) {
-            return Err(anyhow!("Unable to commit"));
-        }
+    pub fn commit_all(&self, mut err_sender: mpsc::Sender<anyhow::Error>, cx: &mut AppContext) {
         let to_stage = self
             .repository_entry
             .status()
@@ -347,15 +343,22 @@ impl RepositoryHandle {
             .map(|entry| entry.repo_path.clone())
             .collect::<Vec<_>>();
         let message = self.commit_message.read(cx).as_rope().clone();
-        self.update_sender
-            .unbounded_send((
-                Message::StageAndCommit(self.git_repo.clone(), message, to_stage),
-                err_sender,
-            ))
-            .map_err(|_| anyhow!("Failed to submit commit operation"))?;
+        let result = self.update_sender.unbounded_send((
+            Message::StageAndCommit(self.git_repo.clone(), message, to_stage),
+            err_sender.clone(),
+        ));
+        if result.is_err() {
+            cx.spawn(|_| async move {
+                err_sender
+                    .send(anyhow!("Failed to submit commit all operation"))
+                    .await
+                    .ok();
+            })
+            .detach();
+            return;
+        }
         self.commit_message.update(cx, |commit_message, cx| {
             commit_message.set_text("", cx);
         });
-        Ok(())
     }
 }