user confirm merge (#565)

Author: Stephan Dilly

CHANGELOG.md

@@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - MacOS config directory now uses `~/.config/gitui` [[@remique](https://github.com/remique)] ([#317](https://github.com/extrawurst/gitui/issues/317)) 
 
 ### Added
-- support for pull (fetch + ff-only merge) ([#319](https://github.com/extrawurst/gitui/issues/319))
+- support for pull (fetch + simple merging) ([#319](https://github.com/extrawurst/gitui/issues/319))
 - show used char count in input texts ([#466](https://github.com/extrawurst/gitui/issues/466))
 - support smoother left/right toggle/keys for commit details ([#418](https://github.com/extrawurst/gitui/issues/418))
 - support *force push* command [[@WizardOhio24](https://github.com/WizardOhio24)] ([#274](https://github.com/extrawurst/gitui/issues/274)) 

asyncgit/src/sync/branch/merge_commit.rs

@@ -78,7 +78,7 @@ pub fn merge_upstream_commit(
             Some("HEAD"),
             &signature,
             &signature,
-            format!("Merge '{}' from {}", branch_name, remote_url)
+            format!("Merge '{}' of {}", branch_name, remote_url)
                 .as_str(),
             &tree,
             parents.as_slice(),
@@ -179,7 +179,7 @@ mod test {
         assert_eq!(
             details.message.unwrap().combine(),
             format!(
-                "Merge 'master' from {}",
+                "Merge 'master' of {}",
                 r1_dir.path().to_str().unwrap()
             )
         );

src/app.rs

@@ -34,7 +34,7 @@ use tui::{
     Frame,
 };
 
-///
+/// the main app type
 pub struct App {
     do_quit: bool,
     help: HelpComponent,
@@ -509,6 +509,10 @@ impl App {
                     .queue
                     .borrow_mut()
                     .push_back(InternalEvent::Push(branch, force)),
+                Action::PullMerge(_) => {
+                    self.pull_popup.try_conflict_free_merge();
+                    flags.insert(NeedsUpdate::ALL);
+                }
             },
             InternalEvent::ConfirmAction(action) => {
                 self.reset.open(action)?;

src/components/pull.rs

@@ -5,7 +5,7 @@ use crate::{
         CommandInfo, Component, DrawableComponent,
     },
     keys::SharedKeyConfig,
-    queue::{InternalEvent, Queue},
+    queue::{Action, InternalEvent, Queue},
     strings, try_or_popup,
     ui::{self, style::SharedTheme},
 };
@@ -131,7 +131,7 @@ impl PullComponent {
                 self.git_fetch.last_result()?
             {
                 if err.is_empty() {
-                    self.do_merge()?;
+                    self.try_ff_merge()?;
                 } else {
                     self.queue.borrow_mut().push_back(
                         InternalEvent::ShowErrorMsg(format!(
@@ -141,14 +141,13 @@ impl PullComponent {
                     );
                 }
             }
-            self.hide();
         }
 
         Ok(())
     }
 
     // check if something is incoming and try a ff merge then
-    fn do_merge(&self) -> Result<()> {
+    fn try_ff_merge(&mut self) -> Result<()> {
         let branch_compare =
             sync::branch_compare_upstream(CWD, &self.branch)?;
         if branch_compare.behind > 0 {
@@ -157,18 +156,30 @@ impl PullComponent {
                 &self.branch,
             );
             if let Err(err) = merge_res {
-                log::error!("ff merge failed: {}", err);
-
-                try_or_popup!(
-                    self,
-                    "merge failed:",
-                    sync::merge_upstream_commit(CWD, &self.branch)
-                );
+                log::trace!("ff merge failed: {}", err);
+                self.confirm_merge(branch_compare.behind);
+            } else {
+                self.hide();
             }
         }
 
         Ok(())
     }
+
+    pub fn try_conflict_free_merge(&self) {
+        try_or_popup!(
+            self,
+            "merge failed:",
+            sync::merge_upstream_commit(CWD, &self.branch)
+        );
+    }
+
+    fn confirm_merge(&mut self, incoming: usize) {
+        self.queue.borrow_mut().push_back(
+            InternalEvent::ConfirmAction(Action::PullMerge(incoming)),
+        );
+        self.hide();
+    }
 }
 
 impl DrawableComponent for PullComponent {
@@ -232,7 +243,7 @@ impl Component for PullComponent {
 
     fn event(&mut self, ev: Event) -> Result<bool> {
         if self.visible {
-            if let Event::Key(e) = ev {
+            if let Event::Key(_) = ev {
                 if self.input_cred.is_visible() {
                     if self.input_cred.event(ev)? {
                         return Ok(true);
@@ -243,10 +254,6 @@ impl Component for PullComponent {
                         ))?;
                         self.input_cred.hide();
                     }
-                } else if e == self.key_config.exit_popup
-                    && !self.pending
-                {
-                    self.hide();
                 }
             }
             return Ok(true);

src/components/reset.rs

@@ -57,7 +57,7 @@ impl Component for ResetComponent {
         _force_all: bool,
     ) -> CommandBlocking {
         out.push(CommandInfo::new(
-            strings::commands::reset_confirm(&self.key_config),
+            strings::commands::confirm_action(&self.key_config),
             true,
             self.visible,
         ));
@@ -169,6 +169,10 @@ impl ResetComponent {
                         branch.rsplit('/').next().expect("There was no / in the head reference which is impossible in git"),
                     ),
                 ),
+                Action::PullMerge(incoming) => (
+                    strings::confirm_title_merge(&self.key_config),
+                    strings::confirm_msg_merge(&self.key_config,*incoming),
+                ),
             };
         }
 

src/queue.rs

@@ -30,6 +30,7 @@ pub enum Action {
     StashDrop(CommitId),
     DeleteBranch(String),
     ForcePush(String, bool),
+    PullMerge(usize),
 }
 
 ///

src/strings.rs

@@ -90,6 +90,15 @@ pub fn confirm_title_stashdrop(
 ) -> String {
     "Drop".to_string()
 }
+pub fn confirm_title_merge(_key_config: &SharedKeyConfig) -> String {
+    "Merge".to_string()
+}
+pub fn confirm_msg_merge(
+    _key_config: &SharedKeyConfig,
+    incoming: usize,
+) -> String {
+    format!("confirm merge of {} incoming commits? ", incoming)
+}
 pub fn confirm_msg_reset(_key_config: &SharedKeyConfig) -> String {
     "confirm file reset?".to_string()
 }
@@ -599,15 +608,15 @@ pub mod commands {
             CMD_GROUP_GENERAL,
         )
     }
-    pub fn reset_confirm(
+    pub fn confirm_action(
         key_config: &SharedKeyConfig,
     ) -> CommandText {
         CommandText::new(
             format!(
                 "Confirm [{}]",
                 key_config.get_hint(key_config.enter),
             ),
-            "resets the file in question",
+            "confirm action",
             CMD_GROUP_GENERAL,
         )
     }