All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
@ 2010-01-27 13:09 Stefani Seibold
  2010-01-27 18:07 ` Joel Schopp
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Stefani Seibold @ 2010-01-27 13:09 UTC (permalink / raw)
  To: linux-kernel; +Cc: Andrew Morton, apw, jschopp, davej

The time of 80 characters punch card and terminals are over, so i would
be a good thing to set the line length limit to 120. Every display today
should be able handle this. 
  
Signed-off-by: Stefani Seibold <stefani@seibold.net>
---
 checkpatch.pl |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- linux-2.6.33-rc2.orig/scripts/checkpatch.pl	2009-12-03 04:51:21.000000000 +0100
+++ linux-2.6.33-rc2.new/scripts/checkpatch.pl	2010-01-06 17:46:40.057565661 +0100
@@ -1374,13 +1374,13 @@ sub process {
 # check we are in a valid source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
 
-#80 column limit
+#120 column limit
 		if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
 		    $rawline !~ /^.\s*\*\s*\@$Ident\s/ &&
 		    $line !~ /^\+\s*printk\s*\(\s*(?:KERN_\S+\s*)?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ &&
-		    $length > 80)
+		    $length > 120)
 		{
-			WARN("line over 80 characters\n" . $herecurr);
+			WARN("line over 120 characters\n" . $herecurr);
 		}
 
 # check for adding lines without a newline.






^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-27 13:09 [PATCH] checkpatch.pl: remove the 80 charactes punch card limit Stefani Seibold
@ 2010-01-27 18:07 ` Joel Schopp
  2010-01-27 19:01   ` David Daney
  2010-01-28 15:54 ` Andi Kleen
  2010-01-28 16:01 ` Krzysztof Halasa
  2 siblings, 1 reply; 13+ messages in thread
From: Joel Schopp @ 2010-01-27 18:07 UTC (permalink / raw)
  To: Stefani Seibold; +Cc: linux-kernel, Andrew Morton, apw, davej


> The time of 80 characters punch card and terminals are over, so i would
> be a good thing to set the line length limit to 120. Every display today
> should be able handle this. 
>   
Nack.

While the origins of 80 character lines dates back to punchcards there 
is a reason it has survived the test of time.  Lines that go longer are 
hard to comprehend.  Either they are long themselves, in which case 
breaking them up into smaller chunks on multiple lines helps 
readability, or they are starting from deep indentation, in which case 
the function should be refactored or broken up so the logic is more 
digestable. 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-27 18:07 ` Joel Schopp
@ 2010-01-27 19:01   ` David Daney
  2010-01-27 20:31     ` Joel Schopp
  0 siblings, 1 reply; 13+ messages in thread
From: David Daney @ 2010-01-27 19:01 UTC (permalink / raw)
  To: Joel Schopp; +Cc: Stefani Seibold, linux-kernel, Andrew Morton, apw, davej

Joel Schopp wrote:
> 
>> The time of 80 characters punch card and terminals are over, so i would
>> be a good thing to set the line length limit to 120. Every display today
>> should be able handle this.   
> Nack.
> 
> While the origins of 80 character lines dates back to punchcards there 
> is a reason it has survived the test of time.

Has it though?  If that were the undisputed truth, we wouldn't be having 
this discussion.  Also it is likely that there would be very few devices 
capable of displaying more than 80 columns.

> Lines that go longer are hard to comprehend.

Not universally.

> Either they are long themselves, in which case 
> breaking them up into smaller chunks on multiple lines helps 
> readability,

... Or sometimes it results in gibberish.

> or they are starting from deep indentation, in which case 
> the function should be refactored or broken up so the logic is more 
> digestable. --

The problem with the checkpatch.pl tool is that its use results in 
people trying to eliminate warnings.  In the case of the 80 column 
warning, this can result in going against the goal stated in CodingStyle 
Chapter 2:  "Coding style is all about readability and maintainability..."

Perhaps checkpatch.pl needs a third level of diagnostic.  Perhaps:

NOTICE: line over 80 characters

Indicating that the line in question should be given extra attention, 
but weaker than a WARNING.

In any event, it is always fun to discuss these questions of style.


David Daney


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-27 19:01   ` David Daney
@ 2010-01-27 20:31     ` Joel Schopp
  2010-01-27 20:42       ` David Daney
  2010-01-27 21:36       ` Stefani Seibold
  0 siblings, 2 replies; 13+ messages in thread
From: Joel Schopp @ 2010-01-27 20:31 UTC (permalink / raw)
  To: David Daney; +Cc: Stefani Seibold, linux-kernel, Andrew Morton, apw, davej


>>
>>
>> While the origins of 80 character lines dates back to punchcards 
>> there is a reason it has survived the test of time.
>
> Has it though?  If that were the undisputed truth, we wouldn't be 
> having this discussion.

If you know of a usability study that quantifies the effect of line 
length on readibility of C code I'm willing to listen, and I'm sure 
others are too.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-27 20:31     ` Joel Schopp
@ 2010-01-27 20:42       ` David Daney
  2010-01-27 21:36       ` Stefani Seibold
  1 sibling, 0 replies; 13+ messages in thread
From: David Daney @ 2010-01-27 20:42 UTC (permalink / raw)
  To: Joel Schopp; +Cc: Stefani Seibold, linux-kernel, Andrew Morton, apw, davej

Joel Schopp wrote:
> 
>>>
>>>
>>> While the origins of 80 character lines dates back to punchcards 
>>> there is a reason it has survived the test of time.
>>
>> Has it though?  If that were the undisputed truth, we wouldn't be 
>> having this discussion.
> 
> If you know of a usability study that quantifies the effect of line 
> length on readibility of C code I'm willing to listen, and I'm sure 
> others are too.


Good point.  As with most things related to kernel development, a 
usability study or other market research from a reputable institution is 
a vital first step before taking any action.

We don't have any good peer reviewed research on the subject that I am 
aware of.  I guess even contemplating a change at this early point would 
be rash and dangerous.

I withdraw my previous comments with respect to the 80 Column Question. 
  Instead I would recommend elevating the WARNING to ERROR status.


David Daney

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-27 20:31     ` Joel Schopp
  2010-01-27 20:42       ` David Daney
@ 2010-01-27 21:36       ` Stefani Seibold
  1 sibling, 0 replies; 13+ messages in thread
From: Stefani Seibold @ 2010-01-27 21:36 UTC (permalink / raw)
  To: Joel Schopp; +Cc: David Daney, linux-kernel, Andrew Morton, apw, davej

Am Mittwoch, den 27.01.2010, 14:31 -0600 schrieb Joel Schopp:
> >>
> >>
> >> While the origins of 80 character lines dates back to punchcards 
> >> there is a reason it has survived the test of time.
> >
> > Has it though?  If that were the undisputed truth, we wouldn't be 
> > having this discussion.
> 
> If you know of a usability study that quantifies the effect of line 
> length on readibility of C code I'm willing to listen, and I'm sure 
> others are too.

Show me the usability study with claims that 80 columns is the wisdom in
software engineering. Why not 73, 90 or 95? The only reason for the 80
columns is a historic one. 

And the programming rules for linux doesn't manifest the 80 character
per line.

Code will get in many cases harder to read, especially together with the
tab size of 8. Multiline C statements makes the code IMHO harder to
read.

Stefani



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-27 13:09 [PATCH] checkpatch.pl: remove the 80 charactes punch card limit Stefani Seibold
  2010-01-27 18:07 ` Joel Schopp
@ 2010-01-28 15:54 ` Andi Kleen
  2010-01-28 16:39   ` Alexander Clouter
  2010-01-28 17:58   ` David Daney
  2010-01-28 16:01 ` Krzysztof Halasa
  2 siblings, 2 replies; 13+ messages in thread
From: Andi Kleen @ 2010-01-28 15:54 UTC (permalink / raw)
  To: Stefani Seibold; +Cc: linux-kernel, Andrew Morton, apw, jschopp, davej

Stefani Seibold <stefani@seibold.net> writes:

> The time of 80 characters punch card and terminals are over, so i would
> be a good thing to set the line length limit to 120. Every display today
> should be able handle this. 
>   
> Signed-off-by: Stefani Seibold <stefani@seibold.net>

Full Ack!

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-27 13:09 [PATCH] checkpatch.pl: remove the 80 charactes punch card limit Stefani Seibold
  2010-01-27 18:07 ` Joel Schopp
  2010-01-28 15:54 ` Andi Kleen
@ 2010-01-28 16:01 ` Krzysztof Halasa
  2 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Halasa @ 2010-01-28 16:01 UTC (permalink / raw)
  To: Stefani Seibold; +Cc: linux-kernel, Andrew Morton, apw, jschopp, davej

Stefani Seibold <stefani@seibold.net> writes:

> The time of 80 characters punch card and terminals are over, so i would
> be a good thing to set the line length limit to 120. Every display today
> should be able handle this. 

I thought it's been already agreed that the limit is lifted altogether?
Not even the 132 that was considered by some people sane enough.

The other thing is that fine "code complexity", but I think checkpatch
can't check it ATM, can it?
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-28 15:54 ` Andi Kleen
@ 2010-01-28 16:39   ` Alexander Clouter
  2010-01-29  5:19     ` tytso
  2010-02-01 10:53     ` Simon Farnsworth
  2010-01-28 17:58   ` David Daney
  1 sibling, 2 replies; 13+ messages in thread
From: Alexander Clouter @ 2010-01-28 16:39 UTC (permalink / raw)
  To: linux-kernel

Andi Kleen <andi@firstfloor.org> wrote:
> 
>> The time of 80 characters punch card and terminals are over, so i would
>> be a good thing to set the line length limit to 120. Every display today
>> should be able handle this. 
>>   
>> Signed-off-by: Stefani Seibold <stefani@seibold.net>
> 
> Full Ack!
> 
NACK Attack!

There is a reason Knuth only put so many words on a line with TeX.

Eugh.

Cheers

-- 
Alexander Clouter
.sigmonster says: Keep out of reach of children.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-28 15:54 ` Andi Kleen
  2010-01-28 16:39   ` Alexander Clouter
@ 2010-01-28 17:58   ` David Daney
  1 sibling, 0 replies; 13+ messages in thread
From: David Daney @ 2010-01-28 17:58 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Stefani Seibold, linux-kernel, Andrew Morton, apw, jschopp, davej

Andi Kleen wrote:
> Stefani Seibold <stefani@seibold.net> writes:
> 
>> The time of 80 characters punch card and terminals are over, so i would
>> be a good thing to set the line length limit to 120. Every display today
>> should be able handle this. 
>>   
>> Signed-off-by: Stefani Seibold <stefani@seibold.net>
> 
> Full Ack!
> 
> -Andi


I agree, but if people still have problems with it, perhaps reevaluating 
Joe Perches' patch:

http://lkml.org/lkml/2009/12/18/3

could be an alternative.

David Daney

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-28 16:39   ` Alexander Clouter
@ 2010-01-29  5:19     ` tytso
  2010-01-29 18:49       ` Joe Perches
  2010-02-01 10:53     ` Simon Farnsworth
  1 sibling, 1 reply; 13+ messages in thread
From: tytso @ 2010-01-29  5:19 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: linux-kernel

On Thu, Jan 28, 2010 at 04:39:29PM +0000, Alexander Clouter wrote:
> Andi Kleen <andi@firstfloor.org> wrote:
> > 
> >> The time of 80 characters punch card and terminals are over, so i would
> >> be a good thing to set the line length limit to 120. Every display today
> >> should be able handle this. 
> >>   
> >> Signed-off-by: Stefani Seibold <stefani@seibold.net>
> > 
> > Full Ack!
> > 
> NACK Attack!
> 
> There is a reason Knuth only put so many words on a line with TeX.

Linus has already agreed that the 80 character limit is stupid.  Let's
just nuke this check from checkpatch.pl and move on.

     	       	     	  		    - Ted

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-29  5:19     ` tytso
@ 2010-01-29 18:49       ` Joe Perches
  0 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2010-01-29 18:49 UTC (permalink / raw)
  To: tytso, Andy Whitcroft, Andrew Morton; +Cc: Alexander Clouter, linux-kernel

On Fri, 2010-01-29 at 00:19 -0500, tytso@mit.edu wrote:
> Linus has already agreed that the 80 character limit is stupid.  Let's
> just nuke this check from checkpatch.pl and move on.

Andy, can you update checkpatch please?

I proposed this: http://lkml.org/lkml/2009/12/18/3

If you're too busy, I can collect the several
additional patches that have been suggested and
push them to you or to Andrew Morton.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] checkpatch.pl: remove the 80 charactes punch card limit
  2010-01-28 16:39   ` Alexander Clouter
  2010-01-29  5:19     ` tytso
@ 2010-02-01 10:53     ` Simon Farnsworth
  1 sibling, 0 replies; 13+ messages in thread
From: Simon Farnsworth @ 2010-02-01 10:53 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: linux-kernel

Alexander Clouter wrote:
> Andi Kleen <andi@firstfloor.org> wrote:
>>> The time of 80 characters punch card and terminals are over, so i would
>>> be a good thing to set the line length limit to 120. Every display today
>>> should be able handle this. 
>>>   
>>> Signed-off-by: Stefani Seibold <stefani@seibold.net>
>> Full Ack!
>>
> NACK Attack!
> 
> There is a reason Knuth only put so many words on a line with TeX.
> 
If you're going to cite Knuth as a reason for the 80 character per line
limit, you should understand the difference between ribbon length and
line length.

While there are good reasons to constrain the number of characters in a
ribbon, and to prohibit too much of a shift in indentation between two
neighbouring lines, there's no particularly strong readability reason to
limit a line to 80 characters.

Thus, assuming that maintainers read the patches they're sent, and apply
a bit of common sense (insisting on sensibly sized ribbons, and getting
grouchy about deep indentation), an 80 character line length limit isn't
needed.

-- 
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2010-02-01 11:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-27 13:09 [PATCH] checkpatch.pl: remove the 80 charactes punch card limit Stefani Seibold
2010-01-27 18:07 ` Joel Schopp
2010-01-27 19:01   ` David Daney
2010-01-27 20:31     ` Joel Schopp
2010-01-27 20:42       ` David Daney
2010-01-27 21:36       ` Stefani Seibold
2010-01-28 15:54 ` Andi Kleen
2010-01-28 16:39   ` Alexander Clouter
2010-01-29  5:19     ` tytso
2010-01-29 18:49       ` Joe Perches
2010-02-01 10:53     ` Simon Farnsworth
2010-01-28 17:58   ` David Daney
2010-01-28 16:01 ` Krzysztof Halasa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.