[PATCH 0/8] Update the CodingGuidelines

[PATCH 0/8] Update the CodingGuidelines

[Junio C Hamano]

After seeing a large torrent of patches from Elia, I was re-reading
the CodingGuidelines to see if our preference of $( ... ) over `...`
was properly described, and then I spotted a typo and also lack of
examples on a few points that I recall pointing out during recent
reviews, which grew to this series.

Junio C Hamano (8):
  CodingGuidelines: typofix
  CodingGuidelines: give an example for case/esac statement
  CodingGuidelines: give an example for redirection
  CodingGuidelines: give an example for control statements
  CodingGuidelines: give an example for shell function preamble
  CodingGuidelines: call the conditional statement "if ()", not "if()"
  CodingGuidelines: on comparison
  CodingGuidelines: once it is in, it is not worth the code churn

 Documentation/CodingGuidelines | 81 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 75 insertions(+), 6 deletions(-)

-- 
2.0.0-rc1-355-gd6d6511

[PATCH 1/8] CodingGuidelines: typofix

[Junio C Hamano]

The sentence lacked the necessary verb.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index f424dbd..fdf6269 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -15,7 +15,7 @@ code.  For Git in general, three rough rules are:
    let's use it".
 
    Again, we live in the real world, and it is sometimes a
-   judgement call, the decision based more on real world
+   judgement call, the decision is based more on real world
    constraints people face than what the paper standard says.
 
 Make your code readable and sensible, and don't try to be clever.
-- 
2.0.0-rc1-355-gd6d6511

Re: [PATCH 1/8] CodingGuidelines: typofix

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> The sentence lacked the necessary verb.

No, it didn't.

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/CodingGuidelines | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index f424dbd..fdf6269 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -15,7 +15,7 @@ code.  For Git in general, three rough rules are:
>     let's use it".
>  
>     Again, we live in the real world, and it is sometimes a
                                               ^^
> -   judgement call, the decision based more on real world
> +   judgement call, the decision is based more on real world
>     constraints people face than what the paper standard says.

There is one common "is" for original statement and paraphrase.  Adding
another one turns this into two sentences which cannot sensibly be
connected with a comma.

If you want to fix something here, do s/judgement/judgment/ instead.

-- 
David Kastrup

Re: [PATCH 1/8] CodingGuidelines: typofix

[Junio C Hamano]

David Kastrup <dak@gnu.org> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> The sentence lacked the necessary verb.
>
> No, it didn't.
>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  Documentation/CodingGuidelines | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
>> index f424dbd..fdf6269 100644
>> --- a/Documentation/CodingGuidelines
>> +++ b/Documentation/CodingGuidelines
>> @@ -15,7 +15,7 @@ code.  For Git in general, three rough rules are:
>>     let's use it".
>>  
>>     Again, we live in the real world, and it is sometimes a
>                                                ^^
>> -   judgement call, the decision based more on real world
>> +   judgement call, the decision is based more on real world
>>     constraints people face than what the paper standard says.
>
> There is one common "is" for original statement and paraphrase.  Adding
> another one turns this into two sentences which cannot sensibly be
> connected with a comma.

Thanks for spotting.

I thought (but I see I didn't by mistake) that I split them into two
sentences, replacing the comma with a semicolon.

> If you want to fix something here, do s/judgement/judgment/ instead.

That too.

Re: [PATCH 1/8] CodingGuidelines: typofix

[Jeff King]

On Thu, May 01, 2014 at 10:51:19AM -0700, Junio C Hamano wrote:

> > If you want to fix something here, do s/judgement/judgment/ instead.
> 
> That too.

FWIW, neither is outright wrong; it is an America/British variation, and
apparently dictionaries disagree on which is preferred.

-Peff

Re: [PATCH 1/8] CodingGuidelines: typofix

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, May 01, 2014 at 10:51:19AM -0700, Junio C Hamano wrote:
>
>> > If you want to fix something here, do s/judgement/judgment/ instead.
>> 
>> That too.
>
> FWIW, neither is outright wrong; it is an America/British variation, and
> apparently dictionaries disagree on which is preferred.

My reading of various "grammar" sites was that even though variation
exists[*1*], the form without 'e' is the traditionally preferred
form, and that is why I said "That too".

But let's follow this one:

http://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q

which seems to say that with 'e' is more common.


[Footnote]

*1* To Americans, the form with 'e' is abomination.  Wikipedia
claims that (1) without 'e' is in legal and (2) with 'e' in other
contexts in British (this particular one is a non-legal use), and
(3) both are equally acceptable in non-legal contexts in Austraria
and Canada.

Re: [PATCH 1/8] CodingGuidelines: typofix

[Jeff King]

On Fri, May 02, 2014 at 11:31:10AM -0700, Junio C Hamano wrote:

> But let's follow this one:
> 
> http://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q
> 
> which seems to say that with 'e' is more common.

Grammar by democracy. ;)

> *1* To Americans, the form with 'e' is abomination.  Wikipedia
> claims that (1) without 'e' is in legal and (2) with 'e' in other
> contexts in British (this particular one is a non-legal use), and
> (3) both are equally acceptable in non-legal contexts in Austraria
> and Canada.

That is what I found most interesting about the discussion. The reason I
bothered to look it up and say something is that as an American, I would
without a doubt spell it with the "e", contradicting what I found
online. Oh well. 

-Peff

Re: [PATCH 1/8] CodingGuidelines: typofix

[Felipe Contreras]

Jeff King wrote:
> On Fri, May 02, 2014 at 11:31:10AM -0700, Junio C Hamano wrote:
> 
> > But let's follow this one:
> > 
> > http://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q
> > 
> > which seems to say that with 'e' is more common.
> 
> Grammar by democracy. ;)

Languages are a democracy. There's no authority that decides if
"unibrow" should become part of the English language. We all do.

-- 
Felipe Contreras

Re: [PATCH 1/8] CodingGuidelines: typofix

[David Kastrup]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Jeff King wrote:
>> On Fri, May 02, 2014 at 11:31:10AM -0700, Junio C Hamano wrote:
>> 
>> > But let's follow this one:
>> > 
>> > http://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q
>> > 
>> > which seems to say that with 'e' is more common.
>> 
>> Grammar by democracy. ;)
>
> Languages are a democracy. There's no authority that decides if
> "unibrow" should become part of the English language. We all do.

Well, and the U.S. justice system rather supports the hyphenation judge-
mental.

-- 
David Kastrup

[PATCH 2/8] CodingGuidelines: give an example for case/esac statement

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index fdf6269..9b103cb 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -34,7 +34,17 @@ For shell scripts specifically (not exhaustive):
 
  - We use tabs for indentation.
 
- - Case arms are indented at the same depth as case and esac lines.
+ - Case arms are indented at the same depth as case and esac lines,
+   like this:
+
+	case "$variable" in
+	pattern1)
+		do this
+		;;
+	pattern2)
+		do that
+		;;
+	esac
 
  - Redirection operators should be written with space before, but no
    space after them.  In other words, write 'echo test >"$file"'
-- 
2.0.0-rc1-355-gd6d6511

[PATCH 3/8] CodingGuidelines: give an example for redirection

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 9b103cb..1e0c4cf 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -53,6 +53,12 @@ For shell scripts specifically (not exhaustive):
    redirection target in a variable (as shown above), our code does so
    because some versions of bash issue a warning without the quotes.
 
+	cat hello > world < universe	# incorrect
+	cat hello >world <universe	# correct
+
+	echo hello >$world		# incorrect
+	echo hello >"$world"		# correct
+
  - We prefer $( ... ) for command substitution; unlike ``, it
    properly nests.  It should have been the way Bourne spelled
    it from day one, but unfortunately isn't.
-- 
2.0.0-rc1-355-gd6d6511

[PATCH v2 3/8] CodingGuidelines: give an example for redirection

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Instead of using trailing "# correct" comment on each of the
   one-liners, group the good ones and the bad ones into two groups
   and give (incorrect)/(correct) header like the other patches as
   suggested.  How does this look?

   The only reason I originally did these differently is because
   these are two independent examples, and I thought it would make
   the difference stand out better to the readers if we showed
   good/bad pairs of "cat" and "echo" that way.  But as long as we
   are not going to add more examples here, I think the result is
   not too bad.

 Documentation/CodingGuidelines | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 6a8e94b..6bfe96e 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -61,6 +61,14 @@ For shell scripts specifically (not exhaustive):
    redirection target in a variable (as shown above), our code does so
    because some versions of bash issue a warning without the quotes.
 
+	(incorrect)
+	cat hello > world < universe
+	echo hello >$world
+
+	(correct)
+	cat hello >world <universe
+	echo hello >"$world"
+
  - We prefer $( ... ) for command substitution; unlike ``, it
    properly nests.  It should have been the way Bourne spelled
    it from day one, but unfortunately isn't.
-- 
2.0.0-rc1-355-gd6d6511

[PATCH 4/8] CodingGuidelines: give an example for control statements

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 1e0c4cf..d72e912 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):
    "then" should be on the next line for if statements, and "do"
    should be on the next line for "while" and "for".
 
+	(incorrect)
+	if test -f hello; then
+		do this
+	fi
+
+	(correct)
+	if test -f hello
+	then
+		do this
+	fi
+
  - We prefer "test" over "[ ... ]".
 
  - We do not write the noiseword "function" in front of shell
-- 
2.0.0-rc1-355-gd6d6511

Re: [PATCH 4/8] CodingGuidelines: give an example for control statements

[Stefan Beller]

On 30.04.2014 23:45, Junio C Hamano wrote:
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/CodingGuidelines | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 1e0c4cf..d72e912 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):
>     "then" should be on the next line for if statements, and "do"
>     should be on the next line for "while" and "for".
>  
> +	(incorrect)

At the other patches you used #comments behind oneliners,
not sure if that's also suitable here for consistency of the documentation.

> +	if test -f hello; then
> +		do this
> +	fi
> +
> +	(correct)
> +	if test -f hello
> +	then
> +		do this
> +	fi
> +
>   - We prefer "test" over "[ ... ]".
>  
>   - We do not write the noiseword "function" in front of shell
> 

Re: [PATCH 4/8] CodingGuidelines: give an example for control statements

[Junio C Hamano]

Stefan Beller <stefanbeller@gmail.com> writes:

> On 30.04.2014 23:45, Junio C Hamano wrote:
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  Documentation/CodingGuidelines | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
>> index 1e0c4cf..d72e912 100644
>> --- a/Documentation/CodingGuidelines
>> +++ b/Documentation/CodingGuidelines
>> @@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):
>>     "then" should be on the next line for if statements, and "do"
>>     should be on the next line for "while" and "for".
>>  
>> +	(incorrect)
>
> At the other patches you used #comments behind oneliners,
> not sure if that's also suitable here for consistency of the documentation.

I think you meant this one (are there others?):

+	cat hello > world < universe	# incorrect
+	cat hello >world <universe	# correct
+
+	echo hello >$world		# incorrect
+	echo hello >"$world"		# correct
+

I can certainly go with this instead over there:

	(incorrect)
	cat hello > world < universe
	echo hello >$world

	(correct)
	cat hello >world <universe
	echo hello >"$world"

I do not think it is wise to use the trailing comment style on all
the incorrect/correct lines for these examples.

>> +	if test -f hello; then
>> +		do this
>> +	fi
>> +
>> +	(correct)
>> +	if test -f hello
>> +	then
>> +		do this
>> +	fi
>> +
>>   - We prefer "test" over "[ ... ]".
>>  
>>   - We do not write the noiseword "function" in front of shell
>> 

Re: [PATCH 4/8] CodingGuidelines: give an example for control statements

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/CodingGuidelines | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 1e0c4cf..d72e912 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):
>     "then" should be on the next line for if statements, and "do"
>     should be on the next line for "while" and "for".
>  
> +	(incorrect)
> +	if test -f hello; then
> +		do this
> +	fi
> +
> +	(correct)
> +	if test -f hello
> +	then
> +		do this
> +	fi
> +
>   - We prefer "test" over "[ ... ]".
>  
>   - We do not write the noiseword "function" in front of shell

s/noiseword/bashism/

-- 
David Kastrup

Re: [PATCH 4/8] CodingGuidelines: give an example for control statements

[Junio C Hamano]

David Kastrup <dak@gnu.org> writes:

>>   - We do not write the noiseword "function" in front of shell
>
> s/noiseword/bashism/

That is outside the scope of this patch, but since you brought it
up...

I did consider between noiseword and bashism when I wrote this part,
and decided against "bashism".  XCU 2.4 "Reserved Words" lists it
(among others) and says

    ... may be recognized as reserved words on some implementations
    ... causing unspecified results

Even if "bash" were not the only shell that uses "function" keyword
to introduce a shell function definition, we wouldn't use it.  As we
say in the introductory part, we may say "It is not in POSIX, but it
is supported so widely and using it give us so great a benefit, so
we do use it" for some things, but "function" is not one of them.

The reason is because it is a noiseword and its use is not necessary
in order to define a shell function.

[PATCH 5/8] CodingGuidelines: give an example for shell function preamble

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index d72e912..1b0cd2b 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -113,9 +113,17 @@ For shell scripts specifically (not exhaustive):
  - We do not write the noiseword "function" in front of shell
    functions.
 
- - We prefer a space between the function name and the parentheses. The
-   opening "{" should also be on the same line.
-   E.g.: my_function () {
+ - We prefer a space between the function name and the parentheses,
+   and no space inside the parentheses. The opening "{" should also
+   be on the same line.
+
+	(incorrect)
+	my_function(){
+		...
+
+	(correct)
+	my_function () {
+		...
 
  - As to use of grep, stick to a subset of BRE (namely, no \{m,n\},
    [::], [==], or [..]) for portability.
-- 
2.0.0-rc1-355-gd6d6511

[PATCH 6/8] CodingGuidelines: call the conditional statement "if ()", not "if()"

[Junio C Hamano]

The point immediately before it is about having SP after the control
keyword.  Follow it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 1b0cd2b..21e4272 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -184,7 +184,7 @@ For C programs:
    of "else if" statements, it can make sense to add braces to
    single line blocks.
 
- - We try to avoid assignments inside if().
+ - We try to avoid assignments inside "if ()" condition.
 
  - Try to make your code understandable.  You may put comments
    in, but comments invariably tend to stale out when the code
-- 
2.0.0-rc1-355-gd6d6511

Re: [PATCH 6/8] CodingGuidelines: call the conditional statement "if ()", not "if()"

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> The point immediately before it is about having SP after the control
> keyword.  Follow it.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/CodingGuidelines | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 1b0cd2b..21e4272 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -184,7 +184,7 @@ For C programs:
>     of "else if" statements, it can make sense to add braces to
>     single line blocks.
>  
> - - We try to avoid assignments inside if().
> + - We try to avoid assignments inside "if ()" condition.

That's not grammatical.

-- 
David Kastrup

Re: [PATCH 6/8] CodingGuidelines: call the conditional statement "if ()", not "if()"

[Junio C Hamano]

David Kastrup <dak@gnu.org> writes:

>> - - We try to avoid assignments inside if().
>> + - We try to avoid assignments inside "if ()" condition.
>
> That's not grammatical.

OK,

	... inside the condition part of an "if ()" statement.

then?

Re: [PATCH 6/8] CodingGuidelines: call the conditional statement "if ()", not "if()"

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> David Kastrup <dak@gnu.org> writes:
>
>>> - - We try to avoid assignments inside if().
>>> + - We try to avoid assignments inside "if ()" condition.
>>
>> That's not grammatical.
>
> OK,
>
> 	... inside the condition part of an "if ()" statement.
>
> then?

       ... in the condition of an "if" statement

is what I would write.  "if ()" statement is mixing name and shorthand
for the statement.

-- 
David Kastrup

[PATCH 7/8] CodingGuidelines: on comparison

[Junio C Hamano]

See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 21e4272..86fb9f6 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -212,6 +212,32 @@ For C programs:
  - Double negation is often harder to understand than no negation
    at all.
 
+ - There are two schools of thought when it comes to comparison,
+   especially inside a loop. Some people prefer to have less stable
+   value on the left hand side and more stable value on the right hand
+   side, e.g. if you have a loop that counts variable i down to the
+   lower bound,
+
+	while (i > lower_bound) {
+		do something;
+		i--;
+	}
+
+   Other people prefer to have the textual order of values match the
+   actual order of values in their comparison, so that they can
+   mentally draw a number line from left to right and place these
+   values in order, i.e.
+
+	while (lower_bound < i) {
+		do something;
+		i--;
+	}
+
+   Both are valid, and we use both, even though we tend to see the
+   former the more preferable, the more "stable" the more stable side
+   becomes (comparison with a constant, "i > 0", is an extreme
+   example).  Just do not mix styles in the same part of the code.
+
  - Some clever tricks, like using the !! operator with arithmetic
    constructs, can be extremely confusing to others.  Avoid them,
    unless there is a compelling reason to use them.
-- 
2.0.0-rc1-355-gd6d6511

Re: [PATCH 7/8] CodingGuidelines: on comparison

[Jeff King]

On Wed, Apr 30, 2014 at 02:45:11PM -0700, Junio C Hamano wrote:

> See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Don't you often complain about submitters referencing a discussion
in a commit message without providing some context or summary?

> + - There are two schools of thought when it comes to comparison,
> +   especially inside a loop. Some people prefer to have less stable
> +   value on the left hand side and more stable value on the right hand
> +   side, e.g. if you have a loop that counts variable i down to the
> +   lower bound,

Grammar: /(less|more) stable/the &/

> +   Both are valid, and we use both, even though we tend to see the
> +   former the more preferable, the more "stable" the more stable side
> +   becomes (comparison with a constant, "i > 0", is an extreme
> +   example).  Just do not mix styles in the same part of the code.
> +

I had trouble parsing the first sentence. Maybe:

  Both are valid, and we use both. However, the more "stable" the stable
  side becomes, the more we tend to prefer the former (comparison with a
  constant[...]

-Peff

Re: [PATCH 7/8] CodingGuidelines: on comparison

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Apr 30, 2014 at 02:45:11PM -0700, Junio C Hamano wrote:
>
>> See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126
>> 
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> Don't you often complain about submitters referencing a discussion
> in a commit message without providing some context or summary?

Yes, but the summary of the discussion would be identical to the new
text added by the patch to the documentation tree in this case, so I
didn't find a good introductory text before "See $URL".  Perhaps

    This comes up from time to time.  See $URL for the original
    discussion.

but I do not know if that is much better.

>> + - There are two schools of thought when it comes to comparison,
>> +   especially inside a loop. Some people prefer to have less stable
>> +   value on the left hand side and more stable value on the right hand
>> +   side, e.g. if you have a loop that counts variable i down to the
>> +   lower bound,
>
> Grammar: /(less|more) stable/the &/
>
>> +   Both are valid, and we use both, even though we tend to see the
>> +   former the more preferable, the more "stable" the more stable side
>> +   becomes (comparison with a constant, "i > 0", is an extreme
>> +   example).  Just do not mix styles in the same part of the code.
>> +
>
> I had trouble parsing the first sentence. Maybe:
>
>   Both are valid, and we use both. However, the more "stable" the stable
>   side becomes, the more we tend to prefer the former (comparison with a
>   constant[...]

Thanks.  That is much better.

Re: [PATCH 7/8] CodingGuidelines: on comparison

[Jeff King]

On Fri, May 02, 2014 at 11:18:34AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Wed, Apr 30, 2014 at 02:45:11PM -0700, Junio C Hamano wrote:
> >
> >> See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126
> >> 
> >> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> >
> > Don't you often complain about submitters referencing a discussion
> > in a commit message without providing some context or summary?
> 
> Yes, but the summary of the discussion would be identical to the new
> text added by the patch to the documentation tree in this case, so I
> didn't find a good introductory text before "See $URL".  Perhaps
> 
>     This comes up from time to time.  See $URL for the original
>     discussion.
> 
> but I do not know if that is much better.

I meant something even less in-depth. Your message says only "on
comparison", and I did not even know what "this" in your sentence above
would mean until I followed the link.

  There are arguments for writing a conditional as "a < b" rather than
  "b > a", or vice versa. Let's give guidance on which we prefer.

Not a big deal, but I think it is easy when you have just written the
patch to forget that a reviewer or a reader of "git log" six months have
no has no context at all.

-Peff

Re: [PATCH 7/8] CodingGuidelines: on comparison

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I meant something even less in-depth. Your message says only "on
> comparison", and I did not even know what "this" in your sentence above
> would mean until I followed the link.
>
>   There are arguments for writing a conditional as "a < b" rather than
>   "b > a", or vice versa. Let's give guidance on which we prefer.
>
> Not a big deal, but I think it is easy when you have just written the
> patch to forget that a reviewer or a reader of "git log" six months have
> no has no context at all.

Thanks; I'll steal that one.

[PATCH 8/8] CodingGuidelines: once it is in, it is not worth the code churn

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 86fb9f6..759003e 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -18,6 +18,14 @@ code.  For Git in general, three rough rules are:
    judgement call, the decision is based more on real world
    constraints people face than what the paper standard 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
+
 Make your code readable and sensible, and don't try to be clever.
 
 As for more concrete guidelines, just imitate the existing code
-- 
2.0.0-rc1-355-gd6d6511

[PATCH 9/8] CodingGuidelines: on splitting a long line

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 55 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 02ca67c..4dd07c3 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -249,6 +249,61 @@ For C programs:
    Just do not mix styles in the same part of the code and mimic
    existing styles in the neighbourhood.
 
+ - There are two schools of thought when it comes to splitting a long
+   logical line into multiple lines.  Some people push the second and
+   subsequent lines far enough to the right with tabs and align them:
+
+        if (the_beginning_of_a_very_long_expression_that_has_to ||
+		span_more_than_a_single_line_of ||
+		the_source_text) {
+                ...
+
+   while other people prefer to align the second and the subsequent
+   lines with the column immediately inside the opening parenthesis,
+   with tabs and spaces, following our "tabstop is always a multiple
+   of 8" convention:
+
+        if (the_beginning_of_a_very_long_expression_that_has_to ||
+	    span_more_than_a_single_line_of ||
+	    the_source_text) {
+                ...
+
+   Both are valid, and we use both.  Again, just do not mix styles in
+   the same part of the code and mimic existing styles in the
+   neighbourhood.
+
+ - When splitting a long logical line, some people change line before
+   a binary operator, so that the result looks like a parse tree when
+   you turn your head 90-degrees counterclockwise:
+
+        if (the_beginning_of_a_very_long_expression_that_has_to
+	    || span_more_than_a_single_line_of_the_source_text) {
+
+   while other people prefer to leave the operator at the end of the
+   line:
+
+        if (the_beginning_of_a_very_long_expression_that_has_to ||
+	    span_more_than_a_single_line_of_the_source_text) {
+
+   Both are valid, but we tend to use the latter more, unless the
+   expression gets fairly complex, in which case the former tends to
+   be easier to read.  Again, just do not mix styles in the same part
+   of the code and mimic existing styles in the neighbourhood.
+
+ - When splitting a long logical line, with everything else being
+   equal, it is preferrable to split after the operator at higher
+   level in the parse tree.  That is, this is more preferrable:
+
+	if (a_very_long_variable * that_is_used_in +
+	    a_very_long_expression) {
+		...
+
+   than
+
+	if (a_very_long_variable *
+	    that_is_used_in + a_very_long_expression) {
+		...
+
  - Some clever tricks, like using the !! operator with arithmetic
    constructs, can be extremely confusing to others.  Avoid them,
    unless there is a compelling reason to use them.
-- 
2.0.0-rc1-355-gd6d6511

Re: [PATCH 9/8] CodingGuidelines: on splitting a long line

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

On Fri, May 02, 2014 at 01:51:55PM -0700, Junio C Hamano wrote:
> + - When splitting a long logical line, with everything else being
> +   equal, it is preferrable to split after the operator at higher

dict.org says that it should be "preferable", not "preferrable".

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]