[PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[Joe Perches]

Commit messages are sometimes overly long.
Suggest line wrapping at 72 columns.

Add a checkpatch test for long commit messages as well.

Signed-off-by: Joe Perches <joe@perches.com>
---
On Tue, 2015-03-31 at 11:16 -0700, Joe Perches wrote:
> On Tue, 2015-03-31 at 13:53 -0400, David Miller wrote:
> > From: Ian Morris <ipm@chirality.org.uk>
> > > The following patches address some coding style issues only. No
> > > functional changes and no changes detected by objdiff.
> > >
> > > The IPV6 code uses multiple different styles when comparing with
> > > NULL (I.e. x == NULL and !x as well as x != NULL and x). Generally
> > > the latter form is preferred according to checkpatch and so this
> > > changes aligns the code to this style.
> > 
> > Please trim your commit message lines to 80 columns in the future.
> []
> > The only reliable way to edit and format commit log messages, so that
> > everyone sees something consistent, is to do so in a pure plain ASCII
> > text editor and use newlines to break columns at ~80 columns.
> 
> I think ~72 is the preferred # as by default git commit log output
> is indented a few chars.

 Documentation/SubmittingPatches | 4 ++--
 scripts/checkpatch.pl           | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 447671b..c9de1c3 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -614,8 +614,8 @@ The canonical patch message body contains the following:
 
   - An empty line.
 
-  - The body of the explanation, which will be copied to the
-    permanent changelog to describe this patch.
+  - The body of the explanation, line wrapped at 72 columns, which will
+    be copied to the permanent changelog to describe this patch.
 
   - The "Signed-off-by:" lines, described above, which will
     also go in the changelog.
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d54a814..62a7be5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1898,6 +1898,7 @@ sub process {
 
 	my $in_header_lines = $file ? 0 : 1;
 	my $in_commit_log = 0;		#Scanning lines before patch
+	my $commit_log_long_line = 0;
 	my $reported_maintainer_file = 0;
 	my $non_utf8_charset = 0;
 
@@ -2233,6 +2234,14 @@ sub process {
 			      "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr);
 		}
 
+# Check for line lengths > 72 in commit log, warn once
+		if ($in_commit_log && !$commit_log_long_line &&
+		    length($line) > 72) {
+			WARN("COMMIT_LOG_LONG_LINE",
+			     "Possible unwrapped commit description (prefer maximum 72 chars per line)\n" . $herecurr);
+			$commit_log_long_line = 1;
+		}
+
 # Check for git id commit length and improperly formed commit descriptions
 		if ($in_commit_log && $line =~ /\b(c)ommit\s+([0-9a-f]{5,})/i) {
 			my $init_char = $1;

Re: [PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Tue, 31 Mar 2015 11:40:25 -0700

> Commit messages are sometimes overly long.
> Suggest line wrapping at 72 columns.
> 
> Add a checkpatch test for long commit messages as well.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[Andrew Morton]

On Tue, 31 Mar 2015 11:40:25 -0700 Joe Perches <joe@perches.com> wrote:

> Commit messages are sometimes overly long.
> Suggest line wrapping at 72 columns.

waah, I use 75 and this patch has ruined my life.

Is there some way I can disable just this check without patching
checkpatch?  I suspect a lot of people will be interested in doing
this ;)

Re: [PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[Joe Perches]

On Wed, 2015-04-01 at 12:20 -0700, Andrew Morton wrote:
> On Tue, 31 Mar 2015 11:40:25 -0700 Joe Perches <joe@perches.com> wrote:
> 
> > Commit messages are sometimes overly long.
> > Suggest line wrapping at 72 columns.
> 
> waah, I use 75 and this patch has ruined my life.

hmm... 1/4/15 or is it 4/1/15?

> Is there some way I can disable just this check without patching
> checkpatch?  I suspect a lot of people will be interested in doing
> this ;)

You could s/72/75/.  I won't care.

Using "./scripts/checkpatch.pl --ignore=commit_log_long_line"
would work too.

Re: [PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[Jonathan Corbet]

On Wed, 1 Apr 2015 12:20:01 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> waah, I use 75 and this patch has ruined my life.
> 
> Is there some way I can disable just this check without patching
> checkpatch?  I suspect a lot of people will be interested in doing
> this ;)

I use 75 too, FWIW.

It seems to me that nobody out there complains about 75-character lines
in commit logs; it's the 120-character lines that irritate people.  I
would suggest that this limit could be set to 75 (or even, say, 77) and
still achieve its real purpose while leaving far fewer ruined lives in its
wake. 

jon

Re: [PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[Joe Perches]

On Wed, 2015-04-01 at 21:36 +0200, Jonathan Corbet wrote:
> On Wed, 1 Apr 2015 12:20:01 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> > waah, I use 75 and this patch has ruined my life.

crybaby... :)

> I use 75 too, FWIW.
> 
> It seems to me that nobody out there complains about 75-character lines
> in commit logs; it's the 120-character lines that irritate people.  I
> would suggest that this limit could be set to 75 (or even, say, 77)

77 might be one too many.

git log has a default 4 space indent of the commit message so
75 or 76 would still fit an 80 column screen.

> and
> still achieve its real purpose while leaving far fewer ruined lives in its
> wake. 

I don't wanna be held liable for that ruination, thanks.

Re: [PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns

[Andrew Morton]

On Wed, 01 Apr 2015 13:09:29 -0700 Joe Perches <joe@perches.com> wrote:

> On Wed, 2015-04-01 at 21:36 +0200, Jonathan Corbet wrote:
> > On Wed, 1 Apr 2015 12:20:01 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > > waah, I use 75 and this patch has ruined my life.
> 
> crybaby... :)

I'm very sensitive.

> > I use 75 too, FWIW.
> > 
> > It seems to me that nobody out there complains about 75-character lines
> > in commit logs; it's the 120-character lines that irritate people.  I
> > would suggest that this limit could be set to 75 (or even, say, 77)
> 
> 77 might be one too many.
> 
> git log has a default 4 space indent of the commit message so
> 75 or 76 would still fit an 80 column screen.

That's a good point.  Good enough to be in a changelog ;)

76 sounds a bit risky - I don't think I trust every output device to
dtrt with 80-column text.  I might be wrong about that.  75 would save
my handkerchiefs.

[PATCH V2] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 75 columns

[Joe Perches]

Commit messages lines are sometimes overly long.

Suggest line wrapping at 75 columns so the default git
commit log indentation of 4 plus the commit message text
still fits on an 80 column screen.

Add a checkpatch test for long commit messages lines too.

Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: David Miller <davem@davmloft.net>
---

V2: avoid Andrew Morton's colicky crying binges...

On Wed, 2015-04-01 at 13:18 -0700, Andrew Morton wrote:
> On Wed, 01 Apr 2015 13:09:29 -0700 Joe Perches <joe@perches.com> wrote:
> I'm very sensitive.

A poet too no doubt.

> > git log has a default 4 space indent of the commit message so
> > 75 or 76 would still fit an 80 column screen.
> 
> That's a good point.  Good enough to be in a changelog ;)

If you're going to change it, make sure to DTRT.

Or just take this one...

 Documentation/SubmittingPatches | 4 ++--
 scripts/checkpatch.pl           | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 447671b..c9de1c3 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -614,8 +614,8 @@ The canonical patch message body contains the following:
 
   - An empty line.
 
-  - The body of the explanation, which will be copied to the
-    permanent changelog to describe this patch.
+  - The body of the explanation, line wrapped at 75 columns, which will
+    be copied to the permanent changelog to describe this patch.
 
   - The "Signed-off-by:" lines, described above, which will
     also go in the changelog.
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d54a814..62a7be5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1898,6 +1898,7 @@ sub process {
 
 	my $in_header_lines = $file ? 0 : 1;
 	my $in_commit_log = 0;		#Scanning lines before patch
+	my $commit_log_long_line = 0;
 	my $reported_maintainer_file = 0;
 	my $non_utf8_charset = 0;
 
@@ -2233,6 +2234,14 @@ sub process {
 			      "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr);
 		}
 
+# Check for line lengths > 75 in commit log, warn once
+		if ($in_commit_log && !$commit_log_long_line &&
+		    length($line) > 75) {
+			WARN("COMMIT_LOG_LONG_LINE",
+			     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
+			$commit_log_long_line = 1;
+		}
+
 # Check for git id commit length and improperly formed commit descriptions
 		if ($in_commit_log && $line =~ /\b(c)ommit\s+([0-9a-f]{5,})/i) {
 			my $init_char = $1;