{"thread":{"id":"61033","subject":"[PATCH] branch: advise about ref syntax rules","startedAt":"2024-03-01T15:39:09Z","lastAt":"2024-03-05T20:31:10Z","messageCount":28,"participants":["Kristoffer Haugsbakk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"489742","messageId":"d275d1d179b90592ddd7b5da2ae4573b3f7a37b7.1709307442.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":null,"subject":"[PATCH] branch: advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-01T15:38:41Z","receivedAt":"2024-03-01T15:39:09Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"git-branch(1) will error out if you give it a bad ref name. But the user\nmight not understand why or what part of the name is illegal. The man\npage for git-check-ref-format(1) contains these rules. Let’s advise\nabout it since that is not a command that you just happen upon.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n Hopefully I am using `advice.h` correctly here.\n \n § git-replace(1)\n \n I did not add a hint for a similar message in `builtin/replace.c`.\n \n `builtin/replace.c` has an error message in `check_ref_valid` for an\n invalid ref name:\n \n ```\n return error(_(\"'%s' is not a valid ref name\"), ref->buf);\n ```\n \n But there doesn’t seem to be a point to placing a hint here.\n \n The preceding calls to `repo_get_oid` will catch both missing refs and\n existing refs with invalid names:\n \n ```\n if (repo_get_oid(r, refname, &object))\n \t return error(_(\"failed to resolve '%s' as a valid ref\"), refname);\n ```\n \n Like for example this:\n \n ```\n $ printf $(git rev-parse @~) > .git/refs/heads/hello..goodbye\n $ git replace @ hello..goodbye\n error: failed to resolve 'hello..goodbye' as a valid ref\n […]\n $ git replace @ non-existing\n error: failed to resolve 'non-existing' as a valid ref\n ```\n \n § Alternatives (to this change)\n \n While working on this I also thought that it might be nice to have a\n man page `gitrefsyntax`. That one could use a lot of the content from\n `man git check-ref-format` verbatim. Then the hint could point towards\n that man page. And it seems that AsciiDoc supports _includes_ which\n means that the rules don’t have to be duplicated between the two man\n pages.\n\n branch.c | 7 +++++--\n builtin/branch.c | 7 +++++--\n t/t3200-branch.sh | 10 ++++++++++\n 3 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 6719a181bd1..1386918c60e 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -370,8 +370,11 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n */\n int validate_branchname(const char *name, struct strbuf *ref)\n {\n-\tif (strbuf_check_branch_ref(ref, name))\n-\t\tdie(_(\"'%s' is not a valid branch name\"), name);\n+\tif (strbuf_check_branch_ref(ref, name)) {\n+\t\terror(_(\"'%s' is not a valid branch name\"), name);\n+\t\tadvise(_(\"See `man git check-ref-format`\"));\n+\t\texit(1);\n+\t}\n \n \treturn ref_exists(ref->buf);\n }\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex cfb63cce5fb..fa81e359157 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -576,8 +576,11 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t\t */\n \t\tif (ref_exists(oldref.buf))\n \t\t\trecovery = 1;\n-\t\telse\n-\t\t\tdie(_(\"invalid branch name: '%s'\"), oldname);\n+\t\telse {\n+\t\t\terror(_(\"invalid branch name: '%s'\"), oldname);\n+\t\t\tadvise(_(\"See `man git check-ref-format`\"));\n+\t\t\texit(1);\n+\t\t}\n \t}\n \n \tfor (int i = 0; worktrees[i]; i++) {\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex de7d3014e4f..9400a8baa35 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1725,4 +1725,14 @@ test_expect_success '--track overrides branch.autoSetupMerge' '\n \ttest_cmp_config \"\" --default \"\" branch.foo5.merge\n '\n \n+cat <<\\EOF >expect\n+error: 'foo..bar' is not a valid branch name\n+hint: See `man git check-ref-format`\n+EOF\n+\n+test_expect_success 'errors if given a bad branch name' '\n+\ttest_must_fail git branch foo..bar >actual 2>&1 &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.44.0.106.g650c15c891b\n\n"},{"id":"489747","messageId":"xmqq1q8t7roc.fsf@gitster.g","threadId":"61033","inReplyTo":"d275d1d179b90592ddd7b5da2ae4573b3f7a37b7.1709307442.git.code@khaugsbakk.name","subject":"Re: [PATCH] branch: advise about ref syntax rules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-01T18:06:27Z","receivedAt":"2024-03-01T18:06:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk writes:\n\n> Notes (series):\n> Hopefully I am using `advice.h` correctly here.\n\nLet's see.\n\n> -\tif (strbuf_check_branch_ref(ref, name))\n> -\t\tdie(_(\"'%s' is not a valid branch name\"), name);\n> +\tif (strbuf_check_branch_ref(ref, name)) {\n> +\t\terror(_(\"'%s' is not a valid branch name\"), name);\n> +\t\tadvise(_(\"See `man git check-ref-format`\"));\n> +\t\texit(1);\n> +\t}\n\nThis will give the message with \"hint:\" prefix, which is a good\nstarting point.\n\nThe message is given unconditionally, without any way to turn it\noff. For those who ...\n\n> git-branch(1) will error out if you give it a bad ref name. But the user\n> might not understand why or what part of the name is illegal.\n\n... do not understand why, it is helpful, but once they learned, it\nis one extra line of unwanted text. If you want to give it a way to\nsquelch, see the comment before where enum advice_type is declared\nin advice.h header file. The callsites would become something like\n\n\tadvise_if_enabled(ADVICE_VALID_REF_NAME,\n\t\t_(\"See `man git check-ref-format` for valid refname syntax.\"));\n\nAnother thing is that rewriting die() into error() + advice() +\nmanual exit() is an anti-pattern these days.\n\n\tint code = die_message(_(\"'%s' is not a valid branch name\"), name);\n\tadvice_if_enabled(...); /* see above */\n\texit(code);\n\nIn the same source file, you will find an existing example to mimic.\n\nThanks.\n"},{"id":"489750","messageId":"1ba698b2-a0da-4d62-8174-0ee6d6cd9bbc@app.fastmail.com","threadId":"61033","inReplyTo":"xmqq1q8t7roc.fsf@gitster.g","subject":"Re: [PATCH] branch: advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-01T18:13:33Z","receivedAt":"2024-03-01T18:13:56Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi\n\n> This will give the message with \"hint:\" prefix, which is a good\n> starting point.\n>\n> The message is given unconditionally, without any way to turn it\n> off. For those who ...\n>\n>> git-branch(1) will error out if you give it a bad ref name. But the user\n>> might not understand why or what part of the name is illegal.\n>\n> ... do not understand why, it is helpful, but once they learned, it\n> is one extra line of unwanted text. If you want to give it a way to\n> squelch, see the comment before where enum advice_type is declared\n> in advice.h header file.\n\nI thought of doing that, but I reckoned that people who have a good\nintuition for the ref syntax would not get this error enough to want to\nturn if off.\n\nI’ll add a squelch option in the next version.\n\nCheers\n\n-- \nKristoffer Haugsbakk\n"},{"id":"489754","messageId":"xmqq34t96bvp.fsf@gitster.g","threadId":"61033","inReplyTo":"1ba698b2-a0da-4d62-8174-0ee6d6cd9bbc@app.fastmail.com","subject":"Re: [PATCH] branch: advise about ref syntax rules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-01T18:32:58Z","receivedAt":"2024-03-01T18:33:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> I thought of doing that, but I reckoned that people who have a good\n> intuition for the ref syntax would not get this error enough to want to\n> turn if off.\n\nIf that is your choice, that is perfectly OK, as long as the\nproposed log message clearly records why we did not bother using\nadvice_if_enabled().\n\nIf that is the case, then a rewrite for existing die() would become:\n\n\tint code = die_message(_(\"'%s' is not a valid branch name\"), name);\n\tadvise(_(\"See `man git check-ref-format`\"));\n\texit(code);\n\nThanks.\n"},{"id":"489818","messageId":"cover.1709491818.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"d275d1d179b90592ddd7b5da2ae4573b3f7a37b7.1709307442.git.code@khaugsbakk.name","subject":"[PATCH v2 0/1] advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-03T18:58:20Z","receivedAt":"2024-03-03T18:59:12Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Point the user towards the ref/branch name syntax rules if they give an\ninvalid name.\n\n§ git-replace(1)\n\nI did not add a hint for a similar message in `builtin/replace.c`.\n\n`builtin/replace.c` has an error message in `check_ref_valid` for an\ninvalid ref name:\n\n```\nreturn error(_(\"'%s' is not a valid ref name\"), ref->buf);\n```\n\nBut there doesn’t seem to be a point to placing a hint here.\n\nThe preceding calls to `repo_get_oid` will catch both missing refs and\nexisting refs with invalid names:\n\n```\n if (repo_get_oid(r, refname, &object))\n\t return error(_(\"failed to resolve '%s' as a valid ref\"), refname);\n```\n\nLike for example this:\n\n```\n$ printf $(git rev-parse @~) > .git/refs/heads/hello..goodbye\n$ git replace @ hello..goodbye\nerror: failed to resolve 'hello..goodbye' as a valid ref\n[…]\n$ git replace @ non-existing\nerror: failed to resolve 'non-existing' as a valid ref\n```\n\n§ Alternatives (to this change)\n\nWhile working on this I also thought that it might be nice to have a\nman page `gitrefsyntax`. That one could use a lot of the content from\n`man git check-ref-format` verbatim. Then the hint could point towards\nthat man page. And it seems that AsciiDoc supports _includes_ which\nmeans that the rules don’t have to be duplicated between the two man\npages.\n\n§ Changes in v2\n\n• Make the advise optional via configuration\n • At first I thought that this wasn’t needed but I imagine the advice\n could get repetitive for typos and such\n• Propagate error properly with `die_message(…)` instead of `exit(1)`\n• Flesh out commit message a bit\n\nKristoffer Haugsbakk (1):\n branch: advise about ref syntax rules\n\n Documentation/config/advice.txt | 3 +++\n advice.c | 1 +\n advice.h | 1 +\n branch.c | 8 ++++++--\n builtin/branch.c | 8 ++++++--\n t/t3200-branch.sh | 11 +++++++++++\n 6 files changed, 28 insertions(+), 4 deletions(-)\n\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489819","messageId":"4ad5d4190649dcb5f26c73a6f15ab731891b9dfd.1709491818.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709491818.git.code@khaugsbakk.name","subject":"[PATCH v2 1/1] branch: advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-03T18:58:21Z","receivedAt":"2024-03-03T18:59:18Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"git-branch(1) will error out if you give it a bad ref name. But the user\nmight not understand why or what part of the name is illegal.\n\nThe user might know that there are some limitations based on the *loose\nref* format (filenames), but there are also further rules for\neasier integration with shell-based tools, pathname expansion, and\nplaying well with reference name expressions.\n\nThe man page for git-check-ref-format(1) contains these rules. Let’s\nadvise about it since that is not a command that you just happen\nupon. Also make this advise configurable since you might not want to be\nreminded every time you make a little typo.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n v2:\n • Make the advise optional via configuration\n • Propagate error properly with `die_message(…)` instead of `exit(1)`\n • Flesh out commit message a bit\n\n Documentation/config/advice.txt | 3 +++\n advice.c | 1 +\n advice.h | 1 +\n branch.c | 8 ++++++--\n builtin/branch.c | 8 ++++++--\n t/t3200-branch.sh | 11 +++++++++++\n 6 files changed, 28 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex c7ea70f2e2e..552cfbcd48c 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -94,6 +94,9 @@ advice.*::\n \t\t'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists',\n \t\t'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'\n \t\tsimultaneously.\n+\trefSyntax::\n+\t\tPoint the user towards the ref syntax documentation if\n+\t\tthey give an invalid ref name.\n \tresetNoRefresh::\n \t\tAdvice to consider using the `--no-refresh` option to\n \t\tlinkgit:git-reset[1] when the command takes more than 2 seconds\ndiff --git a/advice.c b/advice.c\nindex 6e9098ff089..550c2968908 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -68,6 +68,7 @@ static struct {\n \t[ADVICE_PUSH_UNQUALIFIED_REF_NAME]\t\t= { \"pushUnqualifiedRefName\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED]\t\t\t= { \"pushUpdateRejected\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]\t\t= { \"pushNonFastForward\" }, /* backwards compatibility */\n+\t[ADVICE_REF_SYNTAX]\t\t\t\t= { \"refSyntax\" },\n \t[ADVICE_RESET_NO_REFRESH_WARNING]\t\t= { \"resetNoRefresh\" },\n \t[ADVICE_RESOLVE_CONFLICT]\t\t\t= { \"resolveConflict\" },\n \t[ADVICE_RM_HINTS]\t\t\t\t= { \"rmHints\" },\ndiff --git a/advice.h b/advice.h\nindex 9d4f49ae38b..d15fe2351ab 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -36,6 +36,7 @@ enum advice_type {\n \tADVICE_PUSH_UNQUALIFIED_REF_NAME,\n \tADVICE_PUSH_UPDATE_REJECTED,\n \tADVICE_PUSH_UPDATE_REJECTED_ALIAS,\n+\tADVICE_REF_SYNTAX,\n \tADVICE_RESET_NO_REFRESH_WARNING,\n \tADVICE_RESOLVE_CONFLICT,\n \tADVICE_RM_HINTS,\ndiff --git a/branch.c b/branch.c\nindex 6719a181bd1..621019fcf4b 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -370,8 +370,12 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n */\n int validate_branchname(const char *name, struct strbuf *ref)\n {\n-\tif (strbuf_check_branch_ref(ref, name))\n-\t\tdie(_(\"'%s' is not a valid branch name\"), name);\n+\tif (strbuf_check_branch_ref(ref, name)) {\n+\t\tint code = die_message(_(\"'%s' is not a valid branch name\"), name);\n+\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n+\t\t\t\t _(\"See `man git check-ref-format`\"));\n+\t\texit(code);\n+\t}\n \n \treturn ref_exists(ref->buf);\n }\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex cfb63cce5fb..1c122ee8a7b 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -576,8 +576,12 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t\t */\n \t\tif (ref_exists(oldref.buf))\n \t\t\trecovery = 1;\n-\t\telse\n-\t\t\tdie(_(\"invalid branch name: '%s'\"), oldname);\n+\t\telse {\n+\t\t\tint code = die_message(_(\"invalid branch name: '%s'\"), oldname);\n+\t\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n+\t\t\t\t\t _(\"See `man git check-ref-format`\"));\n+\t\t\texit(code);\n+\t\t}\n \t}\n \n \tfor (int i = 0; worktrees[i]; i++) {\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex de7d3014e4f..d21fdf09c90 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1725,4 +1725,15 @@ test_expect_success '--track overrides branch.autoSetupMerge' '\n \ttest_cmp_config \"\" --default \"\" branch.foo5.merge\n '\n \n+cat <<\\EOF >expect\n+fatal: 'foo..bar' is not a valid branch name\n+hint: See `man git check-ref-format`\n+hint: Disable this message with \"git config advice.refSyntax false\"\n+EOF\n+\n+test_expect_success 'errors if given a bad branch name' '\n+\ttest_must_fail git branch foo..bar >actual 2>&1 &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489820","messageId":"038da75b-9732-4128-b92a-25f642393b28@app.fastmail.com","threadId":"61033","inReplyTo":"cover.1709491818.git.code@khaugsbakk.name","subject":"Re: [PATCH v2 0/1] advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-03T19:10:44Z","receivedAt":"2024-03-03T19:11:06Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"I forgot the range-diff. But turns out it’s empty.\n\n-- \nKristoffer Haugsbakk\n"},{"id":"489826","messageId":"xmqqil23uebw.fsf@gitster.g","threadId":"61033","inReplyTo":"4ad5d4190649dcb5f26c73a6f15ab731891b9dfd.1709491818.git.code@khaugsbakk.name","subject":"Re: [PATCH v2 1/1] branch: advise about ref syntax rules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-03T22:42:59Z","receivedAt":"2024-03-03T22:43:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk writes:\n\n\nThis has sufficiently been advanced since the previous one, that\nrange-diff would need to be prodded with a larger --creation-factor\nvalue to avoid getting a rather useless output.\n\n1: 5548e6fa34 < -: ---------- branch: advise about ref syntax rules\n-: ---------- > 1: 202d4e29ef branch: advise about ref syntax rules\n\n> git-branch(1) will error out if you give it a bad ref name. But the user\n> might not understand why or what part of the name is illegal.\n>\n> The user might know that there are some limitations based on the *loose\n> ref* format (filenames), but there are also further rules for\n> easier integration with shell-based tools, pathname expansion, and\n> playing well with reference name expressions.\n>\n> The man page for git-check-ref-format(1) contains these rules. Let’s\n> advise about it since that is not a command that you just happen\n> upon. Also make this advise configurable since you might not want to be\n> reminded every time you make a little typo.\n\nNicely written and easily read. Well done.\n\n> +\trefSyntax::\n> +\t\tPoint the user towards the ref syntax documentation if\n> +\t\tthey give an invalid ref name.\n\nI noticed a minor phrasing issue, but many other entries talk about\n\"shown when ...\", even though a handful of them use \"if ...\". Do we\nwant to make them consistent?\n\n> \tresetNoRefresh::\n> \t\tAdvice to consider using the `--no-refresh` option to\n> \t\tlinkgit:git-reset[1] when the command takes more than 2 seconds\n\n> diff --git a/advice.c b/advice.c\n> index 6e9098ff089..550c2968908 100644\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -68,6 +68,7 @@ static struct {\n> \t[ADVICE_PUSH_UNQUALIFIED_REF_NAME]\t\t= { \"pushUnqualifiedRefName\" },\n> \t[ADVICE_PUSH_UPDATE_REJECTED]\t\t\t= { \"pushUpdateRejected\" },\n> \t[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]\t\t= { \"pushNonFastForward\" }, /* backwards compatibility */\n> +\t[ADVICE_REF_SYNTAX]\t\t\t\t= { \"refSyntax\" },\n> \t[ADVICE_RESET_NO_REFRESH_WARNING]\t\t= { \"resetNoRefresh\" },\n> \t[ADVICE_RESOLVE_CONFLICT]\t\t\t= { \"resolveConflict\" },\n> \t[ADVICE_RM_HINTS]\t\t\t\t= { \"rmHints\" },\n> diff --git a/advice.h b/advice.h\n> index 9d4f49ae38b..d15fe2351ab 100644\n> --- a/advice.h\n> +++ b/advice.h\n> @@ -36,6 +36,7 @@ enum advice_type {\n> \tADVICE_PUSH_UNQUALIFIED_REF_NAME,\n> \tADVICE_PUSH_UPDATE_REJECTED,\n> \tADVICE_PUSH_UPDATE_REJECTED_ALIAS,\n> +\tADVICE_REF_SYNTAX,\n> \tADVICE_RESET_NO_REFRESH_WARNING,\n> \tADVICE_RESOLVE_CONFLICT,\n> \tADVICE_RM_HINTS,\n\nBoth of these are in lexicographic order, which is good.\n\n> diff --git a/branch.c b/branch.c\n> index 6719a181bd1..621019fcf4b 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -370,8 +370,12 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n> */\n> int validate_branchname(const char *name, struct strbuf *ref)\n> {\n> -\tif (strbuf_check_branch_ref(ref, name))\n> -\t\tdie(_(\"'%s' is not a valid branch name\"), name);\n> +\tif (strbuf_check_branch_ref(ref, name)) {\n> +\t\tint code = die_message(_(\"'%s' is not a valid branch name\"), name);\n> +\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n> +\t\t\t\t _(\"See `man git check-ref-format`\"));\n> +\t\texit(code);\n> +\t}\n\nNice.\n\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index cfb63cce5fb..1c122ee8a7b 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -576,8 +576,12 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n> \t\t */\n> \t\tif (ref_exists(oldref.buf))\n> \t\t\trecovery = 1;\n> -\t\telse\n> -\t\t\tdie(_(\"invalid branch name: '%s'\"), oldname);\n> +\t\telse {\n> +\t\t\tint code = die_message(_(\"invalid branch name: '%s'\"), oldname);\n> +\t\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n> +\t\t\t\t\t _(\"See `man git check-ref-format`\"));\n> +\t\t\texit(code);\n> +\t\t}\n> \t}\n\nGood, too.\n\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index de7d3014e4f..d21fdf09c90 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -1725,4 +1725,15 @@ test_expect_success '--track overrides branch.autoSetupMerge' '\n> \ttest_cmp_config \"\" --default \"\" branch.foo5.merge\n> '\n> \n> +cat <<\\EOF >expect\n> +fatal: 'foo..bar' is not a valid branch name\n> +hint: See `man git check-ref-format`\n> +hint: Disable this message with \"git config advice.refSyntax false\"\n> +EOF\n> +\n> +test_expect_success 'errors if given a bad branch name' '\n> +\ttest_must_fail git branch foo..bar >actual 2>&1 &&\n> +\ttest_cmp expect actual\n> +'\n\nEven though there are a few ancient style tests that have code to\nset up expectations outside the test_expect_success, most of the\ntests in t3200 do use a more modern style. Let's not make it worse,\nby moving it inside, perhaps like:\n\ntest_expect_success 'errors if given a bad branch name' '\n cat >expect <<-\\EOF &&\n fatal: '\\''foo..bar'\\'' is not a valid branch name\n hint: See `man git check-ref-format`\n hint: Disable this message with \"git config advice.refSyntax false\"\n EOF\n\ttest_must_fail git branch foo..bar >actual 2>&1 &&\n\ttest_cmp expect actual\n'\n\nWe could make a preliminary clean-up to the file in question before\nadding the above test, if we wanted to. Or we can do so after the\ndust settles. Such a fix may look like the attached.\n\nThanks.\n\n t/t3200-branch.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git c/t/t3200-branch.sh w/t/t3200-branch.sh\nindex 94b536ef51..ba1e0eace5 100755\n--- c/t/t3200-branch.sh\n+++ w/t/t3200-branch.sh\n@@ -1112,14 +1112,14 @@ test_expect_success '--set-upstream-to notices an error to set branch as own ups\n \ttest_cmp expect actual\n \"\n \n-# Keep this test last, as it changes the current branch\n-cat >expect <expect <<-EOF &&\n+\t$HEAD refs/heads/g/h/i@{0}: branch: Created from main\n+\tEOF\n \tgit reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&\n \ttest_cmp expect actual\n '\n"},{"id":"489828","messageId":"3fe38b25-64fe-4b35-9cfb-1ab342e29dca@app.fastmail.com","threadId":"61033","inReplyTo":"xmqqil23uebw.fsf@gitster.g","subject":"Re: [PATCH v2 1/1] branch: advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-03T22:58:22Z","receivedAt":"2024-03-03T22:58:43Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Sun, Mar 3, 2024, at 23:42, Junio C Hamano wrote:\n>> +\trefSyntax::\n>> +\t\tPoint the user towards the ref syntax documentation if\n>> +\t\tthey give an invalid ref name.\n>\n> I noticed a minor phrasing issue, but many other entries talk about\n> \"shown when ...\", even though a handful of them use \"if ...\". Do we\n> want to make them consistent?\n\nSure thing. Do you prefer the “shown when” alternative?\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"489921","messageId":"cover.1709590037.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"4ad5d4190649dcb5f26c73a6f15ab731891b9dfd.1709491818.git.code@khaugsbakk.name","subject":"[PATCH v3 0/5] advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-04T22:07:25Z","receivedAt":"2024-03-04T22:08:28Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Point the user towards the ref/branch name syntax rules if they give an\ninvalid name.\n\nAlso make some spatially-appropriate improvements:\n\n• Test style\n• `advice.txt`\n\n§ git-replace(1)\n\n(see previous cover letter)\n\n§ Alternatives (to this change)\n\nWhile working on this I also thought that it might be nice to have a\nman page `gitrefsyntax`. That one could use a lot of the content from\n`man git check-ref-format` verbatim. Then the hint could point towards\nthat man page. And it seems that AsciiDoc supports _includes_ which\nmeans that the rules don’t have to be duplicated between the two man\npages.\n\n§ CC\n\nFor changes to `advice.txt`:\n\nCc: Elijah Newren \nCc: Jean-Noël Avila \n\n§ Changes in v3\n\n• New preliminary patches 1–4\n • Fix test style\n • Improvements to `advice.txt` (style consistency and other)\n• Patch 5/5:\n • Tweak advice doc for the new entry\n • Better test style\n\nKristoffer Haugsbakk (5):\n t3200: improve test style\n advice: make all entries stylistically consistent\n advice: use backticks for code\n advice: use double quotes for regular quoting\n branch: advise about ref syntax rules\n\n Documentation/config/advice.txt | 91 ++++++++++++-----------\n advice.c | 1 +\n advice.h | 1 +\n branch.c | 8 +-\n builtin/branch.c | 8 +-\n t/t3200-branch.sh | 125 +++++++++++++++++---------------\n 6 files changed, 127 insertions(+), 107 deletions(-)\n\nRange-diff against v2:\n-: ----------- > 1: e6a2628ce57 t3200: improve test style\n-: ----------- > 2: d48b4719c27 advice: make all entries stylistically consistent\n-: ----------- > 3: 30d662a04c7 advice: use backticks for code\n-: ----------- > 4: 3028713357f advice: use double quotes for regular quoting\n1: 4ad5d419064 ! 5: 402b7937951 branch: advise about ref syntax rules\n @@ Commit message\n \n \n ## Notes (series) ##\n + v3:\n + • Tweak advice doc for the new entry\n + • Better test style\n v2:\n • Make the advise optional via configuration\n • Propagate error properly with `die_message(…)` instead of `exit(1)`\n @@ Notes (series)\n \n ## Documentation/config/advice.txt ##\n @@ Documentation/config/advice.txt: advice.*::\n - \t\t'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists',\n - \t\t'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'\n + \t\t`pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`,\n + \t\t`pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`\n \t\tsimultaneously.\n +\trefSyntax::\n -+\t\tPoint the user towards the ref syntax documentation if\n -+\t\tthey give an invalid ref name.\n ++\t\tShown when the user provides an illegal ref name: point\n ++\t\ttowards the ref syntax documentation.\n \tresetNoRefresh::\n - \t\tAdvice to consider using the `--no-refresh` option to\n - \t\tlinkgit:git-reset[1] when the command takes more than 2 seconds\n + \t\tShown when linkgit:git-reset[1] takes more than 2\n + \t\tseconds to refresh the index after reset: tell the user\n \n ## advice.c ##\n @@ advice.c: static struct {\n @@ t/t3200-branch.sh: test_expect_success '--track overrides branch.autoSetupMerge'\n \ttest_cmp_config \"\" --default \"\" branch.foo5.merge\n '\n \n -+cat <<\\EOF >expect\n -+fatal: 'foo..bar' is not a valid branch name\n -+hint: See `man git check-ref-format`\n -+hint: Disable this message with \"git config advice.refSyntax false\"\n -+EOF\n -+\n +test_expect_success 'errors if given a bad branch name' '\n ++\tcat <<-\\EOF >expect &&\n ++\tfatal: '\\''foo..bar'\\'' is not a valid branch name\n ++\thint: See `man git check-ref-format`\n ++\thint: Disable this message with \"git config advice.refSyntax false\"\n ++\tEOF\n +\ttest_must_fail git branch foo..bar >actual 2>&1 &&\n +\ttest_cmp expect actual\n +'\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489922","messageId":"e6a2628ce57668aa17101e73edaead0ef34d8a8c.1709590037.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709590037.git.code@khaugsbakk.name","subject":"[PATCH v3 1/5] t3200: improve test style","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-04T22:07:26Z","receivedAt":"2024-03-04T22:08:30Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Some tests use a preliminary heredoc for `expect` or have setup and\nteardown commands before and after, respectively. It is however\npreferred to keep all the logic in the test itself. Let’s move these\ninto the tests.\n\nAlso:\n\n• Remove a now-irrelevant comment about test placement and switch back\n to `main` post-test.\n• Prefer indented literal heredocs (`-\\EOF`) except for a block which\n says that this is intentional\n• Move a `git config` command into the test and mark it as `setup` since\n the next test depends on it\n\nHelped-by: Junio C Hamano \nSigned-off-by: Kristoffer Haugsbakk \n---\n t/t3200-branch.sh | 115 ++++++++++++++++++++++------------------------\n 1 file changed, 56 insertions(+), 59 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex de7d3014e4f..273a57a72d8 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -75,13 +75,13 @@ test_expect_success 'git branch HEAD should fail' '\n \ttest_must_fail git branch HEAD\n '\n \n-cat >expect <expect <<-EOF &&\n+\t$HEAD refs/heads/d/e/f@{0}: branch: Created from main\n+\tEOF\n \tgit reflog show --no-abbrev-commit refs/heads/d/e/f >actual &&\n \ttest_cmp expect actual\n '\n@@ -440,10 +440,10 @@ test_expect_success 'git branch --list -v with --abbrev' '\n \n test_expect_success 'git branch --column' '\n \tCOLUMNS=81 git branch --column=column >actual &&\n-\tcat >expect <<\\EOF &&\n- a/b/c bam foo l * main n o/p r\n- abc bar j/k m/m mb o/o q topic\n-EOF\n+\tcat >expect <<-\\EOF &&\n+\t a/b/c bam foo l * main n o/p r\n+\t abc bar j/k m/m mb o/o q topic\n+\tEOF\n \ttest_cmp expect actual\n '\n \n@@ -453,25 +453,25 @@ test_expect_success 'git branch --column with an extremely long branch name' '\n \ttest_when_finished \"git branch -d $long\" &&\n \tgit branch $long &&\n \tCOLUMNS=80 git branch --column=column >actual &&\n-\tcat >expect <expect <<-EOF &&\n+\t a/b/c\n+\t abc\n+\t bam\n+\t bar\n+\t foo\n+\t j/k\n+\t l\n+\t m/m\n+\t* main\n+\t mb\n+\t n\n+\t o/o\n+\t o/p\n+\t q\n+\t r\n+\t topic\n+\t $long\n+\tEOF\n \ttest_cmp expect actual\n '\n \n@@ -481,10 +481,10 @@ test_expect_success 'git branch with column.*' '\n \tCOLUMNS=80 git branch >actual &&\n \tgit config --unset column.branch &&\n \tgit config --unset column.ui &&\n-\tcat >expect <<\\EOF &&\n- a/b/c bam foo l * main n o/p r\n- abc bar j/k m/m mb o/o q topic\n-EOF\n+\tcat >expect <<-\\EOF &&\n+\t a/b/c bam foo l * main n o/p r\n+\t abc bar j/k m/m mb o/o q topic\n+\tEOF\n \ttest_cmp expect actual\n '\n \n@@ -496,39 +496,36 @@ test_expect_success 'git branch -v with column.ui ignored' '\n \tgit config column.ui column &&\n \tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n \tgit config --unset column.ui &&\n-\tcat >expect <<\\EOF &&\n- a/b/c\n- abc\n- bam\n- bar\n- foo\n- j/k\n- l\n- m/m\n-* main\n- mb\n- n\n- o/o\n- o/p\n- q\n- r\n- topic\n-EOF\n+\tcat >expect <<-\\EOF &&\n+\t a/b/c\n+\t abc\n+\t bam\n+\t bar\n+\t foo\n+\t j/k\n+\t l\n+\t m/m\n+\t* main\n+\t mb\n+\t n\n+\t o/o\n+\t o/p\n+\t q\n+\t r\n+\t topic\n+\tEOF\n \ttest_cmp expect actual\n '\n \n-mv .git/config .git/config-saved\n-\n test_expect_success DEFAULT_REPO_FORMAT 'git branch -m q q2 without config should succeed' '\n+\ttest_when_finished mv .git/config-saved .git/config &&\n+\tmv .git/config .git/config-saved &&\n \tgit branch -m q q2 &&\n \tgit branch -m q2 q\n '\n \n-mv .git/config-saved .git/config\n-\n-git config branch.s/s.dummy Hello\n-\n-test_expect_success 'git branch -m s/s s should work when s/t is deleted' '\n+test_expect_success '(setup) git branch -m s/s s should work when s/t is deleted' '\n+\tgit config branch.s/s.dummy Hello &&\n \tgit branch --create-reflog s/s &&\n \tgit reflog exists refs/heads/s/s &&\n \tgit branch --create-reflog s/t &&\n@@ -1141,14 +1138,14 @@ test_expect_success '--set-upstream-to notices an error to set branch as own ups\n \ttest_cmp expect actual\n \"\n \n-# Keep this test last, as it changes the current branch\n-cat >expect <expect <<-EOF &&\n+\t$HEAD refs/heads/g/h/i@{0}: branch: Created from main\n+\tEOF\n \tgit reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&\n \ttest_cmp expect actual\n '\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489923","messageId":"d48b4719c275ef06da014b6d22983db9ae484db2.1709590037.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709590037.git.code@khaugsbakk.name","subject":"[PATCH v3 2/5] advice: make all entries stylistically consistent","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-04T22:07:27Z","receivedAt":"2024-03-04T22:08:31Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"1. Use “shown” instead of “advice shown”\n • “advice” is implied and a bit repetitive\n2. Use “when” instead of “if”\n3. Lead with “Shown when” and end the entry with the effect it has,\n where applicable\n4. Use “the user” instead of “a user” or “you”\n5. detachedHead: connect clause with a semicolon to make the sentence\n flow better in this new context\n6. implicitIdentity: rewrite description in order to lead with *when*\n the advice is shown (see point (3))\n7. Prefer the present tense (with the exception of pushNonFFMatching)\n8. Use a colon to connect the last clause instead of a comma\n9. waitingForEditor: give example of relevance in this new context\n10. pushUpdateRejected: exception to the above principles\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n Maybe the style that we eventually agree on should be documented outside the\n commit log?\n\n Documentation/config/advice.txt | 80 ++++++++++++++++-----------------\n 1 file changed, 40 insertions(+), 40 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex c7ea70f2e2e..cfca87a6aa2 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -6,23 +6,23 @@ advice.*::\n +\n --\n \taddEmbeddedRepo::\n-\t\tAdvice on what to do when you've accidentally added one\n+\t\tShown when the user accidentally adds one\n \t\tgit repo inside of another.\n \taddEmptyPathspec::\n-\t\tAdvice shown if a user runs the add command without providing\n+\t\tShown when the user runs the add command without providing\n \t\tthe pathspec parameter.\n \taddIgnoredFile::\n-\t\tAdvice shown if a user attempts to add an ignored file to\n+\t\tShown when the user attempts to add an ignored file to\n \t\tthe index.\n \tamWorkDir::\n-\t\tAdvice that shows the location of the patch file when\n-\t\tlinkgit:git-am[1] fails to apply it.\n+\t\tShown when linkgit:git-am[1] fails to apply a patch\n+\t\tfile: tell the location of the file.\n \tambiguousFetchRefspec::\n-\t\tAdvice shown when a fetch refspec for multiple remotes maps to\n+\t\tShown when a fetch refspec for multiple remotes maps to\n \t\tthe same remote-tracking branch namespace and causes branch\n \t\ttracking set-up to fail.\n \tcheckoutAmbiguousRemoteBranchName::\n-\t\tAdvice shown when the argument to\n+\t\tShown when the argument to\n \t\tlinkgit:git-checkout[1] and linkgit:git-switch[1]\n \t\tambiguously resolves to a\n \t\tremote tracking branch on more than one remote in\n@@ -33,31 +33,31 @@ advice.*::\n \t\tto be used by default in some situations where this\n \t\tadvice would be printed.\n \tcommitBeforeMerge::\n-\t\tAdvice shown when linkgit:git-merge[1] refuses to\n+\t\tShown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwriting local changes.\n \tdetachedHead::\n-\t\tAdvice shown when you used\n+\t\tShown when the user uses\n \t\tlinkgit:git-switch[1] or linkgit:git-checkout[1]\n-\t\tto move to the detached HEAD state, to instruct how to\n+\t\tto move to the detached HEAD state; instruct how to\n \t\tcreate a local branch after the fact.\n \tdiverging::\n-\t\tAdvice shown when a fast-forward is not possible.\n+\t\tShown when a fast-forward is not possible.\n \tfetchShowForcedUpdates::\n-\t\tAdvice shown when linkgit:git-fetch[1] takes a long time\n+\t\tShown when linkgit:git-fetch[1] takes a long time\n \t\tto calculate forced updates after ref updates, or to warn\n \t\tthat the check is disabled.\n \tforceDeleteBranch::\n-\t\tAdvice shown when a user tries to delete a not fully merged\n+\t\tShown when the user tries to delete a not fully merged\n \t\tbranch without the force option set.\n \tignoredHook::\n-\t\tAdvice shown if a hook is ignored because the hook is not\n+\t\tShown when a hook is ignored because the hook is not\n \t\tset as executable.\n \timplicitIdentity::\n-\t\tAdvice on how to set your identity configuration when\n-\t\tyour information is guessed from the system username and\n-\t\tdomain name.\n+\t\tShown when the user's information is guessed from the\n+\t\tsystem username and domain name: tell the user how to\n+\t\tset their identity configuration.\n \tnestedTag::\n-\t\tAdvice shown if a user attempts to recursively tag a tag object.\n+\t\tShown when a user attempts to recursively tag a tag object.\n \tpushAlreadyExists::\n \t\tShown when linkgit:git-push[1] rejects an update that\n \t\tdoes not qualify for fast-forwarding (e.g., a tag.)\n@@ -71,12 +71,12 @@ advice.*::\n \t\tobject that is not a commit-ish, or make the remote\n \t\tref point at an object that is not a commit-ish.\n \tpushNonFFCurrent::\n-\t\tAdvice shown when linkgit:git-push[1] fails due to a\n+\t\tShown when linkgit:git-push[1] fails due to a\n \t\tnon-fast-forward update to the current branch.\n \tpushNonFFMatching::\n-\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n-\t\t'matching refs' explicitly (i.e. you used ':', or\n-\t\tspecified a refspec that isn't your current branch) and\n+\t\tShown when the user ran linkgit:git-push[1] and pushed\n+\t\t'matching refs' explicitly (i.e. used ':', or\n+\t\tspecified a refspec that isn't the current branch) and\n \t\tit resulted in a non-fast-forward error.\n \tpushRefNeedsUpdate::\n \t\tShown when linkgit:git-push[1] rejects a forced update of\n@@ -95,17 +95,17 @@ advice.*::\n \t\t'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'\n \t\tsimultaneously.\n \tresetNoRefresh::\n-\t\tAdvice to consider using the `--no-refresh` option to\n-\t\tlinkgit:git-reset[1] when the command takes more than 2 seconds\n-\t\tto refresh the index after reset.\n+\t\tShown when linkgit:git-reset[1] takes more than 2\n+\t\tseconds to refresh the index after reset: tell the user\n+\t\tthat they can use the `--no-refresh` option.\n \tresolveConflict::\n-\t\tAdvice shown by various commands when conflicts\n+\t\tShown by various commands when conflicts\n \t\tprevent the operation from being performed.\n \trmHints::\n-\t\tIn case of failure in the output of linkgit:git-rm[1],\n-\t\tshow directions on how to proceed from the current state.\n+\t\tShown on failure in the output of linkgit:git-rm[1]:\n+\t\tgive directions on how to proceed from the current state.\n \tsequencerInUse::\n-\t\tAdvice shown when a sequencer command is already in progress.\n+\t\tShown when a sequencer command is already in progress.\n \tskippedCherryPicks::\n \t\tShown when linkgit:git-rebase[1] skips a commit that has already\n \t\tbeen cherry-picked onto the upstream branch.\n@@ -123,27 +123,27 @@ advice.*::\n \t\tby linkgit:git-switch[1] or\n \t\tlinkgit:git-checkout[1] when switching branches.\n \tstatusUoption::\n-\t\tAdvise to consider using the `-u` option to linkgit:git-status[1]\n-\t\twhen the command takes more than 2 seconds to enumerate untracked\n-\t\tfiles.\n+\t\tShown when linkgit:git-status[1] takes more than 2\n+\t\tseconds to enumerate untracked files: consider using the\n+\t\t`-u` option.\n \tsubmoduleAlternateErrorStrategyDie::\n-\t\tAdvice shown when a submodule.alternateErrorStrategy option\n+\t\tShown when a submodule.alternateErrorStrategy option\n \t\tconfigured to \"die\" causes a fatal error.\n \tsubmodulesNotUpdated::\n-\t\tAdvice shown when a user runs a submodule command that fails\n+\t\tShown when a user runs a submodule command that fails\n \t\tbecause `git submodule update --init` was not run.\n \tsuggestDetachingHead::\n-\t\tAdvice shown when linkgit:git-switch[1] refuses to detach HEAD\n+\t\tShown when linkgit:git-switch[1] refuses to detach HEAD\n \t\twithout the explicit `--detach` option.\n \tupdateSparsePath::\n-\t\tAdvice shown when either linkgit:git-add[1] or linkgit:git-rm[1]\n+\t\tShown when either linkgit:git-add[1] or linkgit:git-rm[1]\n \t\tis asked to update index entries outside the current sparse\n \t\tcheckout.\n \twaitingForEditor::\n-\t\tPrint a message to the terminal whenever Git is waiting for\n-\t\teditor input from the user.\n+\t\tShown when Git is waiting for editor input. Relevant\n+\t\twhen e.g. the editor is not launched inside the terminal.\n \tworktreeAddOrphan::\n-\t\tAdvice shown when a user tries to create a worktree from an\n-\t\tinvalid reference, to instruct how to create a new unborn\n+\t\tShown when the user tries to create a worktree from an\n+\t\tinvalid reference: instruct how to create a new unborn\n \t\tbranch instead.\n --\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489924","messageId":"30d662a04c75b80166db9ef94f95e8a841994fb5.1709590037.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709590037.git.code@khaugsbakk.name","subject":"[PATCH v3 3/5] advice: use backticks for code","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-04T22:07:28Z","receivedAt":"2024-03-04T22:08:33Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Use backticks for quoting code rather than single quotes.\n\nAlso replace “the add command” with “`git add`”.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n Documentation/config/advice.txt | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex cfca87a6aa2..df447dd5d14 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -2,14 +2,14 @@ advice.*::\n \tThese variables control various optional help messages designed to\n \taid new users. When left unconfigured, Git will give the message\n \talongside instructions on how to squelch it. You can tell Git\n-\tthat you do not need the help message by setting these to 'false':\n+\tthat you do not need the help message by setting these to `false`:\n +\n --\n \taddEmbeddedRepo::\n \t\tShown when the user accidentally adds one\n \t\tgit repo inside of another.\n \taddEmptyPathspec::\n-\t\tShown when the user runs the add command without providing\n+\t\tShown when the user runs `git add` without providing\n \t\tthe pathspec parameter.\n \taddIgnoredFile::\n \t\tShown when the user attempts to add an ignored file to\n@@ -75,7 +75,7 @@ advice.*::\n \t\tnon-fast-forward update to the current branch.\n \tpushNonFFMatching::\n \t\tShown when the user ran linkgit:git-push[1] and pushed\n-\t\t'matching refs' explicitly (i.e. used ':', or\n+\t\t'matching refs' explicitly (i.e. used `:`, or\n \t\tspecified a refspec that isn't the current branch) and\n \t\tit resulted in a non-fast-forward error.\n \tpushRefNeedsUpdate::\n@@ -90,9 +90,9 @@ advice.*::\n \t\trefs/heads/* or refs/tags/* based on the type of the\n \t\tsource object.\n \tpushUpdateRejected::\n-\t\tSet this variable to 'false' if you want to disable\n-\t\t'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists',\n-\t\t'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'\n+\t\tSet this variable to `false` if you want to disable\n+\t\t`pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`,\n+\t\t`pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`\n \t\tsimultaneously.\n \tresetNoRefresh::\n \t\tShown when linkgit:git-reset[1] takes more than 2\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489925","messageId":"3028713357ff77f33c1f96b05b566279683808ac.1709590037.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709590037.git.code@khaugsbakk.name","subject":"[PATCH v3 4/5] advice: use double quotes for regular quoting","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-04T22:07:29Z","receivedAt":"2024-03-04T22:08:34Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Use double quotes like we use for “die” in this document.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n Documentation/config/advice.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex df447dd5d14..c5d3d6790a5 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -75,7 +75,7 @@ advice.*::\n \t\tnon-fast-forward update to the current branch.\n \tpushNonFFMatching::\n \t\tShown when the user ran linkgit:git-push[1] and pushed\n-\t\t'matching refs' explicitly (i.e. used `:`, or\n+\t\t\"matching refs\" explicitly (i.e. used `:`, or\n \t\tspecified a refspec that isn't the current branch) and\n \t\tit resulted in a non-fast-forward error.\n \tpushRefNeedsUpdate::\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489926","messageId":"402b7937951073466bf4527caffd38175391c7da.1709590037.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709590037.git.code@khaugsbakk.name","subject":"[PATCH v3 5/5] branch: advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-04T22:07:30Z","receivedAt":"2024-03-04T22:08:35Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"git-branch(1) will error out if you give it a bad ref name. But the user\nmight not understand why or what part of the name is illegal.\n\nThe user might know that there are some limitations based on the *loose\nref* format (filenames), but there are also further rules for\neasier integration with shell-based tools, pathname expansion, and\nplaying well with reference name expressions.\n\nThe man page for git-check-ref-format(1) contains these rules. Let’s\nadvise about it since that is not a command that you just happen\nupon. Also make this advise configurable since you might not want to be\nreminded every time you make a little typo.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n v3:\n • Tweak advice doc for the new entry\n • Better test style\n v2:\n • Make the advise optional via configuration\n • Propagate error properly with `die_message(…)` instead of `exit(1)`\n • Flesh out commit message a bit\n\n Documentation/config/advice.txt | 3 +++\n advice.c | 1 +\n advice.h | 1 +\n branch.c | 8 ++++++--\n builtin/branch.c | 8 ++++++--\n t/t3200-branch.sh | 10 ++++++++++\n 6 files changed, 27 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex c5d3d6790a5..06a3a3cc9b5 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -94,6 +94,9 @@ advice.*::\n \t\t`pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`,\n \t\t`pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`\n \t\tsimultaneously.\n+\trefSyntax::\n+\t\tShown when the user provides an illegal ref name: point\n+\t\ttowards the ref syntax documentation.\n \tresetNoRefresh::\n \t\tShown when linkgit:git-reset[1] takes more than 2\n \t\tseconds to refresh the index after reset: tell the user\ndiff --git a/advice.c b/advice.c\nindex 6e9098ff089..550c2968908 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -68,6 +68,7 @@ static struct {\n \t[ADVICE_PUSH_UNQUALIFIED_REF_NAME]\t\t= { \"pushUnqualifiedRefName\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED]\t\t\t= { \"pushUpdateRejected\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]\t\t= { \"pushNonFastForward\" }, /* backwards compatibility */\n+\t[ADVICE_REF_SYNTAX]\t\t\t\t= { \"refSyntax\" },\n \t[ADVICE_RESET_NO_REFRESH_WARNING]\t\t= { \"resetNoRefresh\" },\n \t[ADVICE_RESOLVE_CONFLICT]\t\t\t= { \"resolveConflict\" },\n \t[ADVICE_RM_HINTS]\t\t\t\t= { \"rmHints\" },\ndiff --git a/advice.h b/advice.h\nindex 9d4f49ae38b..d15fe2351ab 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -36,6 +36,7 @@ enum advice_type {\n \tADVICE_PUSH_UNQUALIFIED_REF_NAME,\n \tADVICE_PUSH_UPDATE_REJECTED,\n \tADVICE_PUSH_UPDATE_REJECTED_ALIAS,\n+\tADVICE_REF_SYNTAX,\n \tADVICE_RESET_NO_REFRESH_WARNING,\n \tADVICE_RESOLVE_CONFLICT,\n \tADVICE_RM_HINTS,\ndiff --git a/branch.c b/branch.c\nindex 6719a181bd1..621019fcf4b 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -370,8 +370,12 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n */\n int validate_branchname(const char *name, struct strbuf *ref)\n {\n-\tif (strbuf_check_branch_ref(ref, name))\n-\t\tdie(_(\"'%s' is not a valid branch name\"), name);\n+\tif (strbuf_check_branch_ref(ref, name)) {\n+\t\tint code = die_message(_(\"'%s' is not a valid branch name\"), name);\n+\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n+\t\t\t\t _(\"See `man git check-ref-format`\"));\n+\t\texit(code);\n+\t}\n \n \treturn ref_exists(ref->buf);\n }\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex cfb63cce5fb..1c122ee8a7b 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -576,8 +576,12 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t\t */\n \t\tif (ref_exists(oldref.buf))\n \t\t\trecovery = 1;\n-\t\telse\n-\t\t\tdie(_(\"invalid branch name: '%s'\"), oldname);\n+\t\telse {\n+\t\t\tint code = die_message(_(\"invalid branch name: '%s'\"), oldname);\n+\t\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n+\t\t\t\t\t _(\"See `man git check-ref-format`\"));\n+\t\t\texit(code);\n+\t\t}\n \t}\n \n \tfor (int i = 0; worktrees[i]; i++) {\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 273a57a72d8..30a97e3776e 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1722,4 +1722,14 @@ test_expect_success '--track overrides branch.autoSetupMerge' '\n \ttest_cmp_config \"\" --default \"\" branch.foo5.merge\n '\n \n+test_expect_success 'errors if given a bad branch name' '\n+\tcat <<-\\EOF >expect &&\n+\tfatal: '\\''foo..bar'\\'' is not a valid branch name\n+\thint: See `man git check-ref-format`\n+\thint: Disable this message with \"git config advice.refSyntax false\"\n+\tEOF\n+\ttest_must_fail git branch foo..bar >actual 2>&1 &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489930","messageId":"xmqq4jdlmu6q.fsf@gitster.g","threadId":"61033","inReplyTo":"d48b4719c275ef06da014b6d22983db9ae484db2.1709590037.git.code@khaugsbakk.name","subject":"Re: [PATCH v3 2/5] advice: make all entries stylistically consistent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-04T23:52:13Z","receivedAt":"2024-03-04T23:52:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk writes:\n\n> 1. Use “shown” instead of “advice shown”\n> • “advice” is implied and a bit repetitive\n> 2. Use “when” instead of “if”\n> 3. Lead with “Shown when” and end the entry with the effect it has,\n> where applicable\n> 4. Use “the user” instead of “a user” or “you”\n> 5. detachedHead: connect clause with a semicolon to make the sentence\n> flow better in this new context\n> 6. implicitIdentity: rewrite description in order to lead with *when*\n> the advice is shown (see point (3))\n> 7. Prefer the present tense (with the exception of pushNonFFMatching)\n> 8. Use a colon to connect the last clause instead of a comma\n> 9. waitingForEditor: give example of relevance in this new context\n> 10. pushUpdateRejected: exception to the above principles\n\nI'll let others comment on these as general principles. I do not\nimmediately see anything objectionable, but I may change my mind\nafter reading the updated text in the patch.\n\n> Suggested-by: Junio C Hamano \n\nI am getting too much credit for this; I merely suggested to use\n\"when\" instead of \"if\" in the one you are newly adding.\n\n> \tdetachedHead::\n> -\t\tAdvice shown when you used\n> +\t\tShown when the user uses\n> \t\tlinkgit:git-switch[1] or linkgit:git-checkout[1]\n> -\t\tto move to the detached HEAD state, to instruct how to\n> +\t\tto move to the detached HEAD state; instruct how to\n> \t\tcreate a local branch after the fact.\n\nI agree \"Advice shown when\" -> \"Shown when\" is a good change for\nbrevity, but I do not think the other change is an improvement.\n\nThis advice message is shown when the user does X, in order to\ninstruct the user how to do Y after that. And \"to instruct\" is a\ncommon way to say the same thing as \"in order to instruct\".\n\n> \timplicitIdentity::\n> -\t\tAdvice on how to set your identity configuration when\n> -\t\tyour information is guessed from the system username and\n> -\t\tdomain name.\n> +\t\tShown when the user's information is guessed from the\n> +\t\tsystem username and domain name: tell the user how to\n> +\t\tset their identity configuration.\n\nShould that be a colon? Stopping a half-sentence and connecting to\nanother half-sentence is usually done with a semicolon (like you did\nin the new version of detachedHEAD above).\n\n\tShown when ... and domain name, to tell the user how to set\n\ttheir identity configuration.\n\nperhaps? There may be other similar entries whose updated text uses\ncolon followed by an imperative sentence, but I didn't look very\ncarefully.\n\n> \tstatusUoption::\n> -\t\tAdvise to consider using the `-u` option to linkgit:git-status[1]\n> -\t\twhen the command takes more than 2 seconds to enumerate untracked\n> -\t\tfiles.\n> +\t\tShown when linkgit:git-status[1] takes more than 2\n> +\t\tseconds to enumerate untracked files: consider using the\n> +\t\t`-u` option.\n\nEarlier ones after a colon (or semicolon in detachedHEAD case), you\ngave an order to the advice message (e.g. \"hey detachedHead advice,\ntell the user how to create a local branch\"), but this one is giving\nan order to the end user, which feels inconsistent.\n\nI do not have a strong objection against giving an order to the\nadvice message, as long as it is done consistently. If we did so,\nthe part after the colon would start with \"instruct the user ...\" or\n\"tell the user ...\" and the like, and the gist of what this one\nwould say would be \"shown when it is taking too long: suggest the\nuser to consider `-u`\".\n\nFWIW, my earlier \"in order to\" took an approach that is different\nfrom either of the two \"giving an order\" approaches. I was trying\nto make the description explain what the message tries to do and/or\nwhy the message is given (e.g., \"shown if it takes too long in order\nto suggest users to consider the -u option\").\n\nThanks.\n"},{"id":"489932","messageId":"xmqqzfvdlfhp.fsf@gitster.g","threadId":"61033","inReplyTo":"30d662a04c75b80166db9ef94f95e8a841994fb5.1709590037.git.code@khaugsbakk.name","subject":"Re: [PATCH v3 3/5] advice: use backticks for code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-04T23:54:58Z","receivedAt":"2024-03-04T23:55:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk writes:\n\n> Use backticks for quoting code rather than single quotes.\n\nGood. Technically it does not have to be \"code\", but rather what\nthe user would literally type from their keyboard verbatim, but\n\"quoting code\" is so concise way to describe, it probably is good\nenough hint for future developers who will find this commit via \"git\nblame\" and read \"git show\" to read this explanation.\n\n> Also replace “the add command” with “`git add`”.\n>\n> Signed-off-by: Kristoffer Haugsbakk \n> ---\n> Documentation/config/advice.txt | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\n> index cfca87a6aa2..df447dd5d14 100644\n> --- a/Documentation/config/advice.txt\n> +++ b/Documentation/config/advice.txt\n> @@ -2,14 +2,14 @@ advice.*::\n> \tThese variables control various optional help messages designed to\n> \taid new users. When left unconfigured, Git will give the message\n> \talongside instructions on how to squelch it. You can tell Git\n> -\tthat you do not need the help message by setting these to 'false':\n> +\tthat you do not need the help message by setting these to `false`:\n> +\n> --\n> \taddEmbeddedRepo::\n> \t\tShown when the user accidentally adds one\n> \t\tgit repo inside of another.\n> \taddEmptyPathspec::\n> -\t\tShown when the user runs the add command without providing\n> +\t\tShown when the user runs `git add` without providing\n> \t\tthe pathspec parameter.\n> \taddIgnoredFile::\n> \t\tShown when the user attempts to add an ignored file to\n> @@ -75,7 +75,7 @@ advice.*::\n> \t\tnon-fast-forward update to the current branch.\n> \tpushNonFFMatching::\n> \t\tShown when the user ran linkgit:git-push[1] and pushed\n> -\t\t'matching refs' explicitly (i.e. used ':', or\n> +\t\t'matching refs' explicitly (i.e. used `:`, or\n> \t\tspecified a refspec that isn't the current branch) and\n> \t\tit resulted in a non-fast-forward error.\n> \tpushRefNeedsUpdate::\n> @@ -90,9 +90,9 @@ advice.*::\n> \t\trefs/heads/* or refs/tags/* based on the type of the\n> \t\tsource object.\n> \tpushUpdateRejected::\n> -\t\tSet this variable to 'false' if you want to disable\n> -\t\t'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists',\n> -\t\t'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'\n> +\t\tSet this variable to `false` if you want to disable\n> +\t\t`pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`,\n> +\t\t`pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`\n> \t\tsimultaneously.\n> \tresetNoRefresh::\n> \t\tShown when linkgit:git-reset[1] takes more than 2\n"},{"id":"489937","messageId":"xmqqplw9lbav.fsf@gitster.g","threadId":"61033","inReplyTo":"e6a2628ce57668aa17101e73edaead0ef34d8a8c.1709590037.git.code@khaugsbakk.name","subject":"Re: [PATCH v3 1/5] t3200: improve test style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-05T01:25:28Z","receivedAt":"2024-03-05T01:25:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk writes:\n\n> Also:\n>\n> • Remove a now-irrelevant comment about test placement and switch back\n> to `main` post-test.\n> • Prefer indented literal heredocs (`-\\EOF`) except for a block which\n> says that this is intentional\n> • Move a `git config` command into the test and mark it as `setup` since\n> the next test depends on it\n>\n\nEspecially the change to use \"-\\EOF\" to make them align better\ncaused too many tests to be touched, but overall the result may have\nbecome much easier to follow. Good job.\n\n> -mv .git/config .git/config-saved\n> -\n> test_expect_success DEFAULT_REPO_FORMAT 'git branch -m q q2 without config should succeed' '\n> +\ttest_when_finished mv .git/config-saved .git/config &&\n> +\tmv .git/config .git/config-saved &&\n> \tgit branch -m q q2 &&\n> \tgit branch -m q2 q\n> '\n> \n> -mv .git/config-saved .git/config\n\nThe above is a truly valuable clean-up.\n\nBut I am not really sure if the paritcular condition is worth\ntesting in the first place these days. No configuration file means\nwe cannot even read the repository format version, and working under\nsuch a condition is quite a bad promise that we would rather not to\nhaving to keep. But that is an entirely different topic from what\nthis patch is doing.\n\n> -git config branch.s/s.dummy Hello\n> -\n> -test_expect_success 'git branch -m s/s s should work when s/t is deleted' '\n> +test_expect_success '(setup) git branch -m s/s s should work when s/t is deleted' '\n> +\tgit config branch.s/s.dummy Hello &&\n> \tgit branch --create-reflog s/s &&\n> \tgit reflog exists refs/heads/s/s &&\n> \tgit branch --create-reflog s/t &&\n\nI do not know if the change of the title is warranted. It is doing\nits own test, not just setup. It may be merely donw for the side\neffect of making the step unskippable, but still ....\n\n> -# Keep this test last, as it changes the current branch\n\nYes, removal of this line is really appreciated ;-)\n\n> -cat >expect < -$HEAD refs/heads/g/h/i@{0}: branch: Created from main\n> -EOF\n> test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '\n> +\ttest_when_finished git checkout main &&\n> \tGIT_COMMITTER_DATE=\"2005-05-26 23:30\" \\\n> \tgit checkout -b g/h/i -l main &&\n> \ttest_ref_exists refs/heads/g/h/i &&\n> +\tcat >expect <<-EOF &&\n> +\t$HEAD refs/heads/g/h/i@{0}: branch: Created from main\n> +\tEOF\n> \tgit reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&\n> \ttest_cmp expect actual\n> '\n\nThanks.\n"},{"id":"489947","messageId":"166d2baa-933c-44f8-b6fb-94c8bce63a86@app.fastmail.com","threadId":"61033","inReplyTo":"xmqqplw9lbav.fsf@gitster.g","subject":"Re: [PATCH v3 1/5] t3200: improve test style","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T10:27:48Z","receivedAt":"2024-03-05T10:28:11Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Mar 5, 2024, at 02:25, Junio C Hamano wrote:\n> Especially the change to use \"-\\EOF\" to make them align better\n> caused too many tests to be touched, but overall the result may have\n> become much easier to follow. Good job.\n\nI reckon that this can be worth doing now as long as no other topics in\n`next` or `seen` happen to touch the same code. What do you think? I can\nevict hunks if they happen to overlap with other in-flight topics.\n\n>> -mv .git/config .git/config-saved\n>> -\n>> test_expect_success DEFAULT_REPO_FORMAT 'git branch -m q q2 without config should succeed' '\n>> +\ttest_when_finished mv .git/config-saved .git/config &&\n>> +\tmv .git/config .git/config-saved &&\n>> \tgit branch -m q q2 &&\n>> \tgit branch -m q2 q\n>> '\n>>\n>> -mv .git/config-saved .git/config\n>\n> The above is a truly valuable clean-up.\n>\n> But I am not really sure if the paritcular condition is worth\n> testing in the first place these days. No configuration file means\n> we cannot even read the repository format version, and working under\n> such a condition is quite a bad promise that we would rather not to\n> having to keep. But that is an entirely different topic from what\n> this patch is doing.\n\nOkay. I could undo this change and remove the test in its own commit?\n\n>\n>> -git config branch.s/s.dummy Hello\n>> -\n>> -test_expect_success 'git branch -m s/s s should work when s/t is deleted' '\n>> +test_expect_success '(setup) git branch -m s/s s should work when s/t is deleted' '\n>> +\tgit config branch.s/s.dummy Hello &&\n>> \tgit branch --create-reflog s/s &&\n>> \tgit reflog exists refs/heads/s/s &&\n>> \tgit branch --create-reflog s/t &&\n>\n> I do not know if the change of the title is warranted. It is doing\n> its own test, not just setup. It may be merely donw for the side\n> effect of making the step unskippable, but still ....\n\nSure, I’ll remove `(setup)`. The test name suggests that the test\ndepends on the previous one in any case.\n"},{"id":"489948","messageId":"3d8556af-5ff9-4e5c-a2f0-05480d1feba9@app.fastmail.com","threadId":"61033","inReplyTo":"xmqqzfvdlfhp.fsf@gitster.g","subject":"Re: [PATCH v3 3/5] advice: use backticks for code","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T10:29:34Z","receivedAt":"2024-03-05T10:29:57Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Mar 5, 2024, at 00:54, Junio C Hamano wrote:\n>> Use backticks for quoting code rather than single quotes.\n>\n> Good. Technically it does not have to be \"code\", but rather what\n> the user would literally type from their keyboard verbatim, but\n> \"quoting code\" is so concise way to describe, it probably is good\n> enough hint for future developers who will find this commit via \"git\n> blame\" and read \"git show\" to read this explanation.\n\nI agree. Either works fine but “verbatim” is a more general term. I’ll\nuse that.\n"},{"id":"489949","messageId":"83b2748f-0af9-4fce-a88d-a016e85f91ef@app.fastmail.com","threadId":"61033","inReplyTo":"xmqq4jdlmu6q.fsf@gitster.g","subject":"Re: [PATCH v3 2/5] advice: make all entries stylistically consistent","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T10:36:39Z","receivedAt":"2024-03-05T10:37:06Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Mar 5, 2024, at 00:52, Junio C Hamano wrote:\n>> \tdetachedHead::\n>> -\t\tAdvice shown when you used\n>> +\t\tShown when the user uses\n>> \t\tlinkgit:git-switch[1] or linkgit:git-checkout[1]\n>> -\t\tto move to the detached HEAD state, to instruct how to\n>> +\t\tto move to the detached HEAD state; instruct how to\n>> \t\tcreate a local branch after the fact.\n>\n> I agree \"Advice shown when\" -> \"Shown when\" is a good change for\n> brevity, but I do not think the other change is an improvement.\n>\n> This advice message is shown when the user does X, in order to\n> instruct the user how to do Y after that. And \"to instruct\" is a\n> common way to say the same thing as \"in order to instruct\".\n\nWell argued. I’ll go back to the comma.\n\n>> \timplicitIdentity::\n>> -\t\tAdvice on how to set your identity configuration when\n>> -\t\tyour information is guessed from the system username and\n>> -\t\tdomain name.\n>> +\t\tShown when the user's information is guessed from the\n>> +\t\tsystem username and domain name: tell the user how to\n>> +\t\tset their identity configuration.\n>\n> Should that be a colon? Stopping a half-sentence and connecting to\n> another half-sentence is usually done with a semicolon (like you did\n> in the new version of detachedHEAD above).\n>\n> \tShown when ... and domain name, to tell the user how to set\n> \ttheir identity configuration.\n>\n> perhaps? There may be other similar entries whose updated text uses\n> colon followed by an imperative sentence, but I didn't look very\n> carefully.\n\nI’ll spoil it for you: there are a lot of colons. ;)\n\nGood point. I’ll go over it again and probably use more semicolons\ninstead.\n\n>> \tstatusUoption::\n>> -\t\tAdvise to consider using the `-u` option to linkgit:git-status[1]\n>> -\t\twhen the command takes more than 2 seconds to enumerate untracked\n>> -\t\tfiles.\n>> +\t\tShown when linkgit:git-status[1] takes more than 2\n>> +\t\tseconds to enumerate untracked files: consider using the\n>> +\t\t`-u` option.\n>\n> Earlier ones after a colon (or semicolon in detachedHEAD case), you\n> gave an order to the advice message (e.g. \"hey detachedHead advice,\n> tell the user how to create a local branch\"), but this one is giving\n> an order to the end user, which feels inconsistent.\n>\n> I do not have a strong objection against giving an order to the\n> advice message, as long as it is done consistently. If we did so,\n> the part after the colon would start with \"instruct the user ...\" or\n> \"tell the user ...\" and the like, and the gist of what this one\n> would say would be \"shown when it is taking too long: suggest the\n> user to consider `-u`\".\n\nYeah, I paused for a minute when writing that. I’ll change to “tell” or\nsomething similar.\n\n> FWIW, my earlier \"in order to\" took an approach that is different\n> from either of the two \"giving an order\" approaches. I was trying\n> to make the description explain what the message tries to do and/or\n> why the message is given (e.g., \"shown if it takes too long in order\n> to suggest users to consider the -u option\").\n>\n> Thanks.\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"489964","messageId":"xmqqfrx4650c.fsf@gitster.g","threadId":"61033","inReplyTo":"166d2baa-933c-44f8-b6fb-94c8bce63a86@app.fastmail.com","subject":"Re: [PATCH v3 1/5] t3200: improve test style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-05T16:02:43Z","receivedAt":"2024-03-05T16:02:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n>>> -mv .git/config .git/config-saved\n>>> -\n>>> test_expect_success DEFAULT_REPO_FORMAT 'git branch -m q q2 without config should succeed' '\n>>> +\ttest_when_finished mv .git/config-saved .git/config &&\n>>> +\tmv .git/config .git/config-saved &&\n>>> \tgit branch -m q q2 &&\n>>> \tgit branch -m q2 q\n>>> '\n>>>\n>>> -mv .git/config-saved .git/config\n>>\n>> The above is a truly valuable clean-up.\n>>\n>> But I am not really sure if the paritcular condition is worth\n>> testing in the first place these days. No configuration file means\n>> we cannot even read the repository format version, and working under\n>> such a condition is quite a bad promise that we would rather not to\n>> having to keep. But that is an entirely different topic from what\n>> this patch is doing.\n>\n> Okay. I could undo this change and remove the test in its own commit?\n\nNo, please keep it.\n\nI think removing it is totally outside the scope of this series. We\ndo preliminary clean-up in various areas, so that the last step can\ndo the advise thing for \"git branch\". In the context of the series,\nremoving this test does not fit anywhere. It is not a clean-up like\nany other preliminary steps.\n\nThanks.\n"},{"id":"489983","messageId":"cover.1709670287.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709590037.git.code@khaugsbakk.name","subject":"[PATCH v4 0/5] advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T20:29:38Z","receivedAt":"2024-03-05T20:31:04Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Point the user towards the ref/branch name syntax rules if they give an\ninvalid name.\n\nAlso make some spatially-appropriate improvements:\n\n• Test style\n• `advice.txt`\n\n§ git-replace(1)\n\n(see cover letter for v2)\n\n§ Alternatives (to this change)\n\nWhile working on this I also thought that it might be nice to have a\nman page `gitrefsyntax`. That one could use a lot of the content from\n`man git check-ref-format` verbatim. Then the hint could point towards\nthat man page. And it seems that AsciiDoc supports _includes_ which\nmeans that the rules don’t have to be duplicated between the two man\npages.\n\n§ CC\n\nFor changes to `advice.txt`:\n\nCc: Elijah Newren \nCc: Jean-Noël Avila \n\n§ Changes in v4\n\nMostly about the style rewrite in `advice.txt`.\n\n• Patch 1:\n • Drop `(setup)` change\n • Drop superflouos bullet point\n • Don’t use period to end bullet point\n• Patch 2:\n • Drop trailer since this took on a life of its own\n • Drop uses of colons and semicolons in favor of a “to ”\n clause (mostly “to tell”)\n • Simplify some of the “effect clauses” by using “to tell” instead of\n verbs like “instruct”\n• Patch 3:\n • Also quote ref globs\n• Patch 5:\n • Update refSyntax entry for consistency with the rest of the entries\n\nKristoffer Haugsbakk (5):\n t3200: improve test style\n advice: make all entries stylistically consistent\n advice: use backticks for verbatim\n advice: use double quotes for regular quoting\n branch: advise about ref syntax rules\n\n Documentation/config/advice.txt | 95 ++++++++++++------------\n advice.c | 1 +\n advice.h | 1 +\n branch.c | 8 ++-\n builtin/branch.c | 8 ++-\n t/t3200-branch.sh | 123 +++++++++++++++++---------------\n 6 files changed, 128 insertions(+), 108 deletions(-)\n\nRange-diff against v3:\n1: e6a2628ce57 ! 1: ad101c72a60 t3200: improve test style\n @@ Commit message\n Also:\n \n • Remove a now-irrelevant comment about test placement and switch back\n - to `main` post-test.\n + to `main` post-test\n • Prefer indented literal heredocs (`-\\EOF`) except for a block which\n says that this is intentional\n - • Move a `git config` command into the test and mark it as `setup` since\n - the next test depends on it\n \n Helped-by: Junio C Hamano \n Signed-off-by: Kristoffer Haugsbakk \n \n +\n + ## Notes (series) ##\n + v4:\n + • Drop `(setup)` change\n + • Drop superflouos bullet point\n + • Don’t use period to end bullet point\n +\n ## t/t3200-branch.sh ##\n @@ t/t3200-branch.sh: test_expect_success 'git branch HEAD should fail' '\n \ttest_must_fail git branch HEAD\n @@ t/t3200-branch.sh: test_expect_success 'git branch -v with column.ui ignored' '\n -\n -git config branch.s/s.dummy Hello\n -\n --test_expect_success 'git branch -m s/s s should work when s/t is deleted' '\n -+test_expect_success '(setup) git branch -m s/s s should work when s/t is deleted' '\n + test_expect_success 'git branch -m s/s s should work when s/t is deleted' '\n +\tgit config branch.s/s.dummy Hello &&\n \tgit branch --create-reflog s/s &&\n \tgit reflog exists refs/heads/s/s &&\n2: d48b4719c27 ! 2: 7017ff3fff7 advice: make all entries stylistically consistent\n @@ Metadata\n ## Commit message ##\n advice: make all entries stylistically consistent\n \n + In general, rewrite entries to the following form:\n +\n + 1. Clause or sentence describing when the advice is shown\n + 2. Optional “to ” clause which says what the advice is\n + about (e.g. for resetNoRefresh: tell the user that they can use\n + `--no-refresh`)\n +\n + Concretely:\n +\n 1. Use “shown” instead of “advice shown”\n • “advice” is implied and a bit repetitive\n 2. Use “when” instead of “if”\n 3. Lead with “Shown when” and end the entry with the effect it has,\n where applicable\n 4. Use “the user” instead of “a user” or “you”\n - 5. detachedHead: connect clause with a semicolon to make the sentence\n - flow better in this new context\n - 6. implicitIdentity: rewrite description in order to lead with *when*\n + 5. implicitIdentity: rewrite description in order to lead with *when*\n the advice is shown (see point (3))\n - 7. Prefer the present tense (with the exception of pushNonFFMatching)\n - 8. Use a colon to connect the last clause instead of a comma\n - 9. waitingForEditor: give example of relevance in this new context\n - 10. pushUpdateRejected: exception to the above principles\n + 6. Prefer the present tense (with the exception of pushNonFFMatching)\n + 7. waitingForEditor: give example of relevance in this new context\n + 8. pushUpdateRejected: exception to the above principles\n \n - Suggested-by: Junio C Hamano \n Signed-off-by: Kristoffer Haugsbakk \n \n \n ## Notes (series) ##\n - Maybe the style that we eventually agree on should be documented outside the\n - commit log?\n + v4:\n + • Drop trailer since this took on a life of its own\n + • Drop uses of colons and semicolons in favor of a “to ”\n + clause (mostly “to tell”)\n + • Simplify some of the “effect clauses” by using “to tell” instead of\n + verbs like “instruct”\n + v3:\n + • Comment: Maybe the style that we eventually agree on should be\n + documented outside the commit log?\n \n ## Documentation/config/advice.txt ##\n @@ Documentation/config/advice.txt: advice.*::\n @@ Documentation/config/advice.txt: advice.*::\n -\t\tAdvice that shows the location of the patch file when\n -\t\tlinkgit:git-am[1] fails to apply it.\n +\t\tShown when linkgit:git-am[1] fails to apply a patch\n -+\t\tfile: tell the location of the file.\n ++\t\tfile, to tell the user the location of the file.\n \tambiguousFetchRefspec::\n -\t\tAdvice shown when a fetch refspec for multiple remotes maps to\n +\t\tShown when a fetch refspec for multiple remotes maps to\n @@ Documentation/config/advice.txt: advice.*::\n +\t\tShown when the user uses\n \t\tlinkgit:git-switch[1] or linkgit:git-checkout[1]\n -\t\tto move to the detached HEAD state, to instruct how to\n -+\t\tto move to the detached HEAD state; instruct how to\n - \t\tcreate a local branch after the fact.\n +-\t\tcreate a local branch after the fact.\n ++\t\tto move to the detached HEAD state, to tell the user how\n ++\t\tto create a local branch after the fact.\n \tdiverging::\n -\t\tAdvice shown when a fast-forward is not possible.\n +\t\tShown when a fast-forward is not possible.\n @@ Documentation/config/advice.txt: advice.*::\n -\t\tyour information is guessed from the system username and\n -\t\tdomain name.\n +\t\tShown when the user's information is guessed from the\n -+\t\tsystem username and domain name: tell the user how to\n ++\t\tsystem username and domain name, to tell the user how to\n +\t\tset their identity configuration.\n \tnestedTag::\n -\t\tAdvice shown if a user attempts to recursively tag a tag object.\n @@ Documentation/config/advice.txt: advice.*::\n -\t\tlinkgit:git-reset[1] when the command takes more than 2 seconds\n -\t\tto refresh the index after reset.\n +\t\tShown when linkgit:git-reset[1] takes more than 2\n -+\t\tseconds to refresh the index after reset: tell the user\n ++\t\tseconds to refresh the index after reset, to tell the user\n +\t\tthat they can use the `--no-refresh` option.\n \tresolveConflict::\n -\t\tAdvice shown by various commands when conflicts\n @@ Documentation/config/advice.txt: advice.*::\n \trmHints::\n -\t\tIn case of failure in the output of linkgit:git-rm[1],\n -\t\tshow directions on how to proceed from the current state.\n -+\t\tShown on failure in the output of linkgit:git-rm[1]:\n ++\t\tShown on failure in the output of linkgit:git-rm[1], to\n +\t\tgive directions on how to proceed from the current state.\n \tsequencerInUse::\n -\t\tAdvice shown when a sequencer command is already in progress.\n @@ Documentation/config/advice.txt: advice.*::\n -\t\twhen the command takes more than 2 seconds to enumerate untracked\n -\t\tfiles.\n +\t\tShown when linkgit:git-status[1] takes more than 2\n -+\t\tseconds to enumerate untracked files: consider using the\n -+\t\t`-u` option.\n ++\t\tseconds to enumerate untracked files, to tell the user that\n ++\t\tthey can use the `-u` option.\n \tsubmoduleAlternateErrorStrategyDie::\n -\t\tAdvice shown when a submodule.alternateErrorStrategy option\n +\t\tShown when a submodule.alternateErrorStrategy option\n @@ Documentation/config/advice.txt: advice.*::\n -\t\tAdvice shown when a user tries to create a worktree from an\n -\t\tinvalid reference, to instruct how to create a new unborn\n +\t\tShown when the user tries to create a worktree from an\n -+\t\tinvalid reference: instruct how to create a new unborn\n ++\t\tinvalid reference, to tell the user how to create a new unborn\n \t\tbranch instead.\n --\n3: 30d662a04c7 ! 3: df9b872afd1 advice: use backticks for code\n @@ Metadata\n Author: Kristoffer Haugsbakk \n \n ## Commit message ##\n - advice: use backticks for code\n + advice: use backticks for verbatim\n \n - Use backticks for quoting code rather than single quotes.\n + Use backticks for inline-verbatim rather than single quotes. Also quote\n + the unquoted ref globs.\n \n Also replace “the add command” with “`git add`”.\n \n Signed-off-by: Kristoffer Haugsbakk \n \n +\n + ## Notes (series) ##\n + v4:\n + • Also quote ref globs\n +\n ## Documentation/config/advice.txt ##\n @@ Documentation/config/advice.txt: advice.*::\n \tThese variables control various optional help messages designed to\n @@ Documentation/config/advice.txt: advice.*::\n \t\tit resulted in a non-fast-forward error.\n \tpushRefNeedsUpdate::\n @@ Documentation/config/advice.txt: advice.*::\n - \t\trefs/heads/* or refs/tags/* based on the type of the\n + \t\tguess based on the source and destination refs what\n + \t\tremote ref namespace the source belongs in, but where\n + \t\twe can still suggest that the user push to either\n +-\t\trefs/heads/* or refs/tags/* based on the type of the\n ++\t\t`refs/heads/*` or `refs/tags/*` based on the type of the\n \t\tsource object.\n \tpushUpdateRejected::\n -\t\tSet this variable to 'false' if you want to disable\n4: 3028713357f = 4: 15594b2a3a8 advice: use double quotes for regular quoting\n5: 402b7937951 ! 5: 97b53c04894 branch: advise about ref syntax rules\n @@ Commit message\n \n \n ## Notes (series) ##\n + v4:\n + • Update refSyntax entry for consistency with the rest of the entries\n v3:\n • Tweak advice doc for the new entry\n • Better test style\n @@ Documentation/config/advice.txt: advice.*::\n \t\t`pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`\n \t\tsimultaneously.\n +\trefSyntax::\n -+\t\tShown when the user provides an illegal ref name: point\n -+\t\ttowards the ref syntax documentation.\n ++\t\tShown when the user provides an illegal ref name, to\n ++\t\ttell the user about the ref syntax documentation.\n \tresetNoRefresh::\n \t\tShown when linkgit:git-reset[1] takes more than 2\n - \t\tseconds to refresh the index after reset: tell the user\n + \t\tseconds to refresh the index after reset, to tell the user\n \n ## advice.c ##\n @@ advice.c: static struct {\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489984","messageId":"ad101c72a60295c6e008bccf9f5f56c4ca6fab75.1709670287.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709670287.git.code@khaugsbakk.name","subject":"[PATCH v4 1/5] t3200: improve test style","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T20:29:39Z","receivedAt":"2024-03-05T20:31:05Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Some tests use a preliminary heredoc for `expect` or have setup and\nteardown commands before and after, respectively. It is however\npreferred to keep all the logic in the test itself. Let’s move these\ninto the tests.\n\nAlso:\n\n• Remove a now-irrelevant comment about test placement and switch back\n to `main` post-test\n• Prefer indented literal heredocs (`-\\EOF`) except for a block which\n says that this is intentional\n\nHelped-by: Junio C Hamano \nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n v4:\n • Drop `(setup)` change\n • Drop superflouos bullet point\n • Don’t use period to end bullet point\n\n t/t3200-branch.sh | 113 ++++++++++++++++++++++------------------------\n 1 file changed, 55 insertions(+), 58 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex de7d3014e4f..060b27097e8 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -75,13 +75,13 @@ test_expect_success 'git branch HEAD should fail' '\n \ttest_must_fail git branch HEAD\n '\n \n-cat >expect <expect <<-EOF &&\n+\t$HEAD refs/heads/d/e/f@{0}: branch: Created from main\n+\tEOF\n \tgit reflog show --no-abbrev-commit refs/heads/d/e/f >actual &&\n \ttest_cmp expect actual\n '\n@@ -440,10 +440,10 @@ test_expect_success 'git branch --list -v with --abbrev' '\n \n test_expect_success 'git branch --column' '\n \tCOLUMNS=81 git branch --column=column >actual &&\n-\tcat >expect <<\\EOF &&\n- a/b/c bam foo l * main n o/p r\n- abc bar j/k m/m mb o/o q topic\n-EOF\n+\tcat >expect <<-\\EOF &&\n+\t a/b/c bam foo l * main n o/p r\n+\t abc bar j/k m/m mb o/o q topic\n+\tEOF\n \ttest_cmp expect actual\n '\n \n@@ -453,25 +453,25 @@ test_expect_success 'git branch --column with an extremely long branch name' '\n \ttest_when_finished \"git branch -d $long\" &&\n \tgit branch $long &&\n \tCOLUMNS=80 git branch --column=column >actual &&\n-\tcat >expect <expect <<-EOF &&\n+\t a/b/c\n+\t abc\n+\t bam\n+\t bar\n+\t foo\n+\t j/k\n+\t l\n+\t m/m\n+\t* main\n+\t mb\n+\t n\n+\t o/o\n+\t o/p\n+\t q\n+\t r\n+\t topic\n+\t $long\n+\tEOF\n \ttest_cmp expect actual\n '\n \n@@ -481,10 +481,10 @@ test_expect_success 'git branch with column.*' '\n \tCOLUMNS=80 git branch >actual &&\n \tgit config --unset column.branch &&\n \tgit config --unset column.ui &&\n-\tcat >expect <<\\EOF &&\n- a/b/c bam foo l * main n o/p r\n- abc bar j/k m/m mb o/o q topic\n-EOF\n+\tcat >expect <<-\\EOF &&\n+\t a/b/c bam foo l * main n o/p r\n+\t abc bar j/k m/m mb o/o q topic\n+\tEOF\n \ttest_cmp expect actual\n '\n \n@@ -496,39 +496,36 @@ test_expect_success 'git branch -v with column.ui ignored' '\n \tgit config column.ui column &&\n \tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n \tgit config --unset column.ui &&\n-\tcat >expect <<\\EOF &&\n- a/b/c\n- abc\n- bam\n- bar\n- foo\n- j/k\n- l\n- m/m\n-* main\n- mb\n- n\n- o/o\n- o/p\n- q\n- r\n- topic\n-EOF\n+\tcat >expect <<-\\EOF &&\n+\t a/b/c\n+\t abc\n+\t bam\n+\t bar\n+\t foo\n+\t j/k\n+\t l\n+\t m/m\n+\t* main\n+\t mb\n+\t n\n+\t o/o\n+\t o/p\n+\t q\n+\t r\n+\t topic\n+\tEOF\n \ttest_cmp expect actual\n '\n \n-mv .git/config .git/config-saved\n-\n test_expect_success DEFAULT_REPO_FORMAT 'git branch -m q q2 without config should succeed' '\n+\ttest_when_finished mv .git/config-saved .git/config &&\n+\tmv .git/config .git/config-saved &&\n \tgit branch -m q q2 &&\n \tgit branch -m q2 q\n '\n \n-mv .git/config-saved .git/config\n-\n-git config branch.s/s.dummy Hello\n-\n test_expect_success 'git branch -m s/s s should work when s/t is deleted' '\n+\tgit config branch.s/s.dummy Hello &&\n \tgit branch --create-reflog s/s &&\n \tgit reflog exists refs/heads/s/s &&\n \tgit branch --create-reflog s/t &&\n@@ -1141,14 +1138,14 @@ test_expect_success '--set-upstream-to notices an error to set branch as own ups\n \ttest_cmp expect actual\n \"\n \n-# Keep this test last, as it changes the current branch\n-cat >expect <expect <<-EOF &&\n+\t$HEAD refs/heads/g/h/i@{0}: branch: Created from main\n+\tEOF\n \tgit reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&\n \ttest_cmp expect actual\n '\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489985","messageId":"7017ff3fff773412e8c472d8e59a132b0e8faae7.1709670287.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709670287.git.code@khaugsbakk.name","subject":"[PATCH v4 2/5] advice: make all entries stylistically consistent","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T20:29:40Z","receivedAt":"2024-03-05T20:31:07Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"In general, rewrite entries to the following form:\n\n1. Clause or sentence describing when the advice is shown\n2. Optional “to ” clause which says what the advice is\n about (e.g. for resetNoRefresh: tell the user that they can use\n `--no-refresh`)\n\nConcretely:\n\n1. Use “shown” instead of “advice shown”\n • “advice” is implied and a bit repetitive\n2. Use “when” instead of “if”\n3. Lead with “Shown when” and end the entry with the effect it has,\n where applicable\n4. Use “the user” instead of “a user” or “you”\n5. implicitIdentity: rewrite description in order to lead with *when*\n the advice is shown (see point (3))\n6. Prefer the present tense (with the exception of pushNonFFMatching)\n7. waitingForEditor: give example of relevance in this new context\n8. pushUpdateRejected: exception to the above principles\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n v4:\n • Drop trailer since this took on a life of its own\n • Drop uses of colons and semicolons in favor of a “to ”\n clause (mostly “to tell”)\n • Simplify some of the “effect clauses” by using “to tell” instead of\n verbs like “instruct”\n v3:\n • Comment: Maybe the style that we eventually agree on should be\n documented outside the commit log?\n\n Documentation/config/advice.txt | 82 ++++++++++++++++-----------------\n 1 file changed, 41 insertions(+), 41 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex c7ea70f2e2e..72cd9f9e9d9 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -6,23 +6,23 @@ advice.*::\n +\n --\n \taddEmbeddedRepo::\n-\t\tAdvice on what to do when you've accidentally added one\n+\t\tShown when the user accidentally adds one\n \t\tgit repo inside of another.\n \taddEmptyPathspec::\n-\t\tAdvice shown if a user runs the add command without providing\n+\t\tShown when the user runs the add command without providing\n \t\tthe pathspec parameter.\n \taddIgnoredFile::\n-\t\tAdvice shown if a user attempts to add an ignored file to\n+\t\tShown when the user attempts to add an ignored file to\n \t\tthe index.\n \tamWorkDir::\n-\t\tAdvice that shows the location of the patch file when\n-\t\tlinkgit:git-am[1] fails to apply it.\n+\t\tShown when linkgit:git-am[1] fails to apply a patch\n+\t\tfile, to tell the user the location of the file.\n \tambiguousFetchRefspec::\n-\t\tAdvice shown when a fetch refspec for multiple remotes maps to\n+\t\tShown when a fetch refspec for multiple remotes maps to\n \t\tthe same remote-tracking branch namespace and causes branch\n \t\ttracking set-up to fail.\n \tcheckoutAmbiguousRemoteBranchName::\n-\t\tAdvice shown when the argument to\n+\t\tShown when the argument to\n \t\tlinkgit:git-checkout[1] and linkgit:git-switch[1]\n \t\tambiguously resolves to a\n \t\tremote tracking branch on more than one remote in\n@@ -33,31 +33,31 @@ advice.*::\n \t\tto be used by default in some situations where this\n \t\tadvice would be printed.\n \tcommitBeforeMerge::\n-\t\tAdvice shown when linkgit:git-merge[1] refuses to\n+\t\tShown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwriting local changes.\n \tdetachedHead::\n-\t\tAdvice shown when you used\n+\t\tShown when the user uses\n \t\tlinkgit:git-switch[1] or linkgit:git-checkout[1]\n-\t\tto move to the detached HEAD state, to instruct how to\n-\t\tcreate a local branch after the fact.\n+\t\tto move to the detached HEAD state, to tell the user how\n+\t\tto create a local branch after the fact.\n \tdiverging::\n-\t\tAdvice shown when a fast-forward is not possible.\n+\t\tShown when a fast-forward is not possible.\n \tfetchShowForcedUpdates::\n-\t\tAdvice shown when linkgit:git-fetch[1] takes a long time\n+\t\tShown when linkgit:git-fetch[1] takes a long time\n \t\tto calculate forced updates after ref updates, or to warn\n \t\tthat the check is disabled.\n \tforceDeleteBranch::\n-\t\tAdvice shown when a user tries to delete a not fully merged\n+\t\tShown when the user tries to delete a not fully merged\n \t\tbranch without the force option set.\n \tignoredHook::\n-\t\tAdvice shown if a hook is ignored because the hook is not\n+\t\tShown when a hook is ignored because the hook is not\n \t\tset as executable.\n \timplicitIdentity::\n-\t\tAdvice on how to set your identity configuration when\n-\t\tyour information is guessed from the system username and\n-\t\tdomain name.\n+\t\tShown when the user's information is guessed from the\n+\t\tsystem username and domain name, to tell the user how to\n+\t\tset their identity configuration.\n \tnestedTag::\n-\t\tAdvice shown if a user attempts to recursively tag a tag object.\n+\t\tShown when a user attempts to recursively tag a tag object.\n \tpushAlreadyExists::\n \t\tShown when linkgit:git-push[1] rejects an update that\n \t\tdoes not qualify for fast-forwarding (e.g., a tag.)\n@@ -71,12 +71,12 @@ advice.*::\n \t\tobject that is not a commit-ish, or make the remote\n \t\tref point at an object that is not a commit-ish.\n \tpushNonFFCurrent::\n-\t\tAdvice shown when linkgit:git-push[1] fails due to a\n+\t\tShown when linkgit:git-push[1] fails due to a\n \t\tnon-fast-forward update to the current branch.\n \tpushNonFFMatching::\n-\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n-\t\t'matching refs' explicitly (i.e. you used ':', or\n-\t\tspecified a refspec that isn't your current branch) and\n+\t\tShown when the user ran linkgit:git-push[1] and pushed\n+\t\t'matching refs' explicitly (i.e. used ':', or\n+\t\tspecified a refspec that isn't the current branch) and\n \t\tit resulted in a non-fast-forward error.\n \tpushRefNeedsUpdate::\n \t\tShown when linkgit:git-push[1] rejects a forced update of\n@@ -95,17 +95,17 @@ advice.*::\n \t\t'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'\n \t\tsimultaneously.\n \tresetNoRefresh::\n-\t\tAdvice to consider using the `--no-refresh` option to\n-\t\tlinkgit:git-reset[1] when the command takes more than 2 seconds\n-\t\tto refresh the index after reset.\n+\t\tShown when linkgit:git-reset[1] takes more than 2\n+\t\tseconds to refresh the index after reset, to tell the user\n+\t\tthat they can use the `--no-refresh` option.\n \tresolveConflict::\n-\t\tAdvice shown by various commands when conflicts\n+\t\tShown by various commands when conflicts\n \t\tprevent the operation from being performed.\n \trmHints::\n-\t\tIn case of failure in the output of linkgit:git-rm[1],\n-\t\tshow directions on how to proceed from the current state.\n+\t\tShown on failure in the output of linkgit:git-rm[1], to\n+\t\tgive directions on how to proceed from the current state.\n \tsequencerInUse::\n-\t\tAdvice shown when a sequencer command is already in progress.\n+\t\tShown when a sequencer command is already in progress.\n \tskippedCherryPicks::\n \t\tShown when linkgit:git-rebase[1] skips a commit that has already\n \t\tbeen cherry-picked onto the upstream branch.\n@@ -123,27 +123,27 @@ advice.*::\n \t\tby linkgit:git-switch[1] or\n \t\tlinkgit:git-checkout[1] when switching branches.\n \tstatusUoption::\n-\t\tAdvise to consider using the `-u` option to linkgit:git-status[1]\n-\t\twhen the command takes more than 2 seconds to enumerate untracked\n-\t\tfiles.\n+\t\tShown when linkgit:git-status[1] takes more than 2\n+\t\tseconds to enumerate untracked files, to tell the user that\n+\t\tthey can use the `-u` option.\n \tsubmoduleAlternateErrorStrategyDie::\n-\t\tAdvice shown when a submodule.alternateErrorStrategy option\n+\t\tShown when a submodule.alternateErrorStrategy option\n \t\tconfigured to \"die\" causes a fatal error.\n \tsubmodulesNotUpdated::\n-\t\tAdvice shown when a user runs a submodule command that fails\n+\t\tShown when a user runs a submodule command that fails\n \t\tbecause `git submodule update --init` was not run.\n \tsuggestDetachingHead::\n-\t\tAdvice shown when linkgit:git-switch[1] refuses to detach HEAD\n+\t\tShown when linkgit:git-switch[1] refuses to detach HEAD\n \t\twithout the explicit `--detach` option.\n \tupdateSparsePath::\n-\t\tAdvice shown when either linkgit:git-add[1] or linkgit:git-rm[1]\n+\t\tShown when either linkgit:git-add[1] or linkgit:git-rm[1]\n \t\tis asked to update index entries outside the current sparse\n \t\tcheckout.\n \twaitingForEditor::\n-\t\tPrint a message to the terminal whenever Git is waiting for\n-\t\teditor input from the user.\n+\t\tShown when Git is waiting for editor input. Relevant\n+\t\twhen e.g. the editor is not launched inside the terminal.\n \tworktreeAddOrphan::\n-\t\tAdvice shown when a user tries to create a worktree from an\n-\t\tinvalid reference, to instruct how to create a new unborn\n+\t\tShown when the user tries to create a worktree from an\n+\t\tinvalid reference, to tell the user how to create a new unborn\n \t\tbranch instead.\n --\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489986","messageId":"df9b872afd16257acc935180dc27b105e17d6e16.1709670287.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709670287.git.code@khaugsbakk.name","subject":"[PATCH v4 3/5] advice: use backticks for verbatim","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T20:29:41Z","receivedAt":"2024-03-05T20:31:08Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Use backticks for inline-verbatim rather than single quotes. Also quote\nthe unquoted ref globs.\n\nAlso replace “the add command” with “`git add`”.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n v4:\n • Also quote ref globs\n\n Documentation/config/advice.txt | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex 72cd9f9e9d9..c8d6c625f2a 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -2,14 +2,14 @@ advice.*::\n \tThese variables control various optional help messages designed to\n \taid new users. When left unconfigured, Git will give the message\n \talongside instructions on how to squelch it. You can tell Git\n-\tthat you do not need the help message by setting these to 'false':\n+\tthat you do not need the help message by setting these to `false`:\n +\n --\n \taddEmbeddedRepo::\n \t\tShown when the user accidentally adds one\n \t\tgit repo inside of another.\n \taddEmptyPathspec::\n-\t\tShown when the user runs the add command without providing\n+\t\tShown when the user runs `git add` without providing\n \t\tthe pathspec parameter.\n \taddIgnoredFile::\n \t\tShown when the user attempts to add an ignored file to\n@@ -75,7 +75,7 @@ advice.*::\n \t\tnon-fast-forward update to the current branch.\n \tpushNonFFMatching::\n \t\tShown when the user ran linkgit:git-push[1] and pushed\n-\t\t'matching refs' explicitly (i.e. used ':', or\n+\t\t'matching refs' explicitly (i.e. used `:`, or\n \t\tspecified a refspec that isn't the current branch) and\n \t\tit resulted in a non-fast-forward error.\n \tpushRefNeedsUpdate::\n@@ -87,12 +87,12 @@ advice.*::\n \t\tguess based on the source and destination refs what\n \t\tremote ref namespace the source belongs in, but where\n \t\twe can still suggest that the user push to either\n-\t\trefs/heads/* or refs/tags/* based on the type of the\n+\t\t`refs/heads/*` or `refs/tags/*` based on the type of the\n \t\tsource object.\n \tpushUpdateRejected::\n-\t\tSet this variable to 'false' if you want to disable\n-\t\t'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists',\n-\t\t'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'\n+\t\tSet this variable to `false` if you want to disable\n+\t\t`pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`,\n+\t\t`pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`\n \t\tsimultaneously.\n \tresetNoRefresh::\n \t\tShown when linkgit:git-reset[1] takes more than 2\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489987","messageId":"15594b2a3a89203461c3791fdbe8816945f86740.1709670287.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709670287.git.code@khaugsbakk.name","subject":"[PATCH v4 4/5] advice: use double quotes for regular quoting","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T20:29:42Z","receivedAt":"2024-03-05T20:31:09Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Use double quotes like we use for “die” in this document.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n Documentation/config/advice.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex c8d6c625f2a..dd52041bc94 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -75,7 +75,7 @@ advice.*::\n \t\tnon-fast-forward update to the current branch.\n \tpushNonFFMatching::\n \t\tShown when the user ran linkgit:git-push[1] and pushed\n-\t\t'matching refs' explicitly (i.e. used `:`, or\n+\t\t\"matching refs\" explicitly (i.e. used `:`, or\n \t\tspecified a refspec that isn't the current branch) and\n \t\tit resulted in a non-fast-forward error.\n \tpushRefNeedsUpdate::\n-- \n2.44.0.64.g52b67adbeb2\n\n"},{"id":"489988","messageId":"97b53c04894578b23d0c650f69885f734699afc7.1709670287.git.code@khaugsbakk.name","threadId":"61033","inReplyTo":"cover.1709670287.git.code@khaugsbakk.name","subject":"[PATCH v4 5/5] branch: advise about ref syntax rules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-05T20:29:43Z","receivedAt":"2024-03-05T20:31:10Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"git-branch(1) will error out if you give it a bad ref name. But the user\nmight not understand why or what part of the name is illegal.\n\nThe user might know that there are some limitations based on the *loose\nref* format (filenames), but there are also further rules for\neasier integration with shell-based tools, pathname expansion, and\nplaying well with reference name expressions.\n\nThe man page for git-check-ref-format(1) contains these rules. Let’s\nadvise about it since that is not a command that you just happen\nupon. Also make this advise configurable since you might not want to be\nreminded every time you make a little typo.\n\nSigned-off-by: Kristoffer Haugsbakk \n---\n\nNotes (series):\n v4:\n • Update refSyntax entry for consistency with the rest of the entries\n v3:\n • Tweak advice doc for the new entry\n • Better test style\n v2:\n • Make the advise optional via configuration\n • Propagate error properly with `die_message(…)` instead of `exit(1)`\n • Flesh out commit message a bit\n\n Documentation/config/advice.txt | 3 +++\n advice.c | 1 +\n advice.h | 1 +\n branch.c | 8 ++++++--\n builtin/branch.c | 8 ++++++--\n t/t3200-branch.sh | 10 ++++++++++\n 6 files changed, 27 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex dd52041bc94..06c754899c5 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -94,6 +94,9 @@ advice.*::\n \t\t`pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`,\n \t\t`pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate`\n \t\tsimultaneously.\n+\trefSyntax::\n+\t\tShown when the user provides an illegal ref name, to\n+\t\ttell the user about the ref syntax documentation.\n \tresetNoRefresh::\n \t\tShown when linkgit:git-reset[1] takes more than 2\n \t\tseconds to refresh the index after reset, to tell the user\ndiff --git a/advice.c b/advice.c\nindex 6e9098ff089..550c2968908 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -68,6 +68,7 @@ static struct {\n \t[ADVICE_PUSH_UNQUALIFIED_REF_NAME]\t\t= { \"pushUnqualifiedRefName\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED]\t\t\t= { \"pushUpdateRejected\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]\t\t= { \"pushNonFastForward\" }, /* backwards compatibility */\n+\t[ADVICE_REF_SYNTAX]\t\t\t\t= { \"refSyntax\" },\n \t[ADVICE_RESET_NO_REFRESH_WARNING]\t\t= { \"resetNoRefresh\" },\n \t[ADVICE_RESOLVE_CONFLICT]\t\t\t= { \"resolveConflict\" },\n \t[ADVICE_RM_HINTS]\t\t\t\t= { \"rmHints\" },\ndiff --git a/advice.h b/advice.h\nindex 9d4f49ae38b..d15fe2351ab 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -36,6 +36,7 @@ enum advice_type {\n \tADVICE_PUSH_UNQUALIFIED_REF_NAME,\n \tADVICE_PUSH_UPDATE_REJECTED,\n \tADVICE_PUSH_UPDATE_REJECTED_ALIAS,\n+\tADVICE_REF_SYNTAX,\n \tADVICE_RESET_NO_REFRESH_WARNING,\n \tADVICE_RESOLVE_CONFLICT,\n \tADVICE_RM_HINTS,\ndiff --git a/branch.c b/branch.c\nindex 6719a181bd1..621019fcf4b 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -370,8 +370,12 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n */\n int validate_branchname(const char *name, struct strbuf *ref)\n {\n-\tif (strbuf_check_branch_ref(ref, name))\n-\t\tdie(_(\"'%s' is not a valid branch name\"), name);\n+\tif (strbuf_check_branch_ref(ref, name)) {\n+\t\tint code = die_message(_(\"'%s' is not a valid branch name\"), name);\n+\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n+\t\t\t\t _(\"See `man git check-ref-format`\"));\n+\t\texit(code);\n+\t}\n \n \treturn ref_exists(ref->buf);\n }\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex cfb63cce5fb..1c122ee8a7b 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -576,8 +576,12 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t\t */\n \t\tif (ref_exists(oldref.buf))\n \t\t\trecovery = 1;\n-\t\telse\n-\t\t\tdie(_(\"invalid branch name: '%s'\"), oldname);\n+\t\telse {\n+\t\t\tint code = die_message(_(\"invalid branch name: '%s'\"), oldname);\n+\t\t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n+\t\t\t\t\t _(\"See `man git check-ref-format`\"));\n+\t\t\texit(code);\n+\t\t}\n \t}\n \n \tfor (int i = 0; worktrees[i]; i++) {\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 060b27097e8..dd7525d1b8c 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1722,4 +1722,14 @@ test_expect_success '--track overrides branch.autoSetupMerge' '\n \ttest_cmp_config \"\" --default \"\" branch.foo5.merge\n '\n \n+test_expect_success 'errors if given a bad branch name' '\n+\tcat <<-\\EOF >expect &&\n+\tfatal: '\\''foo..bar'\\'' is not a valid branch name\n+\thint: See `man git check-ref-format`\n+\thint: Disable this message with \"git config advice.refSyntax false\"\n+\tEOF\n+\ttest_must_fail git branch foo..bar >actual 2>&1 &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.44.0.64.g52b67adbeb2\n\n"}]}