branch: advise about ref syntax rules

git-branch(1) will error out if you give it a bad ref name. But the user
might not understand why or what part of the name is illegal.

The user might know that there are some limitations based on the *loose
ref* format (filenames), but there are also further rules for
easier integration with shell-based tools, pathname expansion, and
playing well with reference name expressions.

The man page for git-check-ref-format(1) contains these rules. Let’s
advise about it since that is not a command that you just happen
upon. Also make this advise configurable since you might not want to be
reminded every time you make a little typo.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index dd52041bc94f13fe4b3cc718bb5ead5b6d8c1221..06c754899c553e2dcca7bec4c3083a9b1aa88e22 100644 (file)
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -94,6 +94,9 @@ advice.*::
                `pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`,
                `pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`
                simultaneously.
+       refSyntax::
+               Shown when the user provides an illegal ref name, to
+               tell the user about the ref syntax documentation.
        resetNoRefresh::
                Shown when linkgit:git-reset[1] takes more than 2
                seconds to refresh the index after reset, to tell the user

diff --git a/advice.c b/advice.c
index 6e9098ff08935a2670b468d1fe1a36f3c268c055..550c29689082511769c97a374dbb765ffe18de0f 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -68,6 +68,7 @@ static struct {
        [ADVICE_PUSH_UNQUALIFIED_REF_NAME]              = { "pushUnqualifiedRefName" },
        [ADVICE_PUSH_UPDATE_REJECTED]                   = { "pushUpdateRejected" },
        [ADVICE_PUSH_UPDATE_REJECTED_ALIAS]             = { "pushNonFastForward" }, /* backwards compatibility */
+       [ADVICE_REF_SYNTAX]                             = { "refSyntax" },
        [ADVICE_RESET_NO_REFRESH_WARNING]               = { "resetNoRefresh" },
        [ADVICE_RESOLVE_CONFLICT]                       = { "resolveConflict" },
        [ADVICE_RM_HINTS]                               = { "rmHints" },

diff --git a/advice.h b/advice.h
index 9d4f49ae38bcfe3b0bdf9999cf9d66ee6a95739e..d15fe2351abcd9b6dcf96e5934d2a2fb763da4d5 100644 (file)
--- a/advice.h
+++ b/advice.h
@@ -36,6 +36,7 @@ enum advice_type {
        ADVICE_PUSH_UNQUALIFIED_REF_NAME,
        ADVICE_PUSH_UPDATE_REJECTED,
        ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
+       ADVICE_REF_SYNTAX,
        ADVICE_RESET_NO_REFRESH_WARNING,
        ADVICE_RESOLVE_CONFLICT,
        ADVICE_RM_HINTS,

diff --git a/branch.c b/branch.c
index 6719a181bd1f03af21b92d8be71a93142ef700e7..621019fcf4bde0a0568dbae2a1055e12cf8df030 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -370,8 +370,12 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
  */
 int validate_branchname(const char *name, struct strbuf *ref)
 {
-       if (strbuf_check_branch_ref(ref, name))
-               die(_("'%s' is not a valid branch name"), name);
+       if (strbuf_check_branch_ref(ref, name)) {
+               int code = die_message(_("'%s' is not a valid branch name"), name);
+               advise_if_enabled(ADVICE_REF_SYNTAX,
+                                 _("See `man git check-ref-format`"));
+               exit(code);
+       }
 
        return ref_exists(ref->buf);
 }

diff --git a/builtin/branch.c b/builtin/branch.c
index cfb63cce5fb9dff64106907947d0df25a2c25489..1c122ee8a7b081d0690aa5dc3405a5fe294dd0c8 100644 (file)
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -576,8 +576,12 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
                 */
                if (ref_exists(oldref.buf))
                        recovery = 1;
-               else
-                       die(_("invalid branch name: '%s'"), oldname);
+               else {
+                       int code = die_message(_("invalid branch name: '%s'"), oldname);
+                       advise_if_enabled(ADVICE_REF_SYNTAX,
+                                         _("See `man git check-ref-format`"));
+                       exit(code);
+               }
        }
 
        for (int i = 0; worktrees[i]; i++) {

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 060b27097e8a8fe19dc038e6c05505d58cb47999..dd7525d1b8c5023daba6cd24e33d943df94b9136 100755 (executable)
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1722,4 +1722,14 @@ test_expect_success '--track overrides branch.autoSetupMerge' '
        test_cmp_config "" --default "" branch.foo5.merge
 '
 
+test_expect_success 'errors if given a bad branch name' '
+       cat <<-\EOF >expect &&
+       fatal: '\''foo..bar'\'' is not a valid branch name
+       hint: See `man git check-ref-format`
+       hint: Disable this message with "git config advice.refSyntax false"
+       EOF
+       test_must_fail git branch foo..bar >actual 2>&1 &&
+       test_cmp expect actual
+'
+
 test_done