diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 099b9f562..15ed981d8 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -32,7 +32,7 @@ use indexmap::IndexSet; use itertools::Itertools; use jj_lib::backend::{BackendError, ChangeId, CommitId, ObjectId, TreeId}; use jj_lib::commit::Commit; -use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError}; +use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError, GitRemoteManagementError}; use jj_lib::git_backend::GitBackend; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::hex_util::to_reverse_hex; @@ -286,6 +286,12 @@ impl From for CommandError { } } +impl From for CommandError { + fn from(err: GitRemoteManagementError) -> Self { + user_error(format!("{err}")) + } +} + impl From for CommandError { fn from(err: RevsetEvaluationError) -> Self { user_error(format!("{err}")) diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 05e45cca3..b38be0f38 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -268,13 +268,9 @@ fn cmd_git_remote_remove( let mut workspace_command = command.workspace_helper(ui)?; let repo = workspace_command.repo(); let git_repo = get_git_repo(repo.store())?; - if git_repo.find_remote(&args.remote).is_err() { - return Err(user_error("Remote doesn't exist")); - } let mut tx = workspace_command.start_transaction(&format!("remove git remote {}", &args.remote)); - git::remove_remote(tx.mut_repo(), &git_repo, &args.remote) - .map_err(|err| user_error(err.to_string()))?; + git::remove_remote(tx.mut_repo(), &git_repo, &args.remote)?; if tx.mut_repo().has_changes() { tx.finish(ui) } else { @@ -290,13 +286,9 @@ fn cmd_git_remote_rename( let mut workspace_command = command.workspace_helper(ui)?; let repo = workspace_command.repo(); let git_repo = get_git_repo(repo.store())?; - if git_repo.find_remote(&args.old).is_err() { - return Err(user_error("Remote doesn't exist")); - } let mut tx = workspace_command .start_transaction(&format!("rename git remote {} to {}", &args.old, &args.new)); - git::rename_remote(tx.mut_repo(), &git_repo, &args.old, &args.new) - .map_err(|err| user_error(err.to_string()))?; + git::rename_remote(tx.mut_repo(), &git_repo, &args.old, &args.new)?; if tx.mut_repo().has_changes() { tx.finish(ui) } else { diff --git a/cli/tests/test_git_remotes.rs b/cli/tests/test_git_remotes.rs index 5a30945ac..80f935ebe 100644 --- a/cli/tests/test_git_remotes.rs +++ b/cli/tests/test_git_remotes.rs @@ -46,8 +46,9 @@ fn test_git_remotes() { insta::assert_snapshot!(stdout, @"bar http://example.com/repo/bar "); let stderr = test_env.jj_cmd_failure(&repo_path, &["git", "remote", "remove", "nonexistent"]); - insta::assert_snapshot!(stderr, @"Error: Remote doesn't exist -"); + insta::assert_snapshot!(stderr, @r###" + Error: No git remote named 'nonexistent' + "###); } #[test] @@ -61,7 +62,9 @@ fn test_git_remote_rename() { &["git", "remote", "add", "foo", "http://example.com/repo/foo"], ); let stderr = test_env.jj_cmd_failure(&repo_path, &["git", "remote", "rename", "bar", "foo"]); - insta::assert_snapshot!(stderr, @"Error: Remote doesn't exist\n"); + insta::assert_snapshot!(stderr, @r###" + Error: No git remote named 'bar' + "###); let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "rename", "foo", "bar"]); insta::assert_snapshot!(stdout, @""); let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "list"]); diff --git a/lib/src/git.rs b/lib/src/git.rs index 2d5545a36..f59e44c2a 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -548,6 +548,14 @@ pub fn export_some_refs( Ok(failed_branches) } +#[derive(Debug, Error)] +pub enum GitRemoteManagementError { + #[error("No git remote named '{0}'")] + NoSuchRemote(String), + #[error(transparent)] + InternalGitError(git2::Error), +} + fn is_remote_not_found_err(err: &git2::Error) -> bool { matches!( (err.class(), err.code()), @@ -562,8 +570,14 @@ pub fn remove_remote( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, remote_name: &str, -) -> Result<(), git2::Error> { - git_repo.remote_delete(remote_name)?; +) -> Result<(), GitRemoteManagementError> { + git_repo.remote_delete(remote_name).map_err(|err| { + if is_remote_not_found_err(&err) { + GitRemoteManagementError::NoSuchRemote(remote_name.to_owned()) + } else { + GitRemoteManagementError::InternalGitError(err) + } + })?; let mut branches_to_delete = vec![]; for (branch, target) in mut_repo.view().branches() { if target.remote_targets.contains_key(remote_name) { @@ -592,8 +606,16 @@ pub fn rename_remote( git_repo: &git2::Repository, old_remote_name: &str, new_remote_name: &str, -) -> Result<(), git2::Error> { - git_repo.remote_rename(old_remote_name, new_remote_name)?; +) -> Result<(), GitRemoteManagementError> { + git_repo + .remote_rename(old_remote_name, new_remote_name) + .map_err(|err| { + if is_remote_not_found_err(&err) { + GitRemoteManagementError::NoSuchRemote(old_remote_name.to_owned()) + } else { + GitRemoteManagementError::InternalGitError(err) + } + })?; mut_repo.rename_remote(old_remote_name, new_remote_name); let prefix = format!("refs/remotes/{old_remote_name}/"); let git_refs = mut_repo