Lindent fixed to match reality

Lindent fixed to match reality

[Matt Mackall]

I've been fiddling with cleaning up some old code here and suggest the
following to make Lindent match actual practice more closely. This does:

a) (no -psl)

void *foo(void) {

 instead of

void *
foo(void) {

b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
c) (-ncs) "(void *)foo" rather than "(void *) foo"

 tiny-mpm/scripts/Lindent |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN scripts/Lindent~lindent-reality scripts/Lindent
--- tiny/scripts/Lindent~lindent-reality	2004-01-29 13:19:04.000000000 -0600
+++ tiny-mpm/scripts/Lindent	2004-01-29 13:28:23.000000000 -0600
@@ -1,2 +1,2 @@
 #!/bin/sh
-indent -kr -i8 -ts8 -sob -l80 -ss -bs -psl "$@"
+indent -kr -i8 -ts8 -sob -l80 -ss -ncs "$@"

_


-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: Lindent fixed to match reality

[Linus Torvalds]

On Thu, 29 Jan 2004, Matt Mackall wrote:
> 
> a) (no -psl)
> 
> void *foo(void) {
> 
>  instead of
> 
> void *
> foo(void) {

And why not 

	void *foo(void)
	{

which is the _right_ thing to use?

> b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> c) (-ncs) "(void *)foo" rather than "(void *) foo"

Hmm.. I don't know about (c), that one tends to vary by usage.

		Linus

Re: Lindent fixed to match reality

[Matt Mackall]

On Thu, Jan 29, 2004 at 11:44:27AM -0800, Linus Torvalds wrote:
> 
> 
> On Thu, 29 Jan 2004, Matt Mackall wrote:
> > 
> > a) (no -psl)
> > 
> > void *foo(void) {
> > 
> >  instead of
> > 
> > void *
> > foo(void) {
> 
> And why not 
> 
> 	void *foo(void)
> 	{
> 
> which is the _right_ thing to use?

Doh, of course the above is what it actually does.
 
> > b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> > c) (-ncs) "(void *)foo" rather than "(void *) foo"
> 
> Hmm.. I don't know about (c), that one tends to vary by usage.

I did a bit of visual grep for counterinstances of c) in core code but
nothing jumped out at me. I'm pretty sure the former is more common
practice and I at least find it helpful visually given the precedence
of cast operators. Thing is, indent feels obliged to force it one way
or the other, so I think it should err on the no space side.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: Lindent fixed to match reality

[David Weinehall]

On Thu, Jan 29, 2004 at 01:37:28PM -0600, Matt Mackall wrote:
> I've been fiddling with cleaning up some old code here and suggest the
> following to make Lindent match actual practice more closely. This does:
> 
> a) (no -psl)
> 
> void *foo(void) {
> 
>  instead of
> 
> void *
> foo(void) {
> 
> b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"

I can't really see the logic in this, though I know a lot of people do
it.  I try to stay consistent, thus I do:

if ()
for ()
case ()
while ()
sizeof ()
typeof ()

since they're all parts of the language, rather than
functions/macros or invocations of such.

[snip]

Of course, coding-style is religion, and religion as a topic is a
sure-fire way to turn every civil conversation into full out battle,
so I've begun building a bomb-shelter where I'm going to spend the next
few months...


Regards: David Weinehall
-- 
 /) David Weinehall <tao@acc.umu.se> /) Northern lights wander      (\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\)  http://www.acc.umu.se/~tao/    (/   Full colour fire           (/

Re: Lindent fixed to match reality

[Måns Rullgård]

David Weinehall <tao@acc.umu.se> writes:

>> b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
>
> I can't really see the logic in this, though I know a lot of people do
> it.  I try to stay consistent, thus I do:
>
> if ()
> for ()
> case ()
> while ()
> sizeof ()
> typeof ()
>
> since they're all parts of the language, rather than
> functions/macros or invocations of such.

What I fail to see here is why that should make a difference regarding
whitespace before the parens.

-- 
Måns Rullgård
mru@kth.se

Re: Lindent fixed to match reality

[David Weinehall]

On Thu, Jan 29, 2004 at 09:35:03PM +0100, Måns Rullgård wrote:
> David Weinehall <tao@acc.umu.se> writes:
> 
> >> b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> >
> > I can't really see the logic in this, though I know a lot of people do
> > it.  I try to stay consistent, thus I do:
> >
> > if ()
> > for ()
> > case ()
> > while ()
> > sizeof ()
> > typeof ()
> >
> > since they're all parts of the language, rather than
> > functions/macros or invocations of such.
> 
> What I fail to see here is why that should make a difference regarding
> whitespace before the parens.

All I'm trying to say, is that we should be consistent; most code
has:

if (), for (), case (), while ()

(and possibly sizeof foo, typeof foo)

but

sizeof(foo), typeof(foo)

which is what I dislike (consistancy is good.)


Regards: David Weinehall
-- 
 /) David Weinehall <tao@acc.umu.se> /) Northern lights wander      (\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\)  http://www.acc.umu.se/~tao/    (/   Full colour fire           (/

Re: Lindent fixed to match reality

[Måns Rullgård]

David Weinehall <tao@acc.umu.se> writes:

> On Thu, Jan 29, 2004 at 09:35:03PM +0100, Måns Rullgård wrote:
>> David Weinehall <tao@acc.umu.se> writes:
>> 
>> >> b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
>> >
>> > I can't really see the logic in this, though I know a lot of people do
>> > it.  I try to stay consistent, thus I do:
>> >
>> > if ()
>> > for ()
>> > case ()
>> > while ()
>> > sizeof ()
>> > typeof ()
>> >
>> > since they're all parts of the language, rather than
>> > functions/macros or invocations of such.
>> 
>> What I fail to see here is why that should make a difference regarding
>> whitespace before the parens.
>
> All I'm trying to say, is that we should be consistent; most code
> has:
>
> if (), for (), case (), while ()
>
> (and possibly sizeof foo, typeof foo)
>
> but
>
> sizeof(foo), typeof(foo)
>
> which is what I dislike (consistancy is good.)

Well, those have function-like semantics in that they have a value,
unlike an if statement.  That could possibly explain the difference.
I know perfectly well that sizeof is not a function, so don't bother
explaining that.

-- 
Måns Rullgård
mru@kth.se

Re: Lindent fixed to match reality

[Matt Mackall]

On Thu, Jan 29, 2004 at 09:15:56PM +0100, David Weinehall wrote:
> On Thu, Jan 29, 2004 at 01:37:28PM -0600, Matt Mackall wrote:
> > I've been fiddling with cleaning up some old code here and suggest the
> > following to make Lindent match actual practice more closely. This does:
> > 
> > a) (no -psl)
> > 
> > void *foo(void) {
> > 
> >  instead of
> > 
> > void *
> > foo(void) {
> > 
> > b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> 
> I can't really see the logic in this, though I know a lot of people do
> it.  I try to stay consistent, thus I do:
> 
> if ()
> for ()
> case ()
> while ()
> sizeof ()
> typeof ()

Well, sizeof and typeof are operators rather than flow structures so
they're a little different. But what I'm really trying to do here is
make Lindent match actual practice (as a first approximation of the
community's preferred practice) rather than argue the merits of any
particular usage.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: Lindent fixed to match reality

[Andries Brouwer]

On Thu, Jan 29, 2004 at 09:15:56PM +0100, David Weinehall wrote:

> > b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> 
> I can't really see the logic in this, though I know a lot of people do
> it.  I try to stay consistent, thus I do:
> 
> if ()
> for ()
> case ()
> while ()
> sizeof ()
> typeof ()
> 
> since they're all parts of the language, rather than
> functions/macros or invocations of such.

As you say, this is religion. Secondly, there need not be any logic.
But thirdly, if you insist: The first four are about flow of control.
We all agree they have spaces - it is Linux kernel standard.

On the other hand, sizeof is an arithmetical expression, often part
of larger expressions. Now expressions like
	sizeof (*foo)+1
might be confusing, and
	sizeof(*foo) + 1
shows more clearly what the parsing is.

Andries

Re: Lindent fixed to match reality

[David Weinehall]

On Thu, Jan 29, 2004 at 11:37:30PM +0100, Andries Brouwer wrote:
> On Thu, Jan 29, 2004 at 09:15:56PM +0100, David Weinehall wrote:
> 
> > > b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> > 
> > I can't really see the logic in this, though I know a lot of people do
> > it.  I try to stay consistent, thus I do:
> > 
> > if ()
> > for ()
> > case ()
> > while ()
> > sizeof ()
> > typeof ()
> > 
> > since they're all parts of the language, rather than
> > functions/macros or invocations of such.
> 
> As you say, this is religion. Secondly, there need not be any logic.
> But thirdly, if you insist: The first four are about flow of control.
> We all agree they have spaces - it is Linux kernel standard.
> 
> On the other hand, sizeof is an arithmetical expression, often part
> of larger expressions. Now expressions like
> 	sizeof (*foo)+1
> might be confusing, and
> 	sizeof(*foo) + 1
> shows more clearly what the parsing is.

You should at least compare apples to apples, that is:

sizeof (*foo) + 1

vs

sizeof(*foo) + 1

But I guess that was just a typo?  Of course, since the ()'s are useless
here anyway, and doesn't really bring any added bonus, we end up with

sizeof *foo + 1

vs

sizeof*foo + 1

and I'd say the latter looks rather confusing, if not for anything else
because

sizeoffoo

would be invalid code, while

sizeof foo

is perfectly valid.

This is the same as

return *foo;

vs

return*foo;

I personally regard the former to be preferable, but it's it's a
preference, not a something I'd die over.


Regards: David
-- 
 /) David Weinehall <tao@acc.umu.se> /) Northern lights wander      (\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\)  http://www.acc.umu.se/~tao/    (/   Full colour fire           (/

Re: Lindent fixed to match reality

[Tim Hockin]

Ahh, religion.

On Thu, Jan 29, 2004 at 11:54:56PM +0100, David Weinehall wrote:

> But I guess that was just a typo?  Of course, since the ()'s are useless
> here anyway, and doesn't really bring any added bonus, we end up with
> 
> sizeof *foo + 1
> 
> vs
> 
> sizeof*foo + 1

No, you're building a straw man.  Everyone knows that you should always use
the parens on sizeof().  Just because you CAN leave them out SOMETIMES does
not mean you SHOULD.

> sizeoffoo
> 
> would be invalid code, while
> 
> sizeof foo
> 
> is perfectly valid.

ooh, look at that straw man burn!

Re: Lindent fixed to match reality

[David Weinehall]

On Thu, Jan 29, 2004 at 03:17:24PM -0800, Tim Hockin wrote:

[snip]
> No, you're building a straw man.  Everyone knows that you should always use
> the parens on sizeof().  Just because you CAN leave them out SOMETIMES does
> not mean you SHOULD.

"Everyone" also sprinkles far too many parenthesis for their own code,
just to be sure.  I've seen code such as

a = b * c + 1;

written as

a = ((b * c) + 1);
 
The question is rather, why should you insert superfluous parenthesis
when they do no semantic difference, and do not improve readability in
any way?  If you get paid by the byte, then sure, but I don't, so I
won't...

[snip]

As mentioned earlier, if mister divine Pee-sprinkler decides that the
CodingStyle for 2.6 should be without the space for sizeof/typeof, then
I'll follow the leader when/if sending patches to the 2.6 kernel.
2.0 will still have the spaces in place, though.


Regards: David Weinehall
-- 
 /) David Weinehall <tao@acc.umu.se> /) Northern lights wander      (\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\)  http://www.acc.umu.se/~tao/    (/   Full colour fire           (/

Re: Lindent fixed to match reality

[Theodore Ts'o]

On Fri, Jan 30, 2004 at 12:43:36AM +0100, David Weinehall wrote:
> "Everyone" also sprinkles far too many parenthesis for their own code,
> just to be sure.  I've seen code such as
> 
> a = b * c + 1;
> 
> written as
> 
> a = ((b * c) + 1);
>  
> The question is rather, why should you insert superfluous parenthesis
> when they do no semantic difference, and do not improve readability in
> any way?  

I disagree; sometimes adding a few extra parenthesis *does* improve
readability, especially if the expression is complex.  Of course, if
it's that complex, you might be better off defining a few extra
variables and having named sub-expressions (it shouldn't make a
difference to a good compiler).

						- Ted

Re: Lindent fixed to match reality

[David Weinehall]

On Fri, Jan 30, 2004 at 09:44:42AM -0500, Theodore Ts'o wrote:
> On Fri, Jan 30, 2004 at 12:43:36AM +0100, David Weinehall wrote:
> > "Everyone" also sprinkles far too many parenthesis for their own code,
> > just to be sure.  I've seen code such as
> > 
> > a = b * c + 1;
> > 
> > written as
> > 
> > a = ((b * c) + 1);
> >  
> > The question is rather, why should you insert superfluous parenthesis
> > when they do no semantic difference, and do not improve readability in
> > any way?  
> 
> I disagree; sometimes adding a few extra parenthesis *does* improve
> readability, especially if the expression is complex.  Of course, if
> it's that complex, you might be better off defining a few extra
> variables and having named sub-expressions (it shouldn't make a
> difference to a good compiler).

Possibly "when they do" should've been "if they do" to clarify what I
meant.  I don't mean that dropping all superfluous parantheses are
necessarily good, but most of the time, people tend to sprinkle the code
with them just because they lack in their knowledge of basic
C-programming.  I'm not saying this is the case for the kernel, just
that I've seen it far too often.

I agree about the sub-expression bit though.


Regards: David Weinehall
-- 
 /) David Weinehall <tao@acc.umu.se> /) Northern lights wander      (\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\)  http://www.acc.umu.se/~tao/    (/   Full colour fire           (/

Re: Lindent fixed to match reality

[Eric W. Biederman]

David Weinehall <tao@acc.umu.se> writes:

> On Thu, Jan 29, 2004 at 01:37:28PM -0600, Matt Mackall wrote:
> > I've been fiddling with cleaning up some old code here and suggest the
> > following to make Lindent match actual practice more closely. This does:
> > 
> > a) (no -psl)
> > 
> > void *foo(void) {
> > 
> >  instead of
> > 
> > void *
> > foo(void) {
> > 
> > b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> 
> I can't really see the logic in this, though I know a lot of people do
> it.  I try to stay consistent, thus I do:

If consistency was good in a language we would all be using
an RPL or s-expr based language.  Communication is clearer
with redundant information.  Making special cases of common
cases is a good thing.

Eric

Re: Lindent fixed to match reality

[Roland Dreier]

    Matt> b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
    Matt> c) (-ncs) "(void *)foo" rather than "(void *) foo"

I disagree about both b) and c).  In particular, you would always
write "sizeof variable" (since "sizeofvariable" wouldn't even compile)
and there's no sense in fooling people into thinking sizeof is a
function.  Similarly I like the space after the () operator (although
you could argue we don't put space after a unary - operator).

 - Roland

Re: Lindent fixed to match reality

[Erik Hensema]

Matt Mackall (mpm@selenic.com) wrote:
> I've been fiddling with cleaning up some old code here and suggest the
> following to make Lindent match actual practice more closely. This does:
> 
> a) (no -psl)
> 
> void *foo(void) {
> 
>  instead of
> 
> void *
> foo(void) {

You just nicely broke 'find . -name *.c | xargs grep ^foo'.

Why make functions harder to find? It's just one line... Being
able to navigate the source tree with standard unix utils is a
Good Thing.

Even better, IMHO:

void *
foo(void)
{
}

Yes, that takes a full three lines. But within the function body
you can just reverse search for ^{ and you're at the function
declaration. Not nearly as useful as grepping for a function
name, but still a nice thing to have, IMHO.

> 
> b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"

Agreed.

> c) (-ncs) "(void *)foo" rather than "(void *) foo"

Agreed.


-- 
Erik Hensema <erik@hensema.net>

Re: Lindent fixed to match reality

[Krzysztof Halasa]

Erik Hensema <erik@hensema.net> writes:

>> void *foo(void) {
>> 
>>  instead of
>> 
>> void *
>> foo(void) {
>
> You just nicely broke 'find . -name *.c | xargs grep ^foo'.

It was never working with the kernel, so no one can break it.
Just use a little better pattern or use a tool which parses C code.
-- 
Krzysztof Halasa, B*FH

Re: Lindent fixed to match reality

[Linus Torvalds]

On Thu, 29 Jan 2004, Krzysztof Halasa wrote:
> Erik Hensema <erik@hensema.net> writes:
> 
> >> void *foo(void) {
> >> 
> >>  instead of
> >> 
> >> void *
> >> foo(void) {
> >
> > You just nicely broke 'find . -name *.c | xargs grep ^foo'.
> 
> It was never working with the kernel, so no one can break it.
> Just use a little better pattern or use a tool which parses C code.

Indeed.

Never _ever_ make your source-code look worse because your tools suck. Fix 
the tools instead.

There are tons of tools that index sources properly, so please don't try
to make code look like crap because of using broken things like '^foo'.

		Linus

Re: Lindent fixed to match reality

[Jan-Benedict Glaw]

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

On Thu, 2004-01-29 20:37:07 +0000, Erik Hensema <erik@hensema.net>
wrote in message <slrnc1irnj.2is.erik@bender.home.hensema.net>:
> Matt Mackall (mpm@selenic.com) wrote:

> You just nicely broke 'find . -name *.c | xargs grep ^foo'.

That's what I like, too:)

> Why make functions harder to find? It's just one line... Being
> able to navigate the source tree with standard unix utils is a
> Good Thing.
> 
> Even better, IMHO:
> 
> void *
> foo(void)

...which may even help you preparing header files' declarations if you
did a long shot of coding without adding the declarations in time!

> Yes, that takes a full three lines. But within the function body
> you can just reverse search for ^{ and you're at the function
> declaration. Not nearly as useful as grepping for a function
> name, but still a nice thing to have, IMHO.

It's quite useful!

> > b) (no -bs) "sizeof(foo)" rather than "sizeof (foo)"
> Agreed.

I prefer sizeof with a space, but that's pure religion...

> > c) (-ncs) "(void *)foo" rather than "(void *) foo"
> Agreed.

...as well as with casts.

That's all sooo much religion. Let's keep coding and not argue about
indention. Just keep *consistend* within a single file...

MfG, JBG

-- 
   Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481
   "Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg
    fuer einen Freien Staat voll Freier Bürger" | im Internet! |   im Irak!
   ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));

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