[proposal] styleguide tweak

[Cliff Woolley]

So this is NOT intended to be the catalyst for a huge heated debate, it's
just a suggestion, but I propose the following tweaks to the styleguide.
For the most part, it's just clarifying the English. The only real
changes are ones intended to codify two items most of us already do
anyway: (a) if there is a multi-line conditional expression (with
if/for/etc), then you put the { on the next line to separate the code from
the condition; (b) all blocks should have {}'s, even if they are only one
line long.

--Cliff

------------------------------------------------------
eg:

if (foo) {
then();
}

not
if (foo)
then();

and
if (foo && bar && baz &&
quux && crash && burn)
{
then();
}

not
if (foo && bar && baz &&
quux && crash && burn) {
then();
}

------------------------------------------------------


Index: styleguide.xml
===================================================================
RCS file: /home/cvs/httpd-site/xdocs/dev/styleguide.xml,v
retrieving revision 1.2
diff -u -d -r1.2 styleguide.xml
--- styleguide.xml 2 Oct 2002 00:18:06 -0000 1.2
+++ styleguide.xml 2 Oct 2003 18:50:30 -0000
@@ -13,19 +13,18 @@
href="mailto:***@awe.com">***@awe.com</a></strong>. Based on a vote
taken in November, 1996.
<br />
-Further refinements voted upon in July 1997.
+Further refinements voted upon in July 1997 and October 2003.
</section>

<section>
<title>Introduction</title>
-<p>[This bit could state that code should be laid out to be clear to
-someone else familiar with Apache. Functions should be short and
-easily understood. Comments should be provided to explain the
-rationale for code which is not obvious, and to document behavior of
-functions. The guidelines can be broken if necessary to acheive a
-clearer layout]</p>
+<p>Code should be laid out to be clear to someone else familiar with Apache.
+Functions should be short and easily understood. Comments should be provided to
+explain the rationale for code which is not obvious and to document behavior
+of functions. The guidelines can be broken if necessary to achieve a clearer
+layout.</p>

-<p>This style can be generated with the following arguments to GNU indent:</p>
+<p>This style can be approximated with the following arguments to GNU indent:</p>

<p><ul><code>-i4 -npsl -di0 -br -nce -d0 -cli0 -npcs -nfc1 -nut</code></ul></p>

@@ -34,13 +33,18 @@
<section>
<title>The Guidelines</title>
<UL>
- <li>Opening braces are given on the same lines as statements, or on the
- following line at the start of a function definition.
+ <li>Opening braces are given on the same lines as statements, except
+ at the start of a function definition and at the start of a
+ conditional block with a multi-line condition, where they are on their
+ own line.
</li>
- <li>Code inside a block (whether surrounded by braces or not) is
- indented by four space characters. Tab characters are not
- used. Comments are indented to the same level as the surrounding
- code.
+ <li>
+ All blocks (including conditional blocks of only one line) are surrounded
+ by braces.
+ </li>
+ <li>Code inside a block is indented by four space characters. Tab
+ characters are not used. Comments are indented to the same level as
+ the surrounding code.
</li>
<li>Closing braces are always on a separate line from surrounding
code, and are indented to line up with the start of the text on
@@ -48,7 +52,7 @@
</li>
<li>Functions are declared with ANSI-style arguments.
</li>
- <li>There is no space between the function name and the opening bracket
+ <li>There is no space between the function name and the opening parenthesis
of the arguments to the function. There is a single space following
commas in argument lists and the semi-colons in for statements.
</li>
@@ -99,8 +103,10 @@
and the wrapped portion is indented under the first term in the expression
(or the first argument to the function).
Conditional expressions should be wrapped to keep single or
-parenthesized terms as atomic as possible, and place Boolean operators
-at either the start (preferable) or end of the line.
+parenthesized terms as atomic as possible, placing boolean operators
+at either the start or end of the line. The opening brace of the block
+following this kind of multi-line condition should be on its own line
+to help distinguish the code in the block from the condition itself.
</p>
<p>
Example:
@@ -110,8 +116,9 @@
const char *args, void *foo,
int k)

- if (cond1 &amp;&amp; (item2 || item3) &amp;&amp; (!item4)
- &amp;&amp; (item5 || item6) &amp;&amp; item7) {
+ if (cond1 &amp;&amp; (item2 || item3) &amp;&amp; (!item4) &amp;&amp;
+ (item5 || item6) &amp;&amp; item7)
+ {
do_a_thing();
}
</pre>
@@ -127,9 +134,13 @@
<p>Example:</p>

<pre>
- code;
/* comment */
code;
+ /* really
+ * long
+ * comment
+ */
+ code;
</pre>
</li>
<li><strong>Function Declaration and Layout</strong>
@@ -146,7 +157,7 @@
<p>The return type is placed on the same line as the function. Arguments
(if any) are given in ANSI style. If no arguments, declare function
as <samp>void</samp>. No space
- between function name and opening bracket, single space after comma
+ between function name and opening parenthesis, single space after comma
separating each argument. The opening brace is placed on the line
after the definition, indented to line up with the start of the
return type text. The code is indented with four spaces, and the closing
@@ -157,7 +168,7 @@
<li><strong>Function Calls</strong>

<p>Space after commas in function calls. No space between function name
- and opening bracket.</p>
+ and opening parenthesis.</p>
<pre>
f(a, b);
</pre>
@@ -169,7 +180,8 @@
<li><strong>Flow-Control Layout</strong>

<p>Flow-control statements (<samp>if</samp>, <samp>while</samp>,
- <samp>for</samp>, <EM>etc.</EM>) are laid out as in this example:</p>
+ <samp>for</samp>, <EM>etc.</EM>) are laid out as in this example
+ (using braces even when "code;" is a single line):</p>

<pre>
if (expr) {
@@ -180,10 +192,11 @@
}
</pre>

-<p>There is a space between the keyword and the opening bracket.
+<p>There is a space between the keyword and the opening parenthesis.
Opening brace placed on same line as the flow keyword. The code
itself is indented by four spaces. The closing brace is indented to
- line up with the opening brace. If an <samp>else</samp> clause is used, the
+ line up with the first character of the keyword. If an
+ <samp>else</samp> clause is used, the
<samp>else</samp> keyword is placed on the line following the closing
brace and is indented to line up with the corresponding
<samp>if</samp>. <strong>Also
@@ -234,7 +247,7 @@
</pre>
</li>

-<li><strong>Capitalisation of Enums</strong>
+<li><strong>Capitalization of Enums</strong>
<p>No rule.</p>
</li>
</ol>

[Jeff Trawick]

Post by Cliff Woolley
if (foo) {
then();
}
not
if (foo)
then();

fine with me

Post by Cliff Woolley
and
if (foo && bar && baz &&
quux && crash && burn)
{
then();
}
not
if (foo && bar && baz &&
quux && crash && burn) {
then();
}

I'm quite happy that

[httpd-2.0] $ find . -name '*.c' | xargs grep '^[ \t]+{'

currently produces no output, and I'd prefer that it stay that way ;)

(searching apr and apr-util left as an exercise for the reader)

[Bill Stoddard]

Post by Jeff Trawick

Post by Cliff Woolley
if (foo) {
then();
}
not
if (foo)
then();

fine with me

+1

Post by Jeff Trawick

Post by Cliff Woolley
and
if (foo && bar && baz &&
quux && crash && burn)
{
then();
}
not
if (foo && bar && baz &&
quux && crash && burn) {
then();
}

I'm quite happy that
[httpd-2.0] $ find . -name '*.c' | xargs grep '^[ \t]+{'
currently produces no output, and I'd prefer that it stay that way ;)

roger dodger yessiree i agree.

B

[Cliff Woolley]

Post by Jeff Trawick
I'm quite happy that
[httpd-2.0] $ find . -name '*.c' | xargs grep '^[ \t]+{'
currently produces no output, and I'd prefer that it stay that way ;)

That it does, but

[httpd-2.0] $ find . -name '*.c' | xargs egrep '^[ \t]+{'

(note the 'e' on egrep) produces quite a lot of output, much of which is
the kind of thing I'm talking about.

I am NOT saying opening braces can or should be on their own line for
regular code. ONLY when they follow a conditional expression that spans
lines.

So, for example, this snippet from server/util.c is and would remain
incorrect:

else if (!strcmp(cmd->path, "/") == 0)
{

Whereas this other snippet from server/util.c would become officially
okay:

if (r->server->path
&& !strncmp(r->uri, r->server->path, r->server->pathlen)
&& (r->server->path[r->server->pathlen - 1] == '/'
|| r->uri[r->server->pathlen] == '/'
|| r->uri[r->server->pathlen] == '\0'))
{

--Cliff

[Greg Stein]

Post by Cliff Woolley
...
I am NOT saying opening braces can or should be on their own line for
regular code. ONLY when they follow a conditional expression that spans
lines.

I'd prefer they get their own line all the time :-), so I'll certainly
agree with the sentiment of requiring them after a multi-line conditional.

The rationale for the style guide here is simply that the extra whitespace
"terminates" the multi-line conditional for the reader.

[ my personal opinion is that the whitespace is always needed so the
reader doesn't mistake the code for a multi-line conditional ]

Note: I'm not desiring to raise a debate or change the style. Merely
supporting the notion that a line of almost-whitespace should occur after
the multi-line conditional.

Cheers,
-g

[Jeff Trawick]

Post by Greg Stein

Post by Cliff Woolley
...
I am NOT saying opening braces can or should be on their own line for
regular code. ONLY when they follow a conditional expression that spans
lines.

I'd prefer they get their own line all the time :-), so I'll certainly
agree with the sentiment of requiring them after a multi-line conditional.

well, I can certainly understand that up to a point... long ago the
product I was working on adopted a style with opening brace always on
its own line, and I adjusted my brain as appropriate, and it feels
natural *in the scope of other code following similar style*... and of
course opening brace on its own line is the one true way since it is
consistent with the first brace of a function

one true way or not, consistency with other style guidelines for this
body of code and the bulk of existing code is more important, and I
think this special case handling of the opening brace is going to be a
wart... doesn't match aesthetically, and is an extra rule that doesn't
solve a problem that a number of people have encountered

but I don't really want to argue either, just put up my 2cents and then
shut up

I'm really done :)

[Paul J. Reder]

Post by Jeff Trawick
one true way or not, consistency with other style guidelines for this
body of code and the bulk of existing code is more important, and I
think this special case handling of the opening brace is going to be a
wart... doesn't match aesthetically, and is an extra rule that doesn't
solve a problem that a number of people have encountered

As much as I would like to have all the code formatted according to the
the "one true way" (my personal preference), I have to agree with Jeff
that it should be all or nothing rather than with special exceptions.
Since the code is all formatted in !"one true way" I'd have to vote to
keep it that way. IMHO, an exception for multiline conditionals would
just confuse things.

Now if we want to talk about reformatting *all* the code to the
"one true way"... :) *duck*

[Bill Stoddard]

Post by Paul J. Reder

Post by Jeff Trawick
one true way or not, consistency with other style guidelines for this
body of code and the bulk of existing code is more important, and I
think this special case handling of the opening brace is going to be a
wart... doesn't match aesthetically, and is an extra rule that doesn't
solve a problem that a number of people have encountered

As much as I would like to have all the code formatted according to the
the "one true way" (my personal preference), I have to agree with Jeff
that it should be all or nothing rather than with special exceptions.
Since the code is all formatted in !"one true way" I'd have to vote to
keep it that way. IMHO, an exception for multiline conditionals would
just confuse things.

I have to agree.

Post by Paul J. Reder
Now if we want to talk about reformatting *all* the code to the
"one true way"... :) *duck*

Definitely not, for all the reasons mention by Dean Gaudet in many past
posts.

Bill

[Jeff Trawick]

Post by Cliff Woolley

Post by Jeff Trawick
I'm quite happy that
[httpd-2.0] $ find . -name '*.c' | xargs grep '^[ \t]+{'
currently produces no output, and I'd prefer that it stay that way ;)

That it does, but
[httpd-2.0] $ find . -name '*.c' | xargs egrep '^[ \t]+{'
(note the 'e' on egrep) produces quite a lot of output, much of which is
the kind of thing I'm talking about.

and much of which, it is fair to point out, are structure definitions :)

dang, why does

grep '^{'

work for finding opening braces at the beginning of the line, but

grep '^ +{'

not work for finding opening braces after at least one space?

oh, shoot... + is not handled by default with grep

okay, really shutting up now...

[Greg Stein]

Post by Cliff Woolley
So this is NOT intended to be the catalyst for a huge heated debate, it's
just a suggestion, but I propose the following tweaks to the styleguide.
For the most part, it's just clarifying the English. The only real
changes are ones intended to codify two items most of us already do
anyway: (a) if there is a multi-line conditional expression (with
if/for/etc), then you put the { on the next line to separate the code from
the condition; (b) all blocks should have {}'s, even if they are only one
line long.
...
if (foo && bar && baz &&
quux && crash && burn)
{
then();
}

"should be":

if (foo && bar && baz
&& quux && crash && burn)
{
then();
}

Note the operator on the next line. It lends focus to the fact that you're
combining conditions, and _how_ they are being combined (or/and). It is
harder to read them at the end of a conditional. It isn't as obvious with
your example, but if you have something like:

if (some_function_call(with, arguments, out, the wazoo) ||
another_function_call(with, more, arguments))
{

vs

if (some_function_call(with, arguments, out, the wazoo)
|| another_function_call(with, more, arguments))
{

The latter is much easier to quickly read/parse.

Cheers,
-g

[William A. Rowe, Jr.]

Post by Cliff Woolley

Post by Cliff Woolley
So this is NOT intended to be the catalyst for a huge heated debate, it's
just a suggestion, but I propose the following tweaks to the styleguide.
For the most part, it's just clarifying the English. The only real
changes are ones intended to codify two items most of us already do
anyway: (a) if there is a multi-line conditional expression (with
if/for/etc), then you put the { on the next line to separate the code from
the condition; (b) all blocks should have {}'s, even if they are only one
line long.
...
if (foo && bar && baz &&
quux && crash && burn)
{
then();
}

if (foo && bar && baz
&& quux && crash && burn)
{
then();
}

++1 and I agree with all the rest of the sentiments (I was about
to shoot off the same note as Greg - glad I'm reading ahead again :-)