2004-01-29 19:37:40

by Matt Mackall

[permalink] [raw]
Subject: Lindent fixed to match reality

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


2004-01-29 20:00:34

by Matt Mackall

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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

2004-01-29 19:45:13

by Linus Torvalds

[permalink] [raw]
Subject: Re: Lindent fixed to match reality



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

2004-01-29 20:16:05

by David Weinehall

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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 <[email protected]> /) Northern lights wander (\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\) http://www.acc.umu.se/~tao/ (/ Full colour fire (/

2004-01-29 20:37:18

by Erik Hensema

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

Matt Mackall ([email protected]) 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 <[email protected]>

2004-01-29 20:35:21

by Måns Rullgård

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

David Weinehall <[email protected]> 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
[email protected]

2004-01-29 20:17:21

by Roland Dreier

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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

2004-01-29 20:52:36

by Måns Rullgård

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

David Weinehall <[email protected]> writes:

> On Thu, Jan 29, 2004 at 09:35:03PM +0100, M?ns Rullg?rd wrote:
>> David Weinehall <[email protected]> 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
[email protected]

2004-01-29 20:42:55

by David Weinehall

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

On Thu, Jan 29, 2004 at 09:35:03PM +0100, M?ns Rullg?rd wrote:
> David Weinehall <[email protected]> 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 <[email protected]> /) Northern lights wander (\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\) http://www.acc.umu.se/~tao/ (/ Full colour fire (/

2004-01-29 21:46:26

by Matt Mackall

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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

2004-01-29 22:38:17

by Andries Brouwer

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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

2004-01-29 22:55:21

by David Weinehall

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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 <[email protected]> /) Northern lights wander (\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\) http://www.acc.umu.se/~tao/ (/ Full colour fire (/

2004-01-29 23:18:27

by Tim Hockin

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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!

2004-01-29 23:43:42

by David Weinehall

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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 <[email protected]> /) Northern lights wander (\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\) http://www.acc.umu.se/~tao/ (/ Full colour fire (/

2004-01-30 10:38:57

by Jan-Benedict Glaw

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

On Thu, 2004-01-29 20:37:07 +0000, Erik Hensema <[email protected]>
wrote in message <[email protected]>:
> Matt Mackall ([email protected]) 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 [email protected] . +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));


Attachments:
(No filename) (1.48 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2004-01-30 14:39:51

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

Erik Hensema <[email protected]> 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

2004-01-30 14:48:12

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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

2004-01-30 16:37:04

by Linus Torvalds

[permalink] [raw]
Subject: Re: Lindent fixed to match reality



On Thu, 29 Jan 2004, Krzysztof Halasa wrote:
> Erik Hensema <[email protected]> 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

2004-01-30 17:49:29

by David Weinehall

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

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 <[email protected]> /) Northern lights wander (\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\) http://www.acc.umu.se/~tao/ (/ Full colour fire (/

2004-01-30 21:15:39

by Eric W. Biederman

[permalink] [raw]
Subject: Re: Lindent fixed to match reality

David Weinehall <[email protected]> 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