Re: [PATCHv9 1/6] submodule-config: keep update strategy around

From: Stefan Beller <sbeller@google.com>
Date: Feb 9, 2016, 22:19 UTC
Message-ID: <CAGZ79kYm79C7tLECeJCrS3JoCudUT-8geovoMpT8qoHpYBjUAg@mail.gmail.com>
In-Reply-To: <xmqqh9hh382i.fsf@gitster.mtv.corp.google.com>

On Tue, Feb 9, 2016 at 1:08 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

>> +     } else if (!strcmp(item.buf, "update")) {
>> +             if (!value)
>> +                     ret = config_error_nonbool(var);
>> +             else if (!me->overwrite &&
>> +                 submodule->update != SM_UPDATE_UNSPECIFIED)
>
> Funny indentation here (locally fixable).

I looked through the code base and reread our CodingGuidelines to find out what is considered correct. (I assumed we had a gnu-ish coding style w.r.t. breaking overly long lines in conditions, which is having the next line be indented with 4 spaces.)

So I assume by funny you mean "the next line doesn't start below the opening parenthesis"?

That would seem to be consistent as it fits both the 4 space indentation
which is always found below "if (", but we have more than 4 spaces in
other places such as overly long return statements, i.e. refs.c, l 610
(@origin/master)
        return starts_with(refname, "refs/heads/") ||
                starts_with(refname, "refs/remotes/") ||
                starts_with(refname, "refs/notes/") ||
                !strcmp(refname, "HEAD");

which also doesn't seem to align perfectly to me.

Looking for places, which have the pattern "else if (..." with line break, I found several different styles.

builtin/mv.c (@origin/master) has two occurrences of

    else if (...
    <2 additional tabs> condition continues here

and another of

    else if (...
    <1 tab + 1 SP> condition nearly aligned to statement below

Looking at remote.c, I can find 1 tab, plus 3 spaces. ... Giving up to come up with an idea for space based rule other than "visually align it below the original statement".

Puzzled, Stefan