From e53870d787f08d7398ce0ff542142b8e9ec34121 Mon Sep 17 00:00:00 2001 From: Essien Ita Essien <34972+essiene@users.noreply.github.com> Date: Mon, 4 Nov 2024 20:19:26 +0000 Subject: [PATCH] cli: Refactor out `get_fetch_remotes` into git_util.rs Similarly to `git_fetch`, this will be used in `jj git sync`, so moving this out to maintain the same behaviour across `fetch` command and `sync`. * Refactor out `get_fetch_remotes` to `git_util.rs`. * inline the different `get_*_remote*` functions into `get_fetch_remotes`; The resulting function is still small enough to be easily readable. * Move `get_single_remote` into `git_util.rs` and update call sites. * Update use references All tests still pass. Issue: #1039 --- cli/src/commands/git/fetch.rs | 56 +++-------------------------------- cli/src/commands/git/mod.rs | 8 ----- cli/src/commands/git/push.rs | 2 +- cli/src/git_util.rs | 46 ++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 61 deletions(-) diff --git a/cli/src/commands/git/fetch.rs b/cli/src/commands/git/fetch.rs index 808cdf3c73..a4230feb0d 100644 --- a/cli/src/commands/git/fetch.rs +++ b/cli/src/commands/git/fetch.rs @@ -14,13 +14,11 @@ use itertools::Itertools; use jj_lib::repo::Repo; -use jj_lib::settings::ConfigResultExt as _; -use jj_lib::settings::UserSettings; use jj_lib::str_util::StringPattern; use crate::cli_util::CommandHelper; use crate::command_error::CommandError; -use crate::commands::git::get_single_remote; +use crate::git_util::get_fetch_remotes; use crate::git_util::get_git_repo; use crate::git_util::git_fetch; use crate::ui::Ui; @@ -54,59 +52,13 @@ pub fn cmd_git_fetch( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let git_repo = get_git_repo(workspace_command.repo().store())?; - let remotes = if args.all_remotes { - get_all_remotes(&git_repo)? - } else if args.remotes.is_empty() { - get_default_fetch_remotes(ui, command.settings(), &git_repo)? - } else { - args.remotes.clone() - }; + let settings = command.settings(); + let remotes = get_fetch_remotes(ui, settings, &git_repo, &args.remotes, args.all_remotes)?; let mut tx = workspace_command.start_transaction(); - git_fetch( - ui, - &mut tx, - command.settings(), - &git_repo, - &remotes, - &args.branch, - )?; + git_fetch(ui, &mut tx, settings, &git_repo, &remotes, &args.branch)?; tx.finish( ui, format!("fetch from git remote(s) {}", remotes.iter().join(",")), )?; Ok(()) } - -const DEFAULT_REMOTE: &str = "origin"; - -fn get_default_fetch_remotes( - ui: &Ui, - settings: &UserSettings, - git_repo: &git2::Repository, -) -> Result, CommandError> { - const KEY: &str = "git.fetch"; - if let Ok(remotes) = settings.config().get(KEY) { - Ok(remotes) - } else if let Some(remote) = settings.config().get_string(KEY).optional()? { - Ok(vec![remote]) - } else if let Some(remote) = get_single_remote(git_repo)? { - // if nothing was explicitly configured, try to guess - if remote != DEFAULT_REMOTE { - writeln!( - ui.hint_default(), - "Fetching from the only existing remote: {remote}" - )?; - } - Ok(vec![remote]) - } else { - Ok(vec![DEFAULT_REMOTE.to_owned()]) - } -} - -fn get_all_remotes(git_repo: &git2::Repository) -> Result, CommandError> { - let git_remotes = git_repo.remotes()?; - Ok(git_remotes - .iter() - .filter_map(|x| x.map(ToOwned::to_owned)) - .collect()) -} diff --git a/cli/src/commands/git/mod.rs b/cli/src/commands/git/mod.rs index 1c7fee9818..3aa6105ec1 100644 --- a/cli/src/commands/git/mod.rs +++ b/cli/src/commands/git/mod.rs @@ -94,11 +94,3 @@ pub fn maybe_add_gitignore(workspace_command: &WorkspaceCommandHelper) -> Result Ok(()) } } - -fn get_single_remote(git_repo: &git2::Repository) -> Result, CommandError> { - let git_remotes = git_repo.remotes()?; - Ok(match git_remotes.len() { - 1 => git_remotes.get(0).map(ToOwned::to_owned), - _ => None, - }) -} diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index b519e9e972..6ff5ae6d27 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -45,9 +45,9 @@ use crate::cli_util::WorkspaceCommandTransaction; use crate::command_error::user_error; use crate::command_error::user_error_with_hint; use crate::command_error::CommandError; -use crate::commands::git::get_single_remote; use crate::formatter::Formatter; use crate::git_util::get_git_repo; +use crate::git_util::get_single_remote; use crate::git_util::map_git_error; use crate::git_util::with_remote_git_callbacks; use crate::git_util::GitSidebandProgressMessageWriter; diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 9cd8dd5124..30f1f625f4 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -35,6 +35,7 @@ use jj_lib::op_store::RefTarget; use jj_lib::op_store::RemoteRef; use jj_lib::repo::ReadonlyRepo; use jj_lib::repo::Repo; +use jj_lib::settings::ConfigResultExt as _; use jj_lib::settings::UserSettings; use jj_lib::store::Store; use jj_lib::str_util::StringPattern; @@ -67,6 +68,14 @@ pub fn map_git_error(err: git2::Error) -> CommandError { } } +pub fn get_single_remote(git_repo: &git2::Repository) -> Result, CommandError> { + let git_remotes = git_repo.remotes()?; + Ok(match git_remotes.len() { + 1 => git_remotes.get(0).map(ToOwned::to_owned), + _ => None, + }) +} + pub fn get_git_repo(store: &Store) -> Result { match store.backend_impl().downcast_ref::() { None => Err(user_error("The repo is not backed by a git repo")), @@ -535,3 +544,40 @@ fn warn_if_branches_not_found( Ok(()) } + +const DEFAULT_REMOTE: &str = "origin"; +pub fn get_fetch_remotes( + ui: &Ui, + settings: &UserSettings, + git_repo: &git2::Repository, + remotes: &[String], + use_all_remotes: bool, +) -> Result, CommandError> { + if use_all_remotes { + Ok(git_repo + .remotes()? + .iter() + .filter_map(|x| x.map(ToOwned::to_owned)) + .collect()) + } else if !remotes.is_empty() { + Ok(remotes.iter().map(|s| s.clone()).collect_vec()) + } else { + const KEY: &str = "git.fetch"; + if let Ok(remotes) = settings.config().get(KEY) { + Ok(remotes) + } else if let Some(remote) = settings.config().get_string(KEY).optional()? { + Ok(vec![remote]) + } else if let Some(remote) = get_single_remote(git_repo)? { + // if nothing was explicitly configured, try to guess + if remote != DEFAULT_REMOTE { + writeln!( + ui.hint_default(), + "Fetching from the only existing remote: {remote}" + )?; + } + Ok(vec![remote]) + } else { + Ok(vec![DEFAULT_REMOTE.to_owned()]) + } + } +}