Re: [PATCH 0/3] Fix $((...)) coding style

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 4, 2016, 19:43 UTC
Message-ID: <xmqq1t8si7my.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160204155316.GI29880@serenity.lan>

John Keeping <john@keeping.me.uk> writes:

Show 18 quoted lines

> I avoided quoting CodingGuidelines in my previous message, but it says:
>
>  - Fixing style violations while working on a real change as a
>    preparatory clean-up step is good, but otherwise avoid useless code
>    churn for the sake of conforming to the style.
>
>    "Once it _is_ in the tree, it's not really worth the patch noise to
>    go and fix it up."
>    Cf. http://article.gmane.org/gmane.linux.kernel/943020
>
>> Also, we *did* document the policy in the CodingGuidelines:
>> 
>> 	As for more concrete guidelines, just imitate the existing code
>
> This is the first point I made.  To take one example, in
> git-filter-branch.sh there are five occurrences of the sequence "$((";
> your patch changes four of them to remove spaces.  If we standardise on
> having spaces only one needs to change:

I agree that our codebase seems to have a moderately strong preference for having SP around binary operators, i.e.

	$term1 * $term2 + $term3

over not having one:

	$term1*$term2+$term3

and I think that is OK to declare it as our style, primarily because that is how we encourage to write our expressions in C, as you mentioned earlier.

One thing I was wondering about the $(( ... )) syntax while reading this thread was about the SP around the expression, i.e.

	var=$(( $term1 * $term2 + $term3 ))

vs

	var=$(($term1 * $term2 + $term3))

I personally do not have strong preference between the two, but I have a vague impression that we preferred the former because somebody in the past gave us a good explanation why we should.

"git grep" however seems to tell us that we are not clearly decided between the two, so I probably am misremembering it and there is no preference either way.

In any case, it is good to catch a patch that mixes style changes and other things during a review, and it also is good to clean up the style before you start working on a specific part of the code to make gradual improvement without stepping on others' toes.

Thanks.