Re: [PATCH 0/9] new git check-ignore sub-command

Adam Spiers [off-list ref] writes:

On Sun, Sep 2, 2012 at 9:35 PM, Junio C Hamano [off-list ref] wrote:

quoted

 * avoid unnnecessary braces {} around single statement blocks, e.g.

    -if (exclude) {
    +if (exclude)
        return exclude;
    -}

 * else should follow close brace '}' of if clause, e.g.

     if (...) {
         ...
    -}
    -else {
    +} else {
         ...

What about when the if clause requires braces but the else clause
doesn't?  Should it be

	if (...) {
		...;
		...;
	} else
		...;

or

	if (...) {
		...;
		...;
	}
	else
		...;

?

Neither.  We try to do (but often fail ;-)

	if (...) {
        	...;
        	...;
	} else {
        	...;
	}

following the kernel style, unless there is good reason not to.

Signed-off-by: Adam Spiers <redacted>
---

I have begun work on fixing existing code to adhere to these
guidelines on braces, but there are currently a lot of violations,
which means any patches to fix them would be large.  So before I spend
any more time on it, I would like to check whether such patches would
be welcomed?  And if so, should I be doing that on the master branch?

I have also added some simple rules such as `check-brace-before-else'
to the top-level Makefile which perform appropriate `grep -n' commands
to detect violations and for example easily fix them via emacs' M-x
grep.  These rules can be invoked together via a `check-style' target.
Would this also be welcomed?  If so, should the checks all be
introduced in a single commit, or each check along with the code which
was fixed with its help?

BTW I briefly tried to find an existing tool out there which could
already do the checking for us, but only found ones like uncrustify
which rewrite code rather than warning on inconsistencies.  I also saw
that the kernel's scripts/checkpatch.pl only worked with patches and
was also extremely kernel-specific in the nature of its checks.

 Documentation/CodingGuidelines | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 57da6aa..1a2851d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines

@@ -117,17 +117,49 @@ For C programs:
    "char * string".  This makes it easier to understand code
    like "char *string, c;".
 
+ - We avoid unnecessary explicit initialization of BSS-allocated vars
+   (static and globals) to zero or NULL:
+
+	static int n;
+	static char **ch;
+
+   rather than:
+
+	static int n = 0;
+	static char **ch = NULL;
+
+   These are superfluous according to ISO/IEC 9899:1999 (a.k.a. C99);
+   see item 10 in section 6.7.8 ("Initialization") of WG14 N1256 for
+   the exact text:
+
+     http://open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf
+
  - We avoid using braces unnecessarily.  I.e.
 
 	if (bla) {
 		x = 1;
 	}
 
-   is frowned upon.  A gray area is when the statement extends
-   over a few lines, and/or you have a lengthy comment atop of
-   it.  Also, like in the Linux kernel, if there is a long list
-   of "else if" statements, it can make sense to add braces to
-   single line blocks.
+   is frowned upon.  A gray area is when the statement extends over a
+   few lines, and/or you have a lengthy comment atop of it.  Also,
+   like in the Linux kernel, it can make sense to add braces to single
+   line blocks if there are already braces in another branch of the
+   same conditional, and/or there is long list of "else if"
+   statements.
+
+ - When braces are required, we cuddle "else" and "else if", so the
+   preceding closing brace appears on the same line, e.g.
+
+	if (foo) {
+		...;
+	} else if (bar) {
+		...;
+		...;
+	} else {
+		...;
+	}
+
+   following Linux kernel style, unless there is a good reason not to.
 
  - We try to avoid assignments inside if().
 

-- 
1.7.12.147.g6d168f4