Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
3 changes: 3 additions & 0 deletions asyncgit/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
6 changes: 4 additions & 2 deletions asyncgit/src/sync/branch.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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()))?;
}
Expand Down
27 changes: 14 additions & 13 deletions asyncgit/src/sync/cred.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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!(
Expand Down
2 changes: 1 addition & 1 deletion asyncgit/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
88 changes: 72 additions & 16 deletions asyncgit/src/sync/remotes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use git2::{
};
use scopetime::scope_time;

pub const DEFAULT_REMOTE_NAME: &str = "origin";

///
#[derive(Debug, Clone)]
pub enum ProgressNotification {
Expand Down Expand Up @@ -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)
}

///
Expand All @@ -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));
Expand Down Expand Up @@ -288,7 +307,7 @@ mod tests {
}

#[test]
fn test_first_remote() {
fn test_default_remote() {
let td = TempDir::new().unwrap();

debug_cmd_print(
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/components/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down