mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: kevin granade <kevin.granade@gmail.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Krzysztof Halasa <khc@pm.waw.pl>,
	Valdis.Kletnieks@vt.edu, Paul Mundt <lethal@linux-sh.org>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Alasdair G Kergon <agk@redhat.com>,
	dm-devel@redhat.com
Subject: Re: [PATCH] Drop 80-character limit in checkpatch.pl
Date: Fri, 18 Dec 2009 15:15:03 -0600	[thread overview]
Message-ID: <7004b08e0912181315n1894ca90m8752b57708cf55eb@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0912181137140.23738@hs20-bc2-1.build.redhat.com>

On Fri, Dec 18, 2009 at 10:43 AM, Mikulas Patocka <mpatocka@redhat.com> wrote:
>> Note: I'm not specifically arguing for keeping the 80-column rule, the
>> project I work on uses 100 columns, and that's quite workable, but I
>> haven't had any problem working with 80 columns as a limit either.  I
>> do however think that just removing the limit without replacing it
>> with something better is a bad idea.
>>
>> -Kevin Granade
>
> But think what happens when someone views that 100-char code on 80-char
> terminal (or for example 94-char, that I used for some times too) ---
> every second line will be wasted with just 20 characters on the left. On
> the other hand, if you have unlimited line length, it will look better on
> 80-char terminal.

1. I think that is why the limit is at 80 characters, so it's at the
lowest common denominator of screen and will not wrap at all.
(actually wasn't the original limit an email mangling issue?)
2. Aside from choosing the specific limit of 80, I don't think line
wrapping is a major goal of the line length limitation, as I said
before it is a heuristic that is indicative of BAD CODE.  The wrapping
issue is somewhat incidental.
3. I don't trust automated line-wrapping to provide readable code
anyway.  People I think will be much better at doing this, for
example, a common issue is with formatted string functions like scanf.
 A person will (ok, should) know to preferentially wrap after the
format string, and if they are really being nice can generalize about
other things, like name-value pairs:

scanf("%d,%d;%d,%d;%d,%d",
         key1, value1,
         key2, value2,
         key3, value3);

whereas automated wrapping will only know about a limited number of
rules, generally purely syntax-based: (yes, this is somewhat
contrived, but I think it illustrates the kind of limitation I'm
talking about.)

scanf("%d,%d;%d,%d;%d,%d", key1, value1, key2,
         value2, key3, value3);

But this is all really beside the point, if you really want to you can
write a text editor that will unwrap the text for you, and then
re-wrap it exactly how you want it.  I don't think this would be all
that much harder than one that could intelligently wrap the lines in
the first place.

The point I keep coming back to is to follow the *important* coding
guidelines, like avoiding excessive nesting and properly factoring
your code into sub-modules (macros, functions, inlined functions, etc.
as appropriate.)  The line limit acts as a warning that you are
probably doing something wrong, just shortening the line in question
is not the answer.

Other incidental issues:
"wasting lines" - Don't care, factor properly and a single unit of
code should fit in a screenfull or two.
"effort wasted manually wrapping lines" - Also don't care, if you
think the text editor should be smart enough to wrap lines
intelligently, just use it to do it for you.  (hint: they're generally
not actually that smart).  Also consider the alternative there, if
multi-hundred character lines are the norm, just how easy are the
contents of those lines to manipulate?

>
> Mikulas
>

  parent reply	other threads:[~2009-12-18 21:15 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-15 21:57 Mikulas Patocka
2009-12-15 22:26 ` Bartlomiej Zolnierkiewicz
2009-12-17  9:31   ` Américo Wang
2009-12-17 15:14     ` Linus Torvalds
2009-12-17 15:18       ` Bartlomiej Zolnierkiewicz
2009-12-17 15:37         ` Linus Torvalds
2009-12-17 16:08           ` Bartlomiej Zolnierkiewicz
2009-12-17 16:21             ` Linus Torvalds
2009-12-17 16:30               ` Janakiram Sistla
2009-12-17 18:05               ` Andi Kleen
2009-12-18 13:31               ` Pádraig Brady
2009-12-18 16:32               ` Mikulas Patocka
2009-12-18 22:33                 ` Krzysztof Halasa
2009-12-18 13:04         ` Jiri Kosina
2009-12-18 13:55           ` Bartlomiej Zolnierkiewicz
2009-12-18 14:39             ` Krzysztof Halasa
2009-12-27 17:15             ` Jon Smirl
2009-12-21  6:32           ` Paul Mundt
2009-12-22 15:10             ` Jiri Kosina
2009-12-16 10:58 ` Andi Kleen
2009-12-16 19:59 ` Alex Chiang
2009-12-17  6:12 ` Paul Mundt
2009-12-17  8:34   ` Krzysztof Halasa
2009-12-17 23:29     ` Mikulas Patocka
2009-12-17 23:35       ` Al Viro
2009-12-18  4:29       ` Valdis.Kletnieks
2009-12-18  5:12         ` [PATCH] scripts/checkpatch.pl: Change long line warning to 105 chars Joe Perches
2009-12-18  5:57           ` Paul Mundt
2009-12-18 17:43             ` Linus Torvalds
2009-12-18 17:54               ` Joe Perches
2009-12-18 18:41               ` Andi Kleen
2009-12-18 14:37           ` Krzysztof Halasa
2009-12-18 15:12             ` [dm-devel] " Alasdair G Kergon
2009-12-18 16:58               ` Randy Dunlap
2009-12-18 17:12                 ` Mikulas Patocka
2009-12-18 22:36                   ` Krzysztof Halasa
2009-12-18 17:31             ` Joe Perches
2009-12-18 14:28         ` [PATCH] Drop 80-character limit in checkpatch.pl Krzysztof Halasa
2009-12-18 14:52           ` kevin granade
2009-12-18 16:43             ` Mikulas Patocka
2009-12-18 16:50               ` Linus Torvalds
2009-12-18 17:09                 ` Mikulas Patocka
2009-12-18 17:28                   ` Linus Torvalds
2009-12-18 21:15               ` kevin granade [this message]
2009-12-18 15:11           ` Bartlomiej Zolnierkiewicz
2009-12-17 22:37   ` Mikulas Patocka
2009-12-17 23:12     ` Paul Mundt
2009-12-17 23:33       ` Mikulas Patocka

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=7004b08e0912181315n1894ca90m8752b57708cf55eb@mail.gmail.com \
    --to=kevin.granade@gmail.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=khc@pm.waw.pl \
    --cc=lethal@linux-sh.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=torvalds@linux-foundation.org \
    /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®