From a39b304a981c1617ff75e312a7b5759e3789498e Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Tue, 19 Nov 2024 21:19:43 -0800
Subject: [PATCH] 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.
---
 cli/src/git_util.rs         |  2 +-
 cli/tests/test_git_fetch.rs | 10 ++++------
 lib/src/git.rs              |  7 +++++--
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs
index 74ebe2d2f..1edb7a621 100644
--- 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
index e6c2f96ac..232ddae66 100644
--- 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
index 3bc7ef297..5d28e4cbc 100644
--- 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<_>>()