ability to delete remote branch (#838)

* added ability to delete remote branch (closes #622)

asyncgit/src/push.rs

@@ -22,6 +22,8 @@ pub struct PushRequest {
     ///
     pub force: bool,
     ///
+    pub delete: bool,
+    ///
     pub basic_credential: Option<BasicAuthCredential>,
 }
 
@@ -98,6 +100,7 @@ impl AsyncPush {
                 params.remote.as_str(),
                 params.branch.as_str(),
                 params.force,
+                params.delete,
                 params.basic_credential.clone(),
                 Some(progress_sender.clone()),
             );

asyncgit/src/sync/branch/merge_commit.rs

@@ -134,6 +134,7 @@ mod test {
             "origin",
             "master",
             false,
+            false,
             None,
             None,
         )
@@ -151,7 +152,7 @@ mod test {
 
         //push should fail since origin diverged
         assert!(push(
-            clone2_dir, "origin", "master", false, None, None,
+            clone2_dir, "origin", "master", false, false, None, None,
         )
         .is_err());
 
@@ -222,6 +223,7 @@ mod test {
             "origin",
             "master",
             false,
+            false,
             None,
             None,
         )

asyncgit/src/sync/branch/merge_ff.rs

@@ -80,6 +80,7 @@ pub mod test {
             "origin",
             "master",
             false,
+            false,
             None,
             None,
         )
@@ -103,6 +104,7 @@ pub mod test {
             "origin",
             "master",
             false,
+            false,
             None,
             None,
         )

asyncgit/src/sync/branch/merge_rebase.rs

@@ -107,8 +107,10 @@ mod test {
 
         assert_eq!(clone1.head_detached().unwrap(), false);
 
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         assert_eq!(clone1.head_detached().unwrap(), false);
 
@@ -129,8 +131,10 @@ mod test {
 
         assert_eq!(clone2.head_detached().unwrap(), false);
 
-        push(clone2_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone2_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         assert_eq!(clone2.head_detached().unwrap(), false);
 
@@ -201,8 +205,10 @@ mod test {
             Time::new(0, 0),
         );
 
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         // clone2
 
@@ -219,8 +225,10 @@ mod test {
             Time::new(1, 0),
         );
 
-        push(clone2_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone2_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         // clone1
 
@@ -278,8 +286,10 @@ mod test {
         let _commit1 =
             write_commit_file(&clone1, "test.txt", "test", "commit1");
 
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         // clone2
 
@@ -295,8 +305,10 @@ mod test {
             "commit2",
         );
 
-        push(clone2_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone2_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         // clone1
 

asyncgit/src/sync/branch/mod.rs

@@ -471,7 +471,8 @@ mod tests_branches {
 
         write_commit_file(&repo, "f1.txt", "foo", "c1");
         rename_branch(dir, "refs/heads/master", branch_name).unwrap();
-        push(dir, "origin", branch_name, false, None, None).unwrap();
+        push(dir, "origin", branch_name, false, false, None, None)
+            .unwrap();
     }
 
     #[test]
@@ -680,14 +681,17 @@ mod test_remote_branches {
 
         write_commit_file(&clone1, "test.txt", "test", "commit1");
 
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         create_branch(clone1_dir, "foo").unwrap();
 
         write_commit_file(&clone1, "test.txt", "test2", "commit2");
 
-        push(clone1_dir, "origin", "foo", false, None, None).unwrap();
+        push(clone1_dir, "origin", "foo", false, false, None, None)
+            .unwrap();
 
         // clone2
 
@@ -719,11 +723,14 @@ mod test_remote_branches {
         // clone1
 
         write_commit_file(&clone1, "test.txt", "test", "commit1");
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
         create_branch(clone1_dir, "foo").unwrap();
         write_commit_file(&clone1, "test.txt", "test2", "commit2");
-        push(clone1_dir, "origin", "foo", false, None, None).unwrap();
+        push(clone1_dir, "origin", "foo", false, false, None, None)
+            .unwrap();
 
         // clone2
 

asyncgit/src/sync/remotes/push.rs

@@ -95,6 +95,7 @@ pub(crate) fn push(
     remote: &str,
     branch: &str,
     force: bool,
+    delete: bool,
     basic_credential: Option<BasicAuthCredential>,
     progress_sender: Option<Sender<ProgressNotification>>,
 ) -> Result<()> {
@@ -109,15 +110,15 @@ pub(crate) fn push(
     options.remote_callbacks(callbacks.callbacks());
     options.packbuilder_parallelism(0);
 
-    let branch_name = format!("refs/heads/{}", branch);
+    let branch_modifier = match (force, delete) {
-    if force {
+        (true, true) => "+:",
-        remote.push(
+        (false, true) => ":",
-            &[String::from("+") + &branch_name],
+        (true, false) => "+",
-            Some(&mut options),
+        (false, false) => "",
-        )?;
+    };
-    } else {
+    let branch_name =
-        remote.push(&[branch_name.as_str()], Some(&mut options))?;
+        format!("{}refs/heads/{}", branch_modifier, branch);
-    }
+    remote.push(&[branch_name.as_str()], Some(&mut options))?;
 
     if let Some((reference, msg)) =
         callbacks.get_stats()?.push_rejected_msg
@@ -128,7 +129,9 @@ pub(crate) fn push(
         )));
     }
 
-    branch_set_upstream(&repo, branch)?;
+    if !delete {
+        branch_set_upstream(&repo, branch)?;
+    }
 
     Ok(())
 }
@@ -138,7 +141,10 @@ mod tests {
     use super::*;
     use crate::sync::{
         self,
-        tests::{get_commit_ids, repo_init, repo_init_bare},
+        tests::{
+            get_commit_ids, repo_clone, repo_init, repo_init_bare,
+            write_commit_file,
+        },
     };
     use git2::Repository;
     use std::{fs::File, io::Write, path::Path};
@@ -148,7 +154,6 @@ mod tests {
         // This test mimics the scenario of 2 people having 2
         // local branches and both modifying the same file then
         // both pushing, sequentially
-
         let (tmp_repo_dir, repo) = repo_init().unwrap();
         let (tmp_other_repo_dir, other_repo) = repo_init().unwrap();
         let (tmp_upstream_dir, _) = repo_init_bare().unwrap();
@@ -183,6 +188,7 @@ mod tests {
             "origin",
             "master",
             false,
+            false,
             None,
             None,
         )
@@ -208,6 +214,7 @@ mod tests {
                 "origin",
                 "master",
                 false,
+                false,
                 None,
                 None,
             )
@@ -223,6 +230,7 @@ mod tests {
                 "origin",
                 "master",
                 true,
+                false,
                 None,
                 None,
             )
@@ -291,6 +299,7 @@ mod tests {
             "origin",
             "master",
             false,
+            false,
             None,
             None,
         )
@@ -333,6 +342,7 @@ mod tests {
                 "origin",
                 "master",
                 false,
+                false,
                 None,
                 None,
             )
@@ -353,6 +363,7 @@ mod tests {
             "origin",
             "master",
             true,
+            false,
             None,
             None,
         )
@@ -372,4 +383,97 @@ mod tests {
                 .id();
         assert_eq!(new_upstream_parent, repo_2_parent,);
     }
+
+    #[test]
+    fn test_delete_remote_branch() {
+        // This test mimics the scenario of a user creating a branch, push it, and then remove it on the remote
+
+        let (upstream_dir, upstream_repo) = repo_init_bare().unwrap();
+
+        let (tmp_repo_dir, repo) =
+            repo_clone(upstream_dir.path().to_str().unwrap())
+                .unwrap();
+
+        // You need a commit before being able to branch !
+        let commit_1 = write_commit_file(
+            &repo,
+            "temp_file.txt",
+            "SomeContent",
+            "Initial commit",
+        );
+
+        let commits = get_commit_ids(&repo, 1);
+        assert!(commits.contains(&commit_1));
+
+        push(
+            tmp_repo_dir.path().to_str().unwrap(),
+            "origin",
+            "master",
+            false,
+            false,
+            None,
+            None,
+        )
+        .unwrap();
+
+        // Create the local branch
+        sync::create_branch(
+            tmp_repo_dir.path().to_str().unwrap(),
+            "test_branch",
+        )
+        .unwrap();
+
+        // Push the local branch
+        push(
+            tmp_repo_dir.path().to_str().unwrap(),
+            "origin",
+            "test_branch",
+            false,
+            false,
+            None,
+            None,
+        )
+        .unwrap();
+
+        // Test if the branch exits on the remote
+        assert_eq!(
+            upstream_repo
+                .branches(None)
+                .unwrap()
+                .map(|i| i.unwrap())
+                .map(|(i, _)| i.name().unwrap().unwrap().to_string())
+                .filter(|i| i == "test_branch")
+                .next()
+                .is_some(),
+            true
+        );
+
+        // Delete the remote branch
+        assert_eq!(
+            push(
+                tmp_repo_dir.path().to_str().unwrap(),
+                "origin",
+                "test_branch",
+                false,
+                true,
+                None,
+                None,
+            )
+            .is_ok(),
+            true
+        );
+
+        // Test that the branch has be remove from the remote
+        assert_eq!(
+            upstream_repo
+                .branches(None)
+                .unwrap()
+                .map(|i| i.unwrap())
+                .map(|(i, _)| i.name().unwrap().unwrap().to_string())
+                .filter(|i| i == "test_branch")
+                .next()
+                .is_some(),
+            false
+        );
+    }
 }

asyncgit/src/sync/remotes/tags.rs

@@ -177,8 +177,10 @@ mod tests {
 
         sync::tag(clone1_dir, &commit1, "tag1").unwrap();
 
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
         push_tags(clone1_dir, "origin", None, None).unwrap();
 
         // clone2
@@ -221,8 +223,10 @@ mod tests {
 
         sync::tag(clone1_dir, &commit1, "tag1").unwrap();
 
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
         push_tags(clone1_dir, "origin", None, None).unwrap();
 
         // clone2
@@ -252,8 +256,10 @@ mod tests {
 
         sync::tag(clone1_dir, &commit1, "tag1").unwrap();
 
-        push(clone1_dir, "origin", "master", false, None, None)
+        push(
-            .unwrap();
+            clone1_dir, "origin", "master", false, false, None, None,
+        )
+        .unwrap();
 
         let tags_missing =
             tags_missing_remote(clone1_dir, "origin", None).unwrap();

src/app.rs

@@ -644,8 +644,8 @@ impl App {
                 self.file_to_open = path;
                 flags.insert(NeedsUpdate::COMMANDS);
             }
-            InternalEvent::Push(branch, force) => {
+            InternalEvent::Push(branch, force, delete) => {
-                self.push_popup.push(branch, force)?;
+                self.push_popup.push(branch, force, delete)?;
                 flags.insert(NeedsUpdate::ALL);
             }
             InternalEvent::Pull(branch) => {
@@ -693,16 +693,34 @@ impl App {
                 sync::discard_lines(CWD, &path, &lines)?;
                 flags.insert(NeedsUpdate::ALL);
             }
-            Action::DeleteBranch(branch_ref) => {
+            Action::DeleteBranch(branch_ref, true) => {
                 if let Err(e) = sync::delete_branch(CWD, &branch_ref)
                 {
                     self.queue.push(InternalEvent::ShowErrorMsg(
                         e.to_string(),
                     ));
-                } else {
-                    flags.insert(NeedsUpdate::ALL);
-                    self.select_branch_popup.update_branches()?;
                 }
+                flags.insert(NeedsUpdate::ALL);
+                self.select_branch_popup.update_branches()?;
+            }
+            Action::DeleteBranch(branch_ref, false) => {
+                self.queue.push(
+                    if let Some(name) = branch_ref.rsplit('/').next()
+                    {
+                        InternalEvent::Push(
+                            name.to_string(),
+                            false,
+                            true,
+                        )
+                    } else {
+                        InternalEvent::ShowErrorMsg(format!(
+                            "Failed to find the branch name in {}",
+                            branch_ref
+                        ))
+                    },
+                );
+                flags.insert(NeedsUpdate::ALL);
+                self.select_branch_popup.update_branches()?;
             }
             Action::DeleteTag(tag_name) => {
                 if let Err(error) = sync::delete_tag(CWD, &tag_name) {
@@ -715,7 +733,8 @@ impl App {
                 }
             }
             Action::ForcePush(branch, force) => {
-                self.queue.push(InternalEvent::Push(branch, force));
+                self.queue
+                    .push(InternalEvent::Push(branch, force, false));
             }
             Action::PullMerge { rebase, .. } => {
                 self.pull_popup.try_conflict_free_merge(rebase);

src/components/branchlist.rs

@@ -149,7 +149,7 @@ impl Component for BranchListComponent {
                     &self.key_config,
                 ),
                 !self.selection_is_cur_branch(),
-                self.local,
+                true,
             ));
 
             out.push(CommandInfo::new(
@@ -222,6 +222,7 @@ impl Component for BranchListComponent {
                             self.branches[self.selection as usize]
                                 .reference
                                 .clone(),
+                            self.local,
                         ),
                     ));
                 } else if e == self.key_config.merge_branch

src/components/push.rs

@@ -30,10 +30,28 @@ use tui::{
     Frame,
 };
 
+///
+#[derive(PartialEq, Eq)]
+enum PushComponentModifier {
+    None,
+    Force,
+    Delete,
+    ForceDelete,
+}
+
+impl PushComponentModifier {
+    pub(crate) fn force(&self) -> bool {
+        self == &Self::Force || self == &Self::ForceDelete
+    }
+    pub(crate) fn delete(&self) -> bool {
+        self == &Self::Delete || self == &Self::ForceDelete
+    }
+}
+
 ///
 pub struct PushComponent {
+    modifier: PushComponentModifier,
     visible: bool,
-    force: bool,
     git_push: AsyncPush,
     progress: Option<RemoteProgress>,
     pending: bool,
@@ -54,7 +72,7 @@ impl PushComponent {
     ) -> Self {
         Self {
             queue: queue.clone(),
-            force: false,
+            modifier: PushComponentModifier::None,
             pending: false,
             visible: false,
             branch: String::new(),
@@ -74,9 +92,16 @@ impl PushComponent {
         &mut self,
         branch: String,
         force: bool,
+        delete: bool,
     ) -> Result<()> {
         self.branch = branch;
-        self.force = force;
+        self.modifier = match (force, delete) {
+            (true, true) => PushComponentModifier::ForceDelete,
+            (false, true) => PushComponentModifier::Delete,
+            (true, false) => PushComponentModifier::Force,
+            (false, false) => PushComponentModifier::None,
+        };
+
         self.show()?;
 
         if need_username_password()? {
@@ -100,8 +125,8 @@ impl PushComponent {
         cred: Option<BasicAuthCredential>,
         force: bool,
     ) -> Result<()> {
-        let remote = if let Some(remote) =
+        let remote = if let Ok(Some(remote)) =
-            get_branch_remote(CWD, &self.branch)?
+            get_branch_remote(CWD, &self.branch)
         {
             log::info!("push: branch '{}' has upstream for remote '{}' - using that",self.branch,remote);
             remote
@@ -122,6 +147,7 @@ impl PushComponent {
             remote,
             branch: self.branch.clone(),
             force,
+            delete: self.modifier.delete(),
             basic_credential: cred,
         })?;
         Ok(())
@@ -219,7 +245,7 @@ impl DrawableComponent for PushComponent {
                     .block(
                         Block::default()
                             .title(Span::styled(
-                                if self.force {
+                                if self.modifier.force() {
                                     strings::FORCE_PUSH_POPUP_MSG
                                 } else {
                                     strings::PUSH_POPUP_MSG
@@ -276,7 +302,7 @@ impl Component for PushComponent {
                     {
                         self.push_to_remote(
                             Some(self.input_cred.get_cred().clone()),
-                            self.force,
+                            self.modifier.force(),
                         )?;
                         self.input_cred.hide();
                     }

src/components/reset.rs

@@ -157,7 +157,7 @@ impl ResetComponent {
                     strings::confirm_title_reset(),
                     strings::confirm_msg_reset_lines(lines.len()),
                 ),
-                Action::DeleteBranch(branch_ref) => (
+                Action::DeleteBranch(branch_ref, true) => (
                     strings::confirm_title_delete_branch(
                         &self.key_config,
                     ),
@@ -166,6 +166,15 @@ impl ResetComponent {
                         branch_ref,
                     ),
                 ),
+                Action::DeleteBranch(branch_ref, false) => (
+                    strings::confirm_title_delete_remote_branch(
+                        &self.key_config,
+                    ),
+                    strings::confirm_msg_delete_remote_branch(
+                        &self.key_config,
+                        branch_ref,
+                    ),
+                ),
                 Action::DeleteTag(tag_name) => (
                     strings::confirm_title_delete_tag(
                         &self.key_config,

src/queue.rs

@@ -32,7 +32,7 @@ pub enum Action {
     ResetLines(String, Vec<DiffLinePosition>),
     StashDrop(CommitId),
     StashPop(CommitId),
-    DeleteBranch(String),
+    DeleteBranch(String, bool),
     DeleteTag(String),
     ForcePush(String, bool),
     PullMerge { incoming: usize, rebase: bool },
@@ -76,7 +76,7 @@ pub enum InternalEvent {
     ///
     OpenExternalEditor(Option<String>),
     ///
-    Push(String, bool),
+    Push(String, bool, bool),
     ///
     Pull(String),
     ///

src/strings.rs

@@ -174,6 +174,17 @@ pub fn confirm_msg_delete_branch(
 ) -> String {
     format!("Confirm deleting branch: '{}' ?", branch_ref)
 }
+pub fn confirm_title_delete_remote_branch(
+    _key_config: &SharedKeyConfig,
+) -> String {
+    "Delete Remote Branch".to_string()
+}
+pub fn confirm_msg_delete_remote_branch(
+    _key_config: &SharedKeyConfig,
+    branch_ref: &str,
+) -> String {
+    format!("Confirm deleting remote branch: '{}' ?", branch_ref)
+}
 pub fn confirm_title_delete_tag(
     _key_config: &SharedKeyConfig,
 ) -> String {

src/tabs/status.rs

@@ -450,8 +450,9 @@ impl Status {
                         Action::ForcePush(branch, force),
                     ));
                 } else {
-                    self.queue
+                    self.queue.push(InternalEvent::Push(
-                        .push(InternalEvent::Push(branch, force));
+                        branch, force, false,
+                    ));
                 }
             }
         }