Re: [PATCH v5] branch: support for shortcuts like @{-1}, completed
- From
Rubén Justo <rjusto@gmail.com>
- Date
- Oct 10, 2022, 06:05 UTC
- Message-ID
- <bf4a45cb-2f44-88bd-e12e-98ca31d5bbd4@gmail.com>
- In-Reply-To
- <xmqq4jwc5x7y.fsf@gitster.g>
On 10/10/22 2:38, Junio C Hamano wrote:
Show 28 quoted lines
> 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...
> I would like to see some kind of free(head) in a clean-up to not get distracted with that. Not a proper leak though and the leak checkers does not refer to that as leak. So not important. We can go with the unconditional return and let the dust settle.