mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: David Newall <davidn@davidnewall.com>
Cc: Pavel Machek <pavel@ucw.cz>, Krzysztof Halasa <khc@pm.waw.pl>,
	Jeff Garzik <jeff@garzik.org>, Adrian Bunk <bunk@kernel.org>,
	Roland Dreier <rdreier@cisco.com>,
	Glenn Streiff <gstreiff@NetEffect.com>,
	Faisal Latif <flatif@NetEffect.com>,
	linux-kernel@vger.kernel.org, general@lists.openfabrics.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Greg Kroah-Hartman <greg@kroah.com>
Subject: Re: Merging of completely unreviewed drivers
Date: Sat, 23 Feb 2008 09:33:16 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.1.00.0802230913380.21332@woody.linux-foundation.org> (raw)
In-Reply-To: <47C013E2.6040201@davidnewall.com>



On Sat, 23 Feb 2008, David Newall wrote:
> 
> Do you actually get 80 columns wide on it?

Do people really care that deeply?

I still sometimes use small terminal windows - for a while I had my 
default terminal come up as 100x40, but I'm back to the standard 80x24, 
and while I often resize them, I certainly don't always.

And do I find lines longer than 80 charactes unreadable? Hell no.

Quite frankly, on a 80x24 display, I'll take long lines over split-up ones 
*any* day. For things like doing "git grep xyzzy", I'd *much* rather get 
the occasional long line that wraps (or, if I'm in "less -S", that I have 
to press right-arrow to see), than see just a meaningless fragment because 
somebody decided that they should always fit in 80 characters.

So *consistently* long lines are the problem, not the occasional one. The 
occasional one is likely more readable as it is, than split up.

Here's an example from code that actually looks pretty good in general:

	static unsigned long
	calc_delta_mine(unsigned long delta_exec, unsigned long weight,
	                struct load_weight *lw)

and look around that function in general: it's doesn't match the coding 
standard, but also compare the output of

	git grep calc_delta_mine

with the output of something like

	git grep update_load_sub

which actually shows you what the calling convention is.

So putting that long function definition on one line would make it a
108-character line or somethign like that, but it would have advantages
too.  It would have advantages for anything that is line-based (I use
grep for *everything*, but maybe I'm just odd), but it would also
actually be more readable for the people who have bigger windows.

But my point is, some of those advantages remain even with small
terminals, and quite often the downsides aren't even all that huge. 
Most editors wrap or chop the line according to your preferences (mine
are personally to chop), and if it's a fairly uncommon thing, those
downsides shrink further. 

Is 108 characters perhaps *too* long? In the above case it probably is,
since the downside of splitting the patch is pretty small (it's a static
function, only used in that file, the "grep" argument is weak, yadda
yadda).  But I'm just saying that it's not 100% obvious *even*if* you're
on a 80x24 terminal, and in some other cases the downside of splitting
the line can be much bigger (strings or more spread-out function calls
and declarations etc). 

The line length problem would probably be better attacked as something
more akin to the rule

 - do a rolling window of <n> last non-empty lines (n ~ 15 or so)

 - if more than <m> of those lines were longer than 72 charactes,
   somethign is wrong (m ~ 5 or so). 

which talks more about what matters - too deep indentation. And also 
attacks the problem that is really relevant: it's that kind of code that 
ends up being unreadable because so *much* of it is cut off or wrapped.

			Linus


  parent reply	other threads:[~2008-02-23 17:34 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-19 22:59 [2.6 patch] infiniband/hw/nes/nes_verbs.c: fix off-by-one Adrian Bunk
2008-02-20  4:23 ` [ofa-general] " Roland Dreier
2008-02-20  5:57   ` Adrian Bunk
2008-02-20 23:21     ` Roland Dreier
2008-02-20 23:27       ` Glenn Streiff
2008-02-21 12:39         ` Glenn Streiff
2008-02-21 15:49           ` Adrian Bunk
2008-02-21 20:28             ` Roland Dreier
2008-02-21 21:01               ` Merging of completely unreviewed drivers Adrian Bunk
2008-02-21 21:09                 ` Roland Dreier
2008-02-21 21:14                 ` Linus Torvalds
2008-02-21 22:33                   ` Alexey Dobriyan
2008-02-21 22:43                     ` Greg KH
2008-02-21 22:57                       ` Jeff Garzik
2008-02-21 22:58                       ` Alexey Dobriyan
2008-02-21 23:31                       ` Jan Engelhardt
2008-02-21 23:38                   ` Krzysztof Halasa
2008-02-21 23:31                     ` Alan Cox
2008-02-22  0:29                       ` Adrian Bunk
2008-02-21 23:41                     ` Jeff Garzik
2008-02-22  0:05                       ` Krzysztof Halasa
2008-02-22  0:44                         ` Jeff Garzik
2008-02-22  2:02                           ` Krzysztof Halasa
2008-02-22 10:04                         ` Alan Cox
2008-02-22 18:45                         ` Pavel Machek
2008-02-22 22:44                           ` Krzysztof Halasa
2008-02-23  9:43                             ` Pavel Machek
2008-02-23 12:38                               ` David Newall
2008-02-23 15:25                                 ` Pavel Machek
2008-02-24  3:18                                   ` David Newall
2008-02-23 17:33                                 ` Linus Torvalds [this message]
2008-02-24  3:26                                   ` David Newall
2008-02-24  4:47                                     ` Linus Torvalds
2008-02-23 13:58                               ` Krzysztof Halasa
2008-02-22  1:46                     ` David Newall
2008-02-22  2:06                       ` Al Viro
2008-02-22  2:23                         ` Krzysztof Halasa
2008-02-22  3:13                           ` Al Viro
2008-02-22 22:28                             ` Krzysztof Halasa
2008-02-24  7:47                               ` Jörn Engel
2008-02-24 14:47                                 ` Krzysztof Halasa
2008-02-22  3:13                         ` Linus Torvalds
2008-02-22  6:29                           ` [ofa-general] " Junio C Hamano
2008-02-22  9:02                             ` Adrian Bunk
2008-02-22  6:37                           ` Ray Lee
2008-02-23 15:31                             ` Jan Engelhardt
2008-02-24  3:22                               ` David Newall
2008-02-22 22:37                           ` Krzysztof Halasa
2008-02-22 12:29                       ` [ofa-general] " Bart Van Assche
2008-02-22 14:25                         ` David Newall
2008-02-22 15:17                           ` Peter Zijlstra
2008-02-22 16:48                             ` John W. Linville
2008-02-22 22:59                             ` Krzysztof Halasa
2008-02-22 23:14                               ` Al Viro
2008-02-22 15:48                           ` John W. Linville
2008-02-22 18:54                   ` Ingo Molnar
2008-02-22 19:11                     ` [ofa-general] " Bart Van Assche
2008-02-22 19:20                     ` Jeff Garzik
2008-02-22 19:44                       ` Greg KH
2008-02-21 21:30                 ` Greg KH
2008-02-22  1:06                   ` Adrian Bunk
2008-02-21 22:08                 ` Arjan van de Ven
2008-02-21 22:33                   ` Jeff Garzik
2008-02-21 23:40                     ` Adrian Bunk
2008-02-22 18:40                   ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.1.00.0802230913380.21332@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=bunk@kernel.org \
    --cc=davidn@davidnewall.com \
    --cc=flatif@NetEffect.com \
    --cc=general@lists.openfabrics.org \
    --cc=greg@kroah.com \
    --cc=gstreiff@NetEffect.com \
    --cc=jeff@garzik.org \
    --cc=khc@pm.waw.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=rdreier@cisco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®