[PATCH] branch: advise about ref syntax rules
- From
Kristoffer Haugsbakk <code@khaugsbakk.name>
- Date
- Mar 1, 2024, 15:38 UTC
- Message-ID
- <d275d1d179b90592ddd7b5da2ae4573b3f7a37b7.1709307442.git.code@khaugsbakk.name>
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 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.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> ---
Notes (series):
Hopefully I am using `advice.h` correctly here.
§ git-replace(1)
I did not add a hint for a similar message in `builtin/replace.c`.
`builtin/replace.c` has an error message in `check_ref_valid` for an
invalid ref name:
```
return error(_("'%s' is not a valid ref name"), ref->buf);
```
But there doesn’t seem to be a point to placing a hint here.
The preceding calls to `repo_get_oid` will catch both missing refs and
existing refs with invalid names:
```
if (repo_get_oid(r, refname, &object))
return error(_("failed to resolve '%s' as a valid ref"), refname);
```
Like for example this:
```
$ printf $(git rev-parse @~) > .git/refs/heads/hello..goodbye
$ git replace @ hello..goodbye
error: failed to resolve 'hello..goodbye' as a valid ref
[…]
$ git replace @ non-existing
error: failed to resolve 'non-existing' as a valid ref
```
§ Alternatives (to this change)
While working on this I also thought that it might be nice to have a
man page `gitrefsyntax`. That one could use a lot of the content from
`man git check-ref-format` verbatim. Then the hint could point towards
that man page. And it seems that AsciiDoc supports _includes_ which
means that the rules don’t have to be duplicated between the two man
pages.branch.c | 7 +++++-- builtin/branch.c | 7 +++++-- t/t3200-branch.sh | 10 ++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/branch.c b/branch.c index 6719a181bd1..1386918c60e 100644 --- a/branch.c +++ b/branch.c @@ -370,8 +370,11 @@ 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)) { + error(_("'%s' is not a valid branch name"), name); + advise(_("See `man git check-ref-format`")); + exit(1); + } return ref_exists(ref->buf); } diff --git a/builtin/branch.c b/builtin/branch.c index cfb63cce5fb..fa81e359157 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -576,8 +576,11 @@ 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 { + error(_("invalid branch name: '%s'"), oldname); + advise(_("See `man git check-ref-format`")); + exit(1); + } } for (int i = 0; worktrees[i]; i++) { diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index de7d3014e4f..9400a8baa35 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -1725,4 +1725,14 @@ test_expect_success '--track overrides branch.autoSetupMerge' ' test_cmp_config "" --default "" branch.foo5.merge ' +cat <<\EOF >expect +error: 'foo..bar' is not a valid branch name +hint: See `man git check-ref-format` +EOF + +test_expect_success 'errors if given a bad branch name' ' + test_must_fail git branch foo..bar >actual 2>&1 && + test_cmp expect actual +' + test_done
-- 2.44.0.106.g650c15c891b