From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 10 Oct 2022 00:38:41 GMT
Subject: Re: [PATCH v5] branch: support for shortcuts like @{-1}, completed
Message-ID: <xmqq4jwc5x7y.fsf@gitster.g>
In-Reply-To: <dacb1f40-2765-5978-1131-f78ba919e6e6@gmail.com>

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

> 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...