{"thread":{"id":"60343","subject":"[Outreachy][PATCH 0/1] builtin/branch.c: ammend die() error message","startedAt":"2023-10-11T15:24:36Z","lastAt":"2023-10-12T05:32:47Z","messageCount":8,"participants":["Isoken June Ibizugbe","Dragan Simic","Junio C Hamano","Rubén Justo","Isoken Ibizugbe"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"483051","messageId":"20231011152424.6957-1-isokenjune@gmail.com","threadId":"60343","inReplyTo":null,"subject":"[Outreachy][PATCH 0/1] builtin/branch.c: ammend die() error message","fromName":"Isoken June Ibizugbe","fromEmail":"isokenjune@gmail.com","sentAt":"2023-10-11T15:24:19Z","receivedAt":"2023-10-11T15:24:36Z","isPatch":true,"sender":{"key":"isokenjune@gmail.com","avatar":"https://avatars.githubusercontent.com/u/127752393?v=4"},"body":"This patch improves the consistency and clarity of error messages across Git commands. It adheres to Git's Coding Guidelines for error messages:\n\n- Error messages no longer end with a full stop.\n- Capitalization is avoided in error messages.\n- Error messages lead with a description of the issue, enhancing readability.\n\nSigned-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>\n\nIsoken June Ibizugbe (1):\n  branch.c: ammend error messages for die()\n\n builtin/branch.c | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\n-- \n2.42.0.325.g3a06386e31\n\n"},{"id":"483052","messageId":"20231011152424.6957-2-isokenjune@gmail.com","threadId":"60343","inReplyTo":"20231011152424.6957-1-isokenjune@gmail.com","subject":"[PATCH 1/1] branch.c: ammend error messages for die()","fromName":"Isoken June Ibizugbe","fromEmail":"isokenjune@gmail.com","sentAt":"2023-10-11T15:24:20Z","receivedAt":"2023-10-11T15:24:39Z","isPatch":true,"sender":{"key":"isokenjune@gmail.com","avatar":"https://avatars.githubusercontent.com/u/127752393?v=4"},"body":"Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>\n---\n builtin/branch.c | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 2ec190b14a..a756543d64 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,\n \t\t\tcontinue;\n \n \t\tif (is_worktree_being_rebased(wt, target))\n-\t\t\tdie(_(\"Branch %s is being rebased at %s\"),\n+\t\t\tdie(_(\"branch %s is being rebased at %s\"),\n \t\t\t    target, wt->path);\n \n \t\tif (is_worktree_being_bisected(wt, target))\n-\t\t\tdie(_(\"Branch %s is being bisected at %s\"),\n+\t\t\tdie(_(\"branch %s is being bisected at %s\"),\n \t\t\t    target, wt->path);\n \t}\n }\n@@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\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\t\tdie(_(\"invalid branch name: '%s'\"), oldname);\n \t}\n \n \tfor (int i = 0; worktrees[i]; i++) {\n@@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \n \tif ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {\n \t\tif (oldref_usage & IS_HEAD)\n-\t\t\tdie(_(\"No commit on branch '%s' yet.\"), oldname);\n+\t\t\tdie(_(\"no commit on branch '%s' yet\"), oldname);\n \t\telse\n-\t\t\tdie(_(\"No branch named '%s'.\"), oldname);\n+\t\t\tdie(_(\"no branch named '%s'\"), oldname);\n \t}\n \n \t/*\n@@ -624,9 +624,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \n \tif (!copy && !(oldref_usage & IS_ORPHAN) &&\n \t    rename_ref(oldref.buf, newref.buf, logmsg.buf))\n-\t\tdie(_(\"Branch rename failed\"));\n+\t\tdie(_(\"branch rename failed\"));\n \tif (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))\n-\t\tdie(_(\"Branch copy failed\"));\n+\t\tdie(_(\"branch copy failed\"));\n \n \tif (recovery) {\n \t\tif (copy)\n@@ -640,16 +640,16 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \tif (!copy && (oldref_usage & IS_HEAD) &&\n \t    replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,\n \t\t\t\t\t      logmsg.buf))\n-\t\tdie(_(\"Branch renamed to %s, but HEAD is not updated!\"), newname);\n+\t\tdie(_(\"branch renamed to %s, but HEAD is not updated!\"), newname);\n \n \tstrbuf_release(&logmsg);\n \n \tstrbuf_addf(&oldsection, \"branch.%s\", interpreted_oldname);\n \tstrbuf_addf(&newsection, \"branch.%s\", interpreted_newname);\n \tif (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)\n-\t\tdie(_(\"Branch is renamed, but update of config-file failed\"));\n+\t\tdie(_(\"branch is renamed, but update of config-file failed\"));\n \tif (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)\n-\t\tdie(_(\"Branch is copied, but update of config-file failed\"));\n+\t\tdie(_(\"branch is copied, but update of config-file failed\"));\n \tstrbuf_release(&oldref);\n \tstrbuf_release(&newref);\n \tstrbuf_release(&oldsection);\n@@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \n \thead = resolve_refdup(\"HEAD\", 0, &head_oid, NULL);\n \tif (!head)\n-\t\tdie(_(\"Failed to resolve HEAD as a valid ref.\"));\n+\t\tdie(_(\"failed to resolve HEAD as a valid ref\"));\n \tif (!strcmp(head, \"HEAD\"))\n \t\tfilter.detached = 1;\n \telse if (!skip_prefix(head, \"refs/heads/\", &head))\n@@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \n \t\tif (!argc) {\n \t\t\tif (filter.detached)\n-\t\t\t\tdie(_(\"Cannot give description to detached HEAD\"));\n+\t\t\t\tdie(_(\"cannot give description to detached HEAD\"));\n \t\t\tbranch_name = head;\n \t\t} else if (argc == 1) {\n \t\t\tstrbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n@@ -892,7 +892,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif (!argc)\n \t\t\tdie(_(\"branch name required\"));\n \t\telse if ((argc == 1) && filter.detached)\n-\t\t\tdie(copy? _(\"cannot copy the current branch while not on any.\")\n+\t\t\tdie(copy? _(\"cannot copy the current branch while not on any\")\n \t\t\t\t: _(\"cannot rename the current branch while not on any.\"));\n \t\telse if (argc == 1)\n \t\t\tcopy_or_rename_branch(head, argv[0], copy, copy + rename > 1);\n@@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif (!branch) {\n \t\t\tif (!argc || !strcmp(argv[0], \"HEAD\"))\n \t\t\t\tdie(_(\"could not set upstream of HEAD to %s when \"\n-\t\t\t\t      \"it does not point to any branch.\"),\n+\t\t\t\t      \"it does not point to any branch\"),\n \t\t\t\t    new_upstream);\n \t\t\tdie(_(\"no such branch '%s'\"), argv[0]);\n \t\t}\n \n \t\tif (!ref_exists(branch->refname)) {\n \t\t\tif (!argc || branch_checked_out(branch->refname))\n-\t\t\t\tdie(_(\"No commit on branch '%s' yet.\"), branch->name);\n+\t\t\t\tdie(_(\"no commit on branch '%s' yet\"), branch->name);\n \t\t\tdie(_(\"branch '%s' does not exist\"), branch->name);\n \t\t}\n \n@@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif (!branch) {\n \t\t\tif (!argc || !strcmp(argv[0], \"HEAD\"))\n \t\t\t\tdie(_(\"could not unset upstream of HEAD when \"\n-\t\t\t\t      \"it does not point to any branch.\"));\n+\t\t\t\t      \"it does not point to any branch\"));\n \t\t\tdie(_(\"no such branch '%s'\"), argv[0]);\n \t\t}\n \n \t\tif (!branch_has_merge_config(branch))\n-\t\t\tdie(_(\"Branch '%s' has no upstream information\"), branch->name);\n+\t\t\tdie(_(\"branch '%s' has no upstream information\"), branch->name);\n \n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"branch.%s.remote\", branch->name);\n@@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tconst char *start_name = argc == 2 ? argv[1] : head;\n \n \t\tif (filter.kind != FILTER_REFS_BRANCHES)\n-\t\t\tdie(_(\"The -a, and -r, options to 'git branch' do not take a branch name.\\n\"\n+\t\t\tdie(_(\"the -a, and -r, options to 'git branch' do not take a branch name\\n\"\n \t\t\t\t  \"Did you mean to use: -a|-r --list <pattern>?\"));\n \n \t\tif (track == BRANCH_TRACK_OVERRIDE)\n-\t\t\tdie(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.\"));\n+\t\t\tdie(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead\"));\n \n \t\tif (recurse_submodules) {\n \t\t\tcreate_branches_recursively(the_repository, branch_name,\n-- \n2.42.0.325.g3a06386e31\n\n"},{"id":"483053","messageId":"c98dd0b5988f9889ee4c2b5d4e645839@manjaro.org","threadId":"60343","inReplyTo":"20231011152424.6957-1-isokenjune@gmail.com","subject":"Re: [Outreachy][PATCH 0/1] builtin/branch.c: ammend die() error message","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2023-10-11T15:34:17Z","receivedAt":"2023-10-11T15:34:22Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2023-10-11 17:24, Isoken June Ibizugbe wrote:\n> This patch improves the consistency and clarity of error messages\n> across Git commands. It adheres to Git's Coding Guidelines for error\n> messages:\n> \n> - Error messages no longer end with a full stop.\n> - Capitalization is avoided in error messages.\n> - Error messages lead with a description of the issue, enhancing \n> readability.\n> \n> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>\n\nWhen there's only one patch and not a series, there's no need for a \ncover letter, and the description of the patch actually needs to go into \nthe patch itself, so it can be latter pulled into the repository as part \nof the patch submission.\n\n> Isoken June Ibizugbe (1):\n>   branch.c: ammend error messages for die()\n> \n>  builtin/branch.c | 38 +++++++++++++++++++-------------------\n>  1 file changed, 19 insertions(+), 19 deletions(-)\n"},{"id":"483054","messageId":"abf571d080e0b9785451144cb6663c4a@manjaro.org","threadId":"60343","inReplyTo":"CAJHH8bGOSXNNYfOCkS=ck8r-=Mbq62Rs1c4NsgFWYD999kNfRw@mail.gmail.com","subject":"Re: [Outreachy][PATCH 0/1] builtin/branch.c: ammend die() error message","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2023-10-11T15:46:56Z","receivedAt":"2023-10-11T15:47:03Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2023-10-11 17:42, Isoken Ibizugbe wrote:\n> Thanks for the feedback. How do I add the description of the patch.\n\nCould you, please, use inline replying?  We've asked that at least four \ntimes already.\n\nRegarding the description of the patch, just write it as the commit \ncomment in your local git repository.  Using git-format-patch will pick \nthe commit comment up and put it into the email message file you'll then \nsend to the mailing list using git-send-email.  It's all rather simple \nand straightforward.\n\n\n> On Wed, 11 Oct 2023 at 4:34 PM, Dragan Simic <dsimic@manjaro.org>\n> wrote:\n> \n>> On 2023-10-11 17:24, Isoken June Ibizugbe wrote:\n>>> This patch improves the consistency and clarity of error messages\n>>> across Git commands. It adheres to Git's Coding Guidelines for\n>> error\n>>> messages:\n>>> \n>>> - Error messages no longer end with a full stop.\n>>> - Capitalization is avoided in error messages.\n>>> - Error messages lead with a description of the issue, enhancing\n>>> readability.\n>>> \n>>> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>\n>> \n>> When there's only one patch and not a series, there's no need for a\n>> cover letter, and the description of the patch actually needs to go\n>> into\n>> the patch itself, so it can be latter pulled into the repository as\n>> part\n>> of the patch submission.\n>> \n>>> Isoken June Ibizugbe (1):\n>>> branch.c: ammend error messages for die()\n>>> \n>>> builtin/branch.c | 38 +++++++++++++++++++-------------------\n>>> 1 file changed, 19 insertions(+), 19 deletions(-)\n"},{"id":"483062","messageId":"xmqqa5spm5ja.fsf@gitster.g","threadId":"60343","inReplyTo":"20231011152424.6957-2-isokenjune@gmail.com","subject":"Re: [PATCH 1/1] branch.c: ammend error messages for die()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-11T17:29:29Z","receivedAt":"2023-10-11T17:29:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Isoken June Ibizugbe <isokenjune@gmail.com> writes:\n\n> Subject: Re: [PATCH 1/1] branch.c: ammend error messages for die()\n\n\"ammend\" is misspelt, but more importantly, it has less information\ncontents than other possible phrases, e.g.,\n\n  Subject: [PATCH 1/1] branch.c: adjust die() messages to coding guidelines\n\nIn any case, the title of a commit has insufficient space to\ndescribe what the amendment is about, or which exact guideline\nthese messages violate and needs adjustment.  This space before your\nsign-off is where you write it.\n\nOther outreachy candidates have already been given pretty much the\nsame pieces of advice.  It may help candidates to learn from the\nresponses given to other candidates.  For example, I said the same\nthing in https://lore.kernel.org/git/xmqqlecbzl5e.fsf@gitster.g/\n\n> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>\n> ---\n>  builtin/branch.c | 38 +++++++++++++++++++-------------------\n>  1 file changed, 19 insertions(+), 19 deletions(-)\n\nNot a fault of this patch at all, but it is somewhat surprising that\nwe do not break any existing test with this many messages changed.\nDid you run the test suite before making this commit?\n\nMake it a habit to always do \"make test\" before committing your\nwork.  I am not saying \"do not commit what does not pass the tests\".\nWhat I mean is \"be aware of what is still broken (when fixing a bug)\nor what you broke (when adding a new feature, perhaps as an\nunintended side effect), before you commit, so that you can describe\nthem in your commit log message\".\n\nThanks.\n"},{"id":"483064","messageId":"06bc7b39-ed70-460f-b6f1-47a0c94c0538@gmail.com","threadId":"60343","inReplyTo":"20231011152424.6957-2-isokenjune@gmail.com","subject":"Re: [PATCH 1/1] branch.c: ammend error messages for die()","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2023-10-11T18:04:37Z","receivedAt":"2023-10-11T18:05:39Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On 11-oct-2023 16:24:20, Isoken June Ibizugbe wrote:\n\nHi Isoken,\n\n> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>\n> ---\n>  builtin/branch.c | 38 +++++++++++++++++++-------------------\n>  1 file changed, 19 insertions(+), 19 deletions(-)\n> \n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index 2ec190b14a..a756543d64 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,\n>  \t\t\tcontinue;\n>  \n>  \t\tif (is_worktree_being_rebased(wt, target))\n> -\t\t\tdie(_(\"Branch %s is being rebased at %s\"),\n> +\t\t\tdie(_(\"branch %s is being rebased at %s\"),\n\nOK.\n\n>  \t\t\t    target, wt->path);\n>  \n>  \t\tif (is_worktree_being_bisected(wt, target))\n> -\t\t\tdie(_(\"Branch %s is being bisected at %s\"),\n> +\t\t\tdie(_(\"branch %s is being bisected at %s\"),\n\nOK.\n\n>  \t\t\t    target, wt->path);\n>  \t}\n>  }\n> @@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\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\t\tdie(_(\"invalid branch name: '%s'\"), oldname);\n\nOK.\n\n>  \t}\n>  \n>  \tfor (int i = 0; worktrees[i]; i++) {\n> @@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n>  \n>  \tif ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {\n>  \t\tif (oldref_usage & IS_HEAD)\n> -\t\t\tdie(_(\"No commit on branch '%s' yet.\"), oldname);\n> +\t\t\tdie(_(\"no commit on branch '%s' yet\"), oldname);\n>  \t\telse\n> -\t\t\tdie(_(\"No branch named '%s'.\"), oldname);\n> +\t\t\tdie(_(\"no branch named '%s'\"), oldname);\n\nOK. Both.\n\n>  \t}\n>  \n>  \t/*\n> @@ -624,9 +624,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n>  \n>  \tif (!copy && !(oldref_usage & IS_ORPHAN) &&\n>  \t    rename_ref(oldref.buf, newref.buf, logmsg.buf))\n> -\t\tdie(_(\"Branch rename failed\"));\n> +\t\tdie(_(\"branch rename failed\"));\n>  \tif (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))\n> -\t\tdie(_(\"Branch copy failed\"));\n> +\t\tdie(_(\"branch copy failed\"));\n\nDitto\n\n>  \n>  \tif (recovery) {\n>  \t\tif (copy)\n> @@ -640,16 +640,16 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n>  \tif (!copy && (oldref_usage & IS_HEAD) &&\n>  \t    replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,\n>  \t\t\t\t\t      logmsg.buf))\n> -\t\tdie(_(\"Branch renamed to %s, but HEAD is not updated!\"), newname);\n> +\t\tdie(_(\"branch renamed to %s, but HEAD is not updated!\"), newname);\n\nIMO we can go further here and also remove that final \"!\".  But it's OK\nthe way you have done it.\n\n>  \n>  \tstrbuf_release(&logmsg);\n>  \n>  \tstrbuf_addf(&oldsection, \"branch.%s\", interpreted_oldname);\n>  \tstrbuf_addf(&newsection, \"branch.%s\", interpreted_newname);\n>  \tif (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)\n> -\t\tdie(_(\"Branch is renamed, but update of config-file failed\"));\n> +\t\tdie(_(\"branch is renamed, but update of config-file failed\"));\n>  \tif (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)\n> -\t\tdie(_(\"Branch is copied, but update of config-file failed\"));\n> +\t\tdie(_(\"branch is copied, but update of config-file failed\"));\n\nOK, both.\n\n>  \tstrbuf_release(&oldref);\n>  \tstrbuf_release(&newref);\n>  \tstrbuf_release(&oldsection);\n> @@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>  \n>  \thead = resolve_refdup(\"HEAD\", 0, &head_oid, NULL);\n>  \tif (!head)\n> -\t\tdie(_(\"Failed to resolve HEAD as a valid ref.\"));\n> +\t\tdie(_(\"failed to resolve HEAD as a valid ref\"));\n\nOK.\n\n>  \tif (!strcmp(head, \"HEAD\"))\n>  \t\tfilter.detached = 1;\n>  \telse if (!skip_prefix(head, \"refs/heads/\", &head))\n> @@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>  \n>  \t\tif (!argc) {\n>  \t\t\tif (filter.detached)\n> -\t\t\t\tdie(_(\"Cannot give description to detached HEAD\"));\n> +\t\t\t\tdie(_(\"cannot give description to detached HEAD\"));\n\nOK.\n\n>  \t\t\tbranch_name = head;\n>  \t\t} else if (argc == 1) {\n>  \t\t\tstrbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n> @@ -892,7 +892,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>  \t\tif (!argc)\n>  \t\t\tdie(_(\"branch name required\"));\n>  \t\telse if ((argc == 1) && filter.detached)\n> -\t\t\tdie(copy? _(\"cannot copy the current branch while not on any.\")\n> +\t\t\tdie(copy? _(\"cannot copy the current branch while not on any\")\n>  \t\t\t\t: _(\"cannot rename the current branch while not on any.\"));\n\nOK.  But I think you want to modify also the second message, to remove\nits full stop as well.\n\n>  \t\telse if (argc == 1)\n>  \t\t\tcopy_or_rename_branch(head, argv[0], copy, copy + rename > 1);\n> @@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>  \t\tif (!branch) {\n>  \t\t\tif (!argc || !strcmp(argv[0], \"HEAD\"))\n>  \t\t\t\tdie(_(\"could not set upstream of HEAD to %s when \"\n> -\t\t\t\t      \"it does not point to any branch.\"),\n> +\t\t\t\t      \"it does not point to any branch\"),\n\nOK.\n\n>  \t\t\t\t    new_upstream);\n>  \t\t\tdie(_(\"no such branch '%s'\"), argv[0]);\n>  \t\t}\n>  \n>  \t\tif (!ref_exists(branch->refname)) {\n>  \t\t\tif (!argc || branch_checked_out(branch->refname))\n> -\t\t\t\tdie(_(\"No commit on branch '%s' yet.\"), branch->name);\n> +\t\t\t\tdie(_(\"no commit on branch '%s' yet\"), branch->name);\n\nOK.\n\n>  \t\t\tdie(_(\"branch '%s' does not exist\"), branch->name);\n>  \t\t}\n>  \n> @@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>  \t\tif (!branch) {\n>  \t\t\tif (!argc || !strcmp(argv[0], \"HEAD\"))\n>  \t\t\t\tdie(_(\"could not unset upstream of HEAD when \"\n> -\t\t\t\t      \"it does not point to any branch.\"));\n> +\t\t\t\t      \"it does not point to any branch\"));\n>  \t\t\tdie(_(\"no such branch '%s'\"), argv[0]);\n>  \t\t}\n>  \n>  \t\tif (!branch_has_merge_config(branch))\n> -\t\t\tdie(_(\"Branch '%s' has no upstream information\"), branch->name);\n> +\t\t\tdie(_(\"branch '%s' has no upstream information\"), branch->name);\n\nOK, both.\n\n>  \n>  \t\tstrbuf_reset(&buf);\n>  \t\tstrbuf_addf(&buf, \"branch.%s.remote\", branch->name);\n> @@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>  \t\tconst char *start_name = argc == 2 ? argv[1] : head;\n>  \n>  \t\tif (filter.kind != FILTER_REFS_BRANCHES)\n> -\t\t\tdie(_(\"The -a, and -r, options to 'git branch' do not take a branch name.\\n\"\n> +\t\t\tdie(_(\"the -a, and -r, options to 'git branch' do not take a branch name\\n\"\n>  \t\t\t\t  \"Did you mean to use: -a|-r --list <pattern>?\"));\n\nGood; the full stop removed and here that question mark is valuable.  And ...\n\n>  \n>  \t\tif (track == BRANCH_TRACK_OVERRIDE)\n> -\t\t\tdie(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.\"));\n> +\t\t\tdie(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead\"));\n\nalso good.  But since we're here, maybe we can break this long message, remove\nthe first full stop and leave the second part of the message as is, as a\nsuggestion.  Like we do in the previous message, above.\n\nFor your consideration, I mean something like:\n\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -969,7 +969,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n                                  \"Did you mean to use: -a|-r --list <pattern>?\"));\n\n                if (track == BRANCH_TRACK_OVERRIDE)\n-                       die(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.\"));\n+                       die(_(\"the '--set-upstream' option is no longer supported\\n\"\n+                             \"Please use '--track' or '--set-upstream-to' instead.\"));\n\n\n>  \n>  \t\tif (recurse_submodules) {\n>  \t\t\tcreate_branches_recursively(the_repository, branch_name,\n> -- \n> 2.42.0.325.g3a06386e31\n> \n\nOne final comment, leaving aside my suggestions; this changes break\nsome tests that you need to adjust.  Try:\n\n   $ make && (cd t; ./t3200-branch.sh -vi)\n\nOr, as Junio has already suggested in another message:\n\n   $ make test\n\nI have some unfinished work that you might find useful:\n\nhttps://lore.kernel.org/git/eb3c689e-efeb-4468-a10f-dd32bc0ee37b@gmail.com/\n\nThank you for working on this.\n"},{"id":"483074","messageId":"xmqqttqxkmaq.fsf@gitster.g","threadId":"60343","inReplyTo":"06bc7b39-ed70-460f-b6f1-47a0c94c0538@gmail.com","subject":"Re: [PATCH 1/1] branch.c: ammend error messages for die()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-11T19:10:21Z","receivedAt":"2023-10-11T19:10:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rubén Justo <rjusto@gmail.com> writes:\n\n>> @@ -640,16 +640,16 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n>>  \tif (!copy && (oldref_usage & IS_HEAD) &&\n>>  \t    replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,\n>>  \t\t\t\t\t      logmsg.buf))\n>> -\t\tdie(_(\"Branch renamed to %s, but HEAD is not updated!\"), newname);\n>> +\t\tdie(_(\"branch renamed to %s, but HEAD is not updated!\"), newname);\n>\n> IMO we can go further here and also remove that final \"!\".  But it's OK\n> the way you have done it.\n\nThanks for good eyes.  I do not think '!' is adding any value in\nthis case, so removing it is probably a good idea, even if it is\ndone outside the scope of this patch.\n\n>>  \t\telse if ((argc == 1) && filter.detached)\n>> -\t\t\tdie(copy? _(\"cannot copy the current branch while not on any.\")\n>> +\t\t\tdie(copy? _(\"cannot copy the current branch while not on any\")\n>>  \t\t\t\t: _(\"cannot rename the current branch while not on any.\"));\n>\n> OK.  But I think you want to modify also the second message, to remove\n> its full stop as well.\n\nNice spotting.\n\n>> @@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>>  \t\tconst char *start_name = argc == 2 ? argv[1] : head;\n>>  \n>>  \t\tif (filter.kind != FILTER_REFS_BRANCHES)\n>> -\t\t\tdie(_(\"The -a, and -r, options to 'git branch' do not take a branch name.\\n\"\n>> +\t\t\tdie(_(\"the -a, and -r, options to 'git branch' do not take a branch name\\n\"\n>>  \t\t\t\t  \"Did you mean to use: -a|-r --list <pattern>?\"));\n>\n> Good; the full stop removed and here that question mark is valuable.  And ...\n\nThis one is not a single sentence, so retaining the full stop at the\nend of the first sentence would actually be good, in addition to the\ncapitalization of the second sentence.\n\nIn a modern world, I would suspect that we would use die_message()\ninterface to emit the first sentence, show the \"Did you mean...\"\nwith advice_if_enabled(), and then exit with the status returned by\ndie_message(), similar to how branch.c::setup_tracking() does.  When\nit happens, the die() message will be only the first sentence, and\nwhat this patch did can be retained.  The second message will have\nto be reworked to make it more appropriate as an advice message.\n\nThanks for a prompt review.\n"},{"id":"483132","messageId":"CAJHH8bHNsN4BVqG8n0SDn6WkpWdgt7C0=Bbtf91usKZ1G+nCpg@mail.gmail.com","threadId":"60343","inReplyTo":"06bc7b39-ed70-460f-b6f1-47a0c94c0538@gmail.com","subject":"Re: [PATCH 1/1] branch.c: ammend error messages for die()","fromName":"Isoken Ibizugbe","fromEmail":"isokenjune@gmail.com","sentAt":"2023-10-12T05:31:14Z","receivedAt":"2023-10-12T05:32:47Z","isPatch":true,"sender":{"key":"isokenjune@gmail.com","avatar":"https://avatars.githubusercontent.com/u/127752393?v=4"},"body":"On Wed, Oct 11, 2023 at 7:05 PM Rubén Justo <rjusto@gmail.com> wrote:\n>\n> On 11-oct-2023 16:24:20, Isoken June Ibizugbe wrote:\n>\n> Hi Isoken,\n>\n> > Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>\n> > ---\n> >  builtin/branch.c | 38 +++++++++++++++++++-------------------\n> >  1 file changed, 19 insertions(+), 19 deletions(-)\n> >\n> > diff --git a/builtin/branch.c b/builtin/branch.c\n> > index 2ec190b14a..a756543d64 100644\n> > --- a/builtin/branch.c\n> > +++ b/builtin/branch.c\n> > @@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,\n> >                       continue;\n> >\n> >               if (is_worktree_being_rebased(wt, target))\n> > -                     die(_(\"Branch %s is being rebased at %s\"),\n> > +                     die(_(\"branch %s is being rebased at %s\"),\n>\n> OK.\n>\n> >                           target, wt->path);\n> >\n> >               if (is_worktree_being_bisected(wt, target))\n> > -                     die(_(\"Branch %s is being bisected at %s\"),\n> > +                     die(_(\"branch %s is being bisected at %s\"),\n>\n> OK.\n>\n> >                           target, wt->path);\n> >       }\n> >  }\n> > @@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n> >               if (ref_exists(oldref.buf))\n> >                       recovery = 1;\n> >               else\n> > -                     die(_(\"Invalid branch name: '%s'\"), oldname);\n> > +                     die(_(\"invalid branch name: '%s'\"), oldname);\n>\n> OK.\n>\n> >       }\n> >\n> >       for (int i = 0; worktrees[i]; i++) {\n> > @@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n> >\n> >       if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {\n> >               if (oldref_usage & IS_HEAD)\n> > -                     die(_(\"No commit on branch '%s' yet.\"), oldname);\n> > +                     die(_(\"no commit on branch '%s' yet\"), oldname);\n> >               else\n> > -                     die(_(\"No branch named '%s'.\"), oldname);\n> > +                     die(_(\"no branch named '%s'\"), oldname);\n>\n> OK. Both.\n>\n> >       }\n> >\n> >       /*\n> > @@ -624,9 +624,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n> >\n> >       if (!copy && !(oldref_usage & IS_ORPHAN) &&\n> >           rename_ref(oldref.buf, newref.buf, logmsg.buf))\n> > -             die(_(\"Branch rename failed\"));\n> > +             die(_(\"branch rename failed\"));\n> >       if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))\n> > -             die(_(\"Branch copy failed\"));\n> > +             die(_(\"branch copy failed\"));\n>\n> Ditto\n>\n> >\n> >       if (recovery) {\n> >               if (copy)\n> > @@ -640,16 +640,16 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n> >       if (!copy && (oldref_usage & IS_HEAD) &&\n> >           replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,\n> >                                             logmsg.buf))\n> > -             die(_(\"Branch renamed to %s, but HEAD is not updated!\"), newname);\n> > +             die(_(\"branch renamed to %s, but HEAD is not updated!\"), newname);\n>\n> IMO we can go further here and also remove that final \"!\".  But it's OK\n> the way you have done it.\n>\n> >\n> >       strbuf_release(&logmsg);\n> >\n> >       strbuf_addf(&oldsection, \"branch.%s\", interpreted_oldname);\n> >       strbuf_addf(&newsection, \"branch.%s\", interpreted_newname);\n> >       if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)\n> > -             die(_(\"Branch is renamed, but update of config-file failed\"));\n> > +             die(_(\"branch is renamed, but update of config-file failed\"));\n> >       if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)\n> > -             die(_(\"Branch is copied, but update of config-file failed\"));\n> > +             die(_(\"branch is copied, but update of config-file failed\"));\n>\n> OK, both.\n>\n> >       strbuf_release(&oldref);\n> >       strbuf_release(&newref);\n> >       strbuf_release(&oldsection);\n> > @@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> >\n> >       head = resolve_refdup(\"HEAD\", 0, &head_oid, NULL);\n> >       if (!head)\n> > -             die(_(\"Failed to resolve HEAD as a valid ref.\"));\n> > +             die(_(\"failed to resolve HEAD as a valid ref\"));\n>\n> OK.\n>\n> >       if (!strcmp(head, \"HEAD\"))\n> >               filter.detached = 1;\n> >       else if (!skip_prefix(head, \"refs/heads/\", &head))\n> > @@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> >\n> >               if (!argc) {\n> >                       if (filter.detached)\n> > -                             die(_(\"Cannot give description to detached HEAD\"));\n> > +                             die(_(\"cannot give description to detached HEAD\"));\n>\n> OK.\n>\n> >                       branch_name = head;\n> >               } else if (argc == 1) {\n> >                       strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n> > @@ -892,7 +892,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> >               if (!argc)\n> >                       die(_(\"branch name required\"));\n> >               else if ((argc == 1) && filter.detached)\n> > -                     die(copy? _(\"cannot copy the current branch while not on any.\")\n> > +                     die(copy? _(\"cannot copy the current branch while not on any\")\n> >                               : _(\"cannot rename the current branch while not on any.\"));\n>\n> OK.  But I think you want to modify also the second message, to remove\n> its full stop as well.\n>\n> >               else if (argc == 1)\n> >                       copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);\n> > @@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> >               if (!branch) {\n> >                       if (!argc || !strcmp(argv[0], \"HEAD\"))\n> >                               die(_(\"could not set upstream of HEAD to %s when \"\n> > -                                   \"it does not point to any branch.\"),\n> > +                                   \"it does not point to any branch\"),\n>\n> OK.\n>\n> >                                   new_upstream);\n> >                       die(_(\"no such branch '%s'\"), argv[0]);\n> >               }\n> >\n> >               if (!ref_exists(branch->refname)) {\n> >                       if (!argc || branch_checked_out(branch->refname))\n> > -                             die(_(\"No commit on branch '%s' yet.\"), branch->name);\n> > +                             die(_(\"no commit on branch '%s' yet\"), branch->name);\n>\n> OK.\n>\n> >                       die(_(\"branch '%s' does not exist\"), branch->name);\n> >               }\n> >\n> > @@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> >               if (!branch) {\n> >                       if (!argc || !strcmp(argv[0], \"HEAD\"))\n> >                               die(_(\"could not unset upstream of HEAD when \"\n> > -                                   \"it does not point to any branch.\"));\n> > +                                   \"it does not point to any branch\"));\n> >                       die(_(\"no such branch '%s'\"), argv[0]);\n> >               }\n> >\n> >               if (!branch_has_merge_config(branch))\n> > -                     die(_(\"Branch '%s' has no upstream information\"), branch->name);\n> > +                     die(_(\"branch '%s' has no upstream information\"), branch->name);\n>\n> OK, both.\n>\n> >\n> >               strbuf_reset(&buf);\n> >               strbuf_addf(&buf, \"branch.%s.remote\", branch->name);\n> > @@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> >               const char *start_name = argc == 2 ? argv[1] : head;\n> >\n> >               if (filter.kind != FILTER_REFS_BRANCHES)\n> > -                     die(_(\"The -a, and -r, options to 'git branch' do not take a branch name.\\n\"\n> > +                     die(_(\"the -a, and -r, options to 'git branch' do not take a branch name\\n\"\n> >                                 \"Did you mean to use: -a|-r --list <pattern>?\"));\n>\n> Good; the full stop removed and here that question mark is valuable.  And ...\n>\n> >\n> >               if (track == BRANCH_TRACK_OVERRIDE)\n> > -                     die(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.\"));\n> > +                     die(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead\"));\n>\n> also good.  But since we're here, maybe we can break this long message, remove\n> the first full stop and leave the second part of the message as is, as a\n> suggestion.  Like we do in the previous message, above.\n>\n> For your consideration, I mean something like:\n>\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -969,7 +969,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>                                   \"Did you mean to use: -a|-r --list <pattern>?\"));\n>\n>                 if (track == BRANCH_TRACK_OVERRIDE)\n> -                       die(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.\"));\n> +                       die(_(\"the '--set-upstream' option is no longer supported\\n\"\n> +                             \"Please use '--track' or '--set-upstream-to' instead.\"));\n>\n>\n> >\n> >               if (recurse_submodules) {\n> >                       create_branches_recursively(the_repository, branch_name,\n> > --\n> > 2.42.0.325.g3a06386e31\n> >\n>\n> One final comment, leaving aside my suggestions; this changes break\n> some tests that you need to adjust.  Try:\n>\n>    $ make && (cd t; ./t3200-branch.sh -vi)\n>\n> Or, as Junio has already suggested in another message:\n>\n>    $ make test\n>\n> I have some unfinished work that you might find useful:\n>\n> https://lore.kernel.org/git/eb3c689e-efeb-4468-a10f-dd32bc0ee37b@gmail.com/\nThanks for the review. I'll make the requested changes and provide an\nupdate. I'll also make sure to run the tests you suggested and make\nthe necessary changes.\n>\n> Thank you for working on this.\n"}]}