cli git fetch: clarify the error for invalid branch names/globs

Create a clearer error when a branch name contains `*`. Slightly clarify the error message for `?` in branch name/glob.
jj-vcs · Nov 20, 2024 · a39b304 · a39b304
1 parent aa4830e
commit a39b304
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs
@@ -484,7 +484,7 @@ pub fn git_fetch(
                     .any(|pattern| pattern.as_exact().is_some_and(|s| s.contains('*')))
                 {
                     user_error_with_hint(
-                        err,
+                        "Branch names may not include `*`.",
                         "Prefix the pattern with `glob:` to expand `*` as a glob",
                     )
                 } else {

diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs
@@ -612,14 +612,12 @@ fn test_git_fetch_some_of_many_bookmarks() {
         &target_jj_repo_path,
         &["git", "fetch", "--branch", "glob:^:a*"],
     );
-    insta::assert_snapshot!(stderr, @r###"
-    Error: Invalid branch pattern provided. Patterns may not contain the characters `:`, `^`, `?`, `[`, `]`
-    "###);
+    insta::assert_snapshot!(stderr, @"Error: Invalid branch pattern provided. When fetching, branch names and globs may not contain the characters `:`, `^`, `?`, `[`, `]`");
     let stderr = test_env.jj_cmd_failure(&target_jj_repo_path, &["git", "fetch", "--branch", "a*"]);
-    insta::assert_snapshot!(stderr, @r###"
-    Error: Invalid branch pattern provided. Patterns may not contain the characters `:`, `^`, `?`, `[`, `]`
+    insta::assert_snapshot!(stderr, @r"
+    Error: Branch names may not include `*`.
     Hint: Prefix the pattern with `glob:` to expand `*` as a glob
-    "###);
+    ");
 
     // Nothing in our repo before the fetch
     insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"

diff --git a/lib/src/git.rs b/lib/src/git.rs
@@ -1218,7 +1218,7 @@ pub enum GitFetchError {
     #[error("No git remote named '{0}'")]
     NoSuchRemote(String),
     #[error(
-        "Invalid branch pattern provided. Patterns may not contain the characters `{chars}`",
+        "Invalid branch pattern provided. When fetching, branch names and globs may not contain the characters `{chars}`",
         chars = INVALID_REFSPEC_CHARS.iter().join("`, `")
     )]
     InvalidBranchPattern,
@@ -1273,7 +1273,10 @@ pub fn fetch(
         .map(|pattern| {
             pattern
                 .to_glob()
-                .filter(|glob| !glob.contains(INVALID_REFSPEC_CHARS))
+                .filter(
+                    /* This check will fail if `to_glob()` escapes a literal `*` */
+                    |glob| !glob.contains(INVALID_REFSPEC_CHARS),
+                )
                 .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}"))
         })
         .collect::<Option<_>>()