From: Jacob Keller <jacob.keller@gmail.com>
Date: Mon, 16 Jan 2017 04:10:41 GMT
Subject: Re: [PATCH v3 01/38] sequencer: avoid unnecessary curly braces
Message-ID: <CA+P7+xrujKPCxCH9NSvhkxsrs1JojwdgsRvvedZhUVXucsG0QQ@mail.gmail.com>
In-Reply-To: <20170114180550.ebra5qexewetuoyk@sigill.intra.peff.net>

On Sat, Jan 14, 2017 at 10:05 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Jan 14, 2017 at 06:57:13PM +0100, Johannes Schindelin wrote:
>
>> On Thu, 12 Jan 2017, Junio C Hamano wrote:
>>
>> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> >
>> > >
>> > > - if (!commit->parents) {
>> > > + if (!commit->parents)
>> > >           parent = NULL;
>> > > - }
>> > >   else if (commit->parents->next) {
>> > >           /* Reverting or cherry-picking a merge commit */
>> > >           int cnt;
>> >
>> > The result becomes
>> >
>> >     if (...)
>> >             single statement;
>> >     else if (...) {
>> >             multiple;
>> >                 statements;
>> >         }
>> >
>> > which is not quite an improvement.
>>
>> Yet, this used to be the coding style of Git, and your statement comes
>> quite as a surprise to me.
>
> Yeah, I thought we were OK with:
>
>   if (cond)
>         single statement;
>   else {
>         multiple;
>         statements;
>   }
>
> but not the other way around:
>
>   if (cond) {
>         multiple;
>         statements;
>   } else
>         single statement;
>
> I don't know if the "else if" changes that or not, but I certainly have
> written things like your patch does.
>
> -Peff

Personally, I am of the faith that if any branch of the
if-then-else-if-then-else blocks needs braces, then all branches
should use braces. However, I think that

if (condition)
  <line>
else {
  <block>
}

is reasonably close to this, as the main part can still clearly see
the if condition pretty well.

Thanks,
Jake