From dd30800bcd236233c82da80bba0d00956a246260 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 30 Apr 2014 14:23:26 -0700 Subject: CodingGuidelines: once it is in, it is not worth the code churn Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index f424dbd75c..c405b0b9df 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -18,6 +18,14 @@ code. For Git in general, three rough rules are: judgement call, the decision 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 -- cgit v1.2.3 From 79fc3ca1232099fd21e5bdef14c1b6fe1d2dbc1c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 30 Apr 2014 14:24:08 -0700 Subject: CodingGuidelines: give an example for case/esac statement Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index c405b0b9df..169b4358c9 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -42,7 +42,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"' -- cgit v1.2.3 From 6a49909b52b48592234da6a53bfe74ea34c302c6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 30 Apr 2014 14:24:24 -0700 Subject: CodingGuidelines: give an example for redirection Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 169b4358c9..11704fb84c 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 "$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. -- cgit v1.2.3 From 9dbe780174892a35d6f1e19495c13b9edb9d88fd Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 30 Apr 2014 14:24:52 -0700 Subject: CodingGuidelines: give an example for control statements Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 11704fb84c..47db6b3ce3 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -107,6 +107,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 -- cgit v1.2.3 From 6117a3d4946e35c7d4a38c0b443891c81808a838 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 30 Apr 2014 14:25:11 -0700 Subject: CodingGuidelines: give an example for shell function preamble Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 47db6b3ce3..0a574b204c 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -123,9 +123,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. -- cgit v1.2.3 From 691d0dd0a9c901286c2a0a28c30ec4d13bcd2032 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 30 Apr 2014 14:25:47 -0700 Subject: CodingGuidelines: do not call the conditional statement "if()" The point immediately before it is about having SP after the control keyword. Spell it out as 'an "if" statement' instead. Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 0a574b204c..aeaa82451e 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -194,7 +194,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 in the condition of an "if" statement. - Try to make your code understandable. You may put comments in, but comments invariably tend to stale out when the code -- cgit v1.2.3 From 5db9ab82b94fab16e69b0228aaf1e972520bd04a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 30 Apr 2014 14:26:23 -0700 Subject: CodingGuidelines: on comparison 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. See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126 for the original discussion. Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index aeaa82451e..02ca67c4ca 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -222,6 +222,33 @@ 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 the less stable + value on the left hand side and the 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. However, the more "stable" the + stable side becomes, the more we tend to prefer the former + (comparison with a constant, "i > 0", is an extreme example). + Just do not mix styles in the same part of the code and mimic + existing styles in the neighbourhood. + - 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. -- cgit v1.2.3 From f26443da0449c459dab8b91d963bd91dd4335657 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 2 May 2014 13:42:39 -0700 Subject: CodingGuidelines: on splitting a long line Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 55 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 02ca67c4ca..3d08671ad3 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 preferable to split after the operator at higher + level in the parse tree. That is, this is more preferable: + + 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. -- cgit v1.2.3 From 897f964c0dce8e7cc2cc53bb19b83cadce106773 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 20 May 2014 11:12:02 -0700 Subject: CodingGuidelines: avoid "test -a/-o " The construct is error-prone; "test" being built-in in most modern shells, the reason to avoid "test && test " spawning one extra process by using a single "test -a " no longer exists. Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'Documentation') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 3d08671ad3..4d90c77c7b 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -151,6 +151,19 @@ For shell scripts specifically (not exhaustive): interface translatable. See "Marking strings for translation" in po/README. + - We do not write our "test" command with "-a" and "-o" and use "&&" + or "||" to concatenate multiple "test" commands instead, because + the use of "-a/-o" is often error-prone. E.g. + + test -n "$x" -a "$a" = "$b" + + is buggy and breaks when $x is "=", but + + test -n "$x" && test "$a" = "$b" + + does not have such a problem. + + For C programs: - We use tabs to indent, and interpret tabs as taking up to -- cgit v1.2.3