prefer 'origin' as default remote if it exists (#544)

closes #494

Author: Stephan Dilly

CHANGELOG.md

@@ -12,9 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - support mouse scrolling [[@WizardOhio24](https://github.com/WizardOhio24)] ([#306](https://github.com/extrawurst/gitui/issues/306)) 
 - show used char count in input texts ([#466](https://github.com/extrawurst/gitui/issues/466))
 
-![push](assets/char_count.gif)
+![charcount](assets/char_count.gif)
 
 ### Fixed
+- push defaults to 'origin' remote if it exists ([#494](https://github.com/extrawurst/gitui/issues/494))
 - support missing pageUp/down support in branchlist ([#519](https://github.com/extrawurst/gitui/issues/519))
 - don't hide branch name while in commit dialog ([#529](https://github.com/extrawurst/gitui/issues/529))
 - don't discard commit message without confirmation ([#530](https://github.com/extrawurst/gitui/issues/530))

asyncgit/src/error.rs

@@ -12,6 +12,9 @@ pub enum Error {
     #[error("git: remote url not found")]
     UnknownRemote,
 
+    #[error("git: inconclusive remotes")]
+    NoDefaultRemoteFound,
+
     #[error("git: work dir error")]
     NoWorkDir,
 

asyncgit/src/sync/branch.rs

@@ -1,6 +1,8 @@
 //!
 
-use super::{remotes::get_first_remote_in_repo, utils::bytes2string};
+use super::{
+    remotes::get_default_remote_in_repo, utils::bytes2string,
+};
 use crate::{
     error::{Error, Result},
     sync::{utils, CommitId},
@@ -97,7 +99,7 @@ pub(crate) fn branch_set_upstream(
         repo.find_branch(branch_name, BranchType::Local)?;
 
     if branch.upstream().is_err() {
-        let remote = get_first_remote_in_repo(repo)?;
+        let remote = get_default_remote_in_repo(repo)?;
         let upstream_name = format!("{}/{}", remote, branch_name);
         branch.set_upstream(Some(upstream_name.as_str()))?;
     }

asyncgit/src/sync/cred.rs

@@ -1,12 +1,12 @@
 //! credentials git helper
 
+use super::remotes::get_default_remote_in_repo;
+use crate::{
+    error::{Error, Result},
+    CWD,
+};
 use git2::{Config, CredentialHelper};
 
-use crate::error::{Error, Result};
-use crate::CWD;
-
-use super::remotes::get_first_remote_in_repo;
-
 /// basic Authentication Credentials
 #[derive(Debug, Clone, Default, PartialEq)]
 pub struct BasicAuthCredential {
@@ -34,7 +34,7 @@ impl BasicAuthCredential {
 pub fn need_username_password() -> Result<bool> {
     let repo = crate::sync::utils::repo(CWD)?;
     let url = repo
-        .find_remote(&get_first_remote_in_repo(&repo)?)?
+        .find_remote(&get_default_remote_in_repo(&repo)?)?
         .url()
         .ok_or(Error::UnknownRemote)?
         .to_owned();
@@ -46,7 +46,7 @@ pub fn need_username_password() -> Result<bool> {
 pub fn extract_username_password() -> Result<BasicAuthCredential> {
     let repo = crate::sync::utils::repo(CWD)?;
     let url = repo
-        .find_remote(&get_first_remote_in_repo(&repo)?)?
+        .find_remote(&get_default_remote_in_repo(&repo)?)?
         .url()
         .ok_or(Error::UnknownRemote)?
         .to_owned();
@@ -81,16 +81,17 @@ pub fn extract_cred_from_url(url: &str) -> BasicAuthCredential {
 
 #[cfg(test)]
 mod tests {
-    use crate::sync::cred::{
-        extract_cred_from_url, extract_username_password,
-        need_username_password, BasicAuthCredential,
+    use crate::sync::{
+        cred::{
+            extract_cred_from_url, extract_username_password,
+            need_username_password, BasicAuthCredential,
+        },
+        remotes::DEFAULT_REMOTE_NAME,
+        tests::repo_init,
     };
-    use crate::sync::tests::repo_init;
     use serial_test::serial;
     use std::env;
 
-    const DEFAULT_REMOTE_NAME: &str = "origin";
-
     #[test]
     fn test_credential_complete() {
         assert_eq!(

asyncgit/src/sync/mod.rs

@@ -41,7 +41,7 @@ pub use hunks::{reset_hunk, stage_hunk, unstage_hunk};
 pub use ignore::add_to_ignore;
 pub use logwalker::LogWalker;
 pub use remotes::{
-    fetch_origin, get_first_remote, get_remotes, push,
+    fetch_origin, get_default_remote, get_remotes, push,
     ProgressNotification,
 };
 pub use reset::{reset_stage, reset_workdir};

asyncgit/src/sync/remotes.rs

@@ -13,6 +13,8 @@ use git2::{
 };
 use scopetime::scope_time;
 
+pub const DEFAULT_REMOTE_NAME: &str = "origin";
+
 ///
 #[derive(Debug, Clone)]
 pub enum ProgressNotification {
@@ -66,28 +68,45 @@ pub fn get_remotes(repo_path: &str) -> Result<Vec<String>> {
     Ok(remotes)
 }
 
-///
-pub fn get_first_remote(repo_path: &str) -> Result<String> {
+/// tries to find origin or the only remote that is defined if any
+/// in case of multiple remotes and none named *origin* we fail
+pub fn get_default_remote(repo_path: &str) -> Result<String> {
     let repo = utils::repo(repo_path)?;
-    get_first_remote_in_repo(&repo)
+    get_default_remote_in_repo(&repo)
 }
 
-///
-pub(crate) fn get_first_remote_in_repo(
+/// see `get_default_remote`
+pub(crate) fn get_default_remote_in_repo(
     repo: &Repository,
 ) -> Result<String> {
-    scope_time!("get_remotes");
+    scope_time!("get_default_remote_in_repo");
 
     let remotes = repo.remotes()?;
 
-    let first_remote = remotes
-        .iter()
-        .next()
-        .flatten()
-        .map(String::from)
-        .ok_or_else(|| Error::Generic("no remote found".into()))?;
+    // if `origin` exists return that
+    let found_origin = remotes.iter().any(|r| {
+        r.map(|r| r == DEFAULT_REMOTE_NAME).unwrap_or_default()
+    });
+    if found_origin {
+        return Ok(DEFAULT_REMOTE_NAME.into());
+    }
 
-    Ok(first_remote)
+    //if only one remote exists pick that
+    if remotes.len() == 1 {
+        let first_remote = remotes
+            .iter()
+            .next()
+            .flatten()
+            .map(String::from)
+            .ok_or_else(|| {
+                Error::Generic("no remote found".into())
+            })?;
+
+        return Ok(first_remote);
+    }
+
+    //inconclusive
+    Err(Error::NoDefaultRemoteFound)
 }
 
 ///
@@ -96,7 +115,7 @@ pub fn fetch_origin(repo_path: &str, branch: &str) -> Result<usize> {
 
     let repo = utils::repo(repo_path)?;
     let mut remote =
-        repo.find_remote(&get_first_remote_in_repo(&repo)?)?;
+        repo.find_remote(&get_default_remote_in_repo(&repo)?)?;
 
     let mut options = FetchOptions::new();
     options.remote_callbacks(remote_callbacks(None, None));
@@ -288,7 +307,7 @@ mod tests {
     }
 
     #[test]
-    fn test_first_remote() {
+    fn test_default_remote() {
         let td = TempDir::new().unwrap();
 
         debug_cmd_print(
@@ -311,7 +330,44 @@ mod tests {
             vec![String::from("origin"), String::from("second")]
         );
 
-        let first = get_first_remote_in_repo(
+        let first = get_default_remote_in_repo(
+            &utils::repo(repo_path).unwrap(),
+        )
+        .unwrap();
+        assert_eq!(first, String::from("origin"));
+    }
+
+    #[test]
+    fn test_default_remote_out_of_order() {
+        let td = TempDir::new().unwrap();
+
+        debug_cmd_print(
+            td.path().as_os_str().to_str().unwrap(),
+            "git clone https://github.com/extrawurst/brewdump.git",
+        );
+
+        debug_cmd_print(
+            td.path().as_os_str().to_str().unwrap(),
+            "cd brewdump && git remote rename origin alternate",
+        );
+
+        debug_cmd_print(
+            td.path().as_os_str().to_str().unwrap(),
+            "cd brewdump && git remote add origin https://github.com/extrawurst/brewdump.git",
+        );
+
+        let repo_path = td.path().join("brewdump");
+        let repo_path = repo_path.as_os_str().to_str().unwrap();
+
+        //NOTE: aparently remotes are not chronolically sorted but alphabetically
+        let remotes = get_remotes(repo_path).unwrap();
+
+        assert_eq!(
+            remotes,
+            vec![String::from("alternate"), String::from("origin")]
+        );
+
+        let first = get_default_remote_in_repo(
             &utils::repo(repo_path).unwrap(),
         )
         .unwrap();

src/components/push.rs

@@ -15,7 +15,7 @@ use asyncgit::{
             extract_username_password, need_username_password,
             BasicAuthCredential,
         },
-        get_first_remote,
+        get_default_remote,
     },
     AsyncNotification, AsyncPush, PushProgress, PushProgressState,
     PushRequest, CWD,
@@ -102,7 +102,7 @@ impl PushComponent {
         self.pending = true;
         self.progress = None;
         self.git_push.request(PushRequest {
-            remote: get_first_remote(CWD)?,
+            remote: get_default_remote(CWD)?,
             branch: self.branch.clone(),
             force,
             basic_credential: cred,