{"thread":{"id":"58690","subject":"[PATCH] branch: error code with --edit-description","startedAt":"2022-10-25T22:57:27Z","lastAt":"2022-10-25T23:43:54Z","messageCount":2,"participants":["Rubén Justo","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"465740","messageId":"b0f96b35-4e69-a889-bcdf-e0b40b89384f@gmail.com","threadId":"58690","inReplyTo":null,"subject":"[PATCH] branch: error code with --edit-description","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2022-10-25T22:57:18Z","receivedAt":"2022-10-25T22:57:27Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"Since c2d17ba3db0d (branch --edit-description: protect against mistyped\nbranch name, 2012-02-05) we return -1 on error editing the branch\ndescription.\n\nLet's change to 1, which follows the established convention and it is\nbetter for portability reasons.\n\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n builtin/branch.c  | 2 +-\n t/t3200-branch.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex c964ac7bb4..15be0c03ef 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -809,7 +809,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \n \t\tstrbuf_addf(&branch_ref, \"refs/heads/%s\", branch_name);\n \t\tif (!ref_exists(branch_ref.buf))\n-\t\t\tret = error((!argc || !strcmp(head, branch_name))\n+\t\t\terror((!argc || !strcmp(head, branch_name))\n \t\t\t      ? _(\"No commit on branch '%s' yet.\")\n \t\t\t      : _(\"No branch named '%s'.\"),\n \t\t\t      branch_name);\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 38c57de71b..7f605f865b 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1394,7 +1394,7 @@ test_expect_success 'branch --delete --force removes dangling branch' '\n \n test_expect_success 'use --edit-description' '\n \tEDITOR=: git branch --edit-description &&\n-\ttest_must_fail git config branch.main.description &&\n+\ttest_expect_code 1 git config branch.main.description &&\n \n \twrite_script editor <<-\\EOF &&\n \t\techo \"New contents\" >\"$1\"\n-- \n2.36.1\n"},{"id":"465743","messageId":"xmqqv8o7mpu3.fsf@gitster.g","threadId":"58690","inReplyTo":"b0f96b35-4e69-a889-bcdf-e0b40b89384f@gmail.com","subject":"Re: [PATCH] branch: error code with --edit-description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T23:43:48Z","receivedAt":"2022-10-25T23:43:54Z","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> Since c2d17ba3db0d (branch --edit-description: protect against mistyped\n> branch name, 2012-02-05) we return -1 on error editing the branch\n> description.\n>\n> Let's change to 1, which follows the established convention and it is\n> better for portability reasons.\n\nNicely explained.  As ret is initialized to 1 to assume failure (and\nwe set it to 0 when we are happy), the change to stop assigning the\nreturn value from error() is a very sensible implementation, too.\n\nWill queue.  Thanks.\n\n>\n> Signed-off-by: Rubén Justo <rjusto@gmail.com>\n> ---\n>  builtin/branch.c  | 2 +-\n>  t/t3200-branch.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index c964ac7bb4..15be0c03ef 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -809,7 +809,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>  \n>  \t\tstrbuf_addf(&branch_ref, \"refs/heads/%s\", branch_name);\n>  \t\tif (!ref_exists(branch_ref.buf))\n> -\t\t\tret = error((!argc || !strcmp(head, branch_name))\n> +\t\t\terror((!argc || !strcmp(head, branch_name))\n>  \t\t\t      ? _(\"No commit on branch '%s' yet.\")\n>  \t\t\t      : _(\"No branch named '%s'.\"),\n>  \t\t\t      branch_name);\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index 38c57de71b..7f605f865b 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -1394,7 +1394,7 @@ test_expect_success 'branch --delete --force removes dangling branch' '\n>  \n>  test_expect_success 'use --edit-description' '\n>  \tEDITOR=: git branch --edit-description &&\n> -\ttest_must_fail git config branch.main.description &&\n> +\ttest_expect_code 1 git config branch.main.description &&\n>  \n>  \twrite_script editor <<-\\EOF &&\n>  \t\techo \"New contents\" >\"$1\"\n"}]}