Re: [PATCH v5] branch: support for shortcuts like @{-1}, completed

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 10, 2022, 00:38 UTC
Message-ID: <xmqq4jwc5x7y.fsf@gitster.g>
In-Reply-To: <dacb1f40-2765-5978-1131-f78ba919e6e6@gmail.com>

Rubén Justo <rjusto@gmail.com> writes:

Show 16 quoted lines

> Mmm, I don't feel the same here, we already discussed about this. Maybe?:
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 17853225fa..307073cc47 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -817,7 +817,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 strbuf_release(&branch_ref);
>                 strbuf_release(&buf);
>  
> -               return ret;
> +               if (ret)
> +                       return ret; /* some failure happened */
>         } else if (copy) {
>                 if (!argc)
>                         die(_("branch name required"));

Before the above change, the body of the "else if" clause for the option was self contained. With the above change, the reader has to follow to the end of the long top-level cascade to see the rest of the function does not do anything funny.

If we have a big common clean-up after each operation, then, falling through in the success case might be good, but that is not what I am seeing here. So...