Re: [PATCH] branch: error codes for "edit_description"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 8, 2022, 21:45 UTC
- Message-ID
- <xmqq7d2do80r.fsf@gitster.g>
- In-Reply-To
- <de06f80c-4778-7e43-3a88-d7aeae788e0d@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
Show 34 quoted lines
> Stick to the convention of exit with 1 on error. May break some
> third party tool that expects -1 on "No branch named 'bla'" or
> "No commit on branch 'bla' yet." errors.
>
> Test added to avoid regresion on this behaviour.
> ---
>
> Junio, here is the patch for the change you suggested [1]. My concern is
> this might break some third party tool or script. If no one thinks this
> is an issue, I can squash it with the patch I'm responding to.
>
>
> [1] https://lore.kernel.org/git/xmqq7d2fszhk.fsf@gitster.g/
>
> builtin/branch.c | 6 +++---
> t/t3200-branch.sh | 8 ++++++++
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index b1f6519cd9..01b6364f58 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -810,11 +810,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> else
> ret = error(_("No branch named '%s'."),
> branch_name);
> - } else if (edit_branch_description(branch_name))
> - ret = 1;
> + } else
> + ret = edit_branch_description(branch_name);
>
> strbuf_release(&branch_ref);
> - return ret;
> + return -ret;If edit_branch_description() returns -1 on failure, this would consistently exit with 1 on failure, so it does make sense.
It is bad to exit with a value outside 0..255 for portability reasons, but we wrap exit() to chomp off the higher bits, so in practice it may be fine to exit(-1) but still the above is a good clean-up, I would think.