[PATCH] checkpatch: Make the output better readable

[PATCH] checkpatch: Make the output better readable

[Petr Mladek]

I always have troubles to parse checkpatch.pl output when I check
the whole patchset. It is hard to say which messages belongs to
what patch.

This patch does few small changes to make the output look better
for me:

    + delimit each patch from each other with dashes and empty line
    + remove empty line after the summary
    + print message about false positives only once

Output without this patch:
==========================

total: 0 errors, 0 warnings, 133 lines checked

0015-ring_buffer-Use-iterant-kthreads-API-in-the-ring-buf.patch has no obvious style problems and is ready for submission.
total: 0 errors, 0 warnings, 73 lines checked

0016-ring_buffer-Allow-to-cleanly-freeze-the-ring-buffer-.patch has no obvious style problems and is ready for submission.
total: 0 errors, 0 warnings, 25 lines checked

0017-ring_buffer-Allow-to-exit-the-ring-buffer-benchmark-.patch has no obvious style problems and is ready for submission.
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 70 lines checked

0018-kthread-Add-support-for-iteruptible-sleep-with-timeo.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 34 lines checked

0019-kthread-Allow-to-remove-pause-between-threads-in-ite.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 12 lines checked

0020-ring_buffer-Use-the-new-API-for-timeouted-sleep-in-t.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 37 lines checked

0021-jffs2-debug-messages.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 28 lines checked

0022-test-messages-in-ring_buffer_benchmnark.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 7 lines checked

0023-debug-messages-in-lockd.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

Output with this patch:
=======================

total: 0 errors, 0 warnings, 133 lines checked
0015-ring_buffer-Use-iterant-kthreads-API-in-the-ring-buf.patch has no obvious style problems and is ready for submission.
--------------------------------------------------------------------------------

total: 0 errors, 0 warnings, 73 lines checked
0016-ring_buffer-Allow-to-cleanly-freeze-the-ring-buffer-.patch has no obvious style problems and is ready for submission.
--------------------------------------------------------------------------------

total: 0 errors, 0 warnings, 25 lines checked
0017-ring_buffer-Allow-to-exit-the-ring-buffer-benchmark-.patch has no obvious style problems and is ready for submission.
--------------------------------------------------------------------------------

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 70 lines checked
0018-kthread-Add-support-for-iteruptible-sleep-with-timeo.patch has style problems, please review.
--------------------------------------------------------------------------------

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 34 lines checked
0019-kthread-Allow-to-remove-pause-between-threads-in-ite.patch has style problems, please review.
--------------------------------------------------------------------------------

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 12 lines checked
0020-ring_buffer-Use-the-new-API-for-timeouted-sleep-in-t.patch has style problems, please review.
--------------------------------------------------------------------------------

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 37 lines checked
0021-jffs2-debug-messages.patch has style problems, please review.
--------------------------------------------------------------------------------

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 28 lines checked
0022-test-messages-in-ring_buffer_benchmnark.patch has style problems, please review.
--------------------------------------------------------------------------------

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 7 lines checked
0023-debug-messages-in-lockd.patch has style problems, please review.

If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 scripts/checkpatch.pl | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c8032a01d7cf..7022138b14cb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -720,8 +720,14 @@ my @fixed_deleted = ();
 my $fixlinenr = -1;
 
 my $vname;
+my $filenum = 0;
 for my $filename (@ARGV) {
 	my $FILE;
+
+	if ($filenum++ && $quiet == 0) {
+		print "--------------------------------------------------------------------------------\n";
+		print "\n";
+	}
 	if ($file) {
 		open($FILE, '-|', "diff -u /dev/null $filename") ||
 			die "$P: $filename: diff failed - $!\n";
@@ -755,6 +761,14 @@ for my $filename (@ARGV) {
 	build_types();
 }
 
+if ($exit && $quiet == 0) {
+	print << "EOM";
+
+If any of the errors are false positives, please report
+them to the maintainer, see CHECKPATCH in MAINTAINERS.
+EOM
+}
+
 exit($exit);
 
 sub top_of_kernel_tree {
@@ -5578,7 +5592,6 @@ sub process {
 		print "total: $cnt_error errors, $cnt_warn warnings, " .
 			(($check)? "$cnt_chk checks, " : "") .
 			"$cnt_lines lines checked\n";
-		print "\n" if ($quiet == 0);
 	}
 
 	if ($quiet == 0) {
@@ -5643,12 +5656,7 @@ EOM
 		print "$vname has no obvious style problems and is ready for submission.\n"
 	}
 	if ($clean == 0 && $quiet == 0) {
-		print << "EOM";
-$vname has style problems, please review.
-
-If any of these errors are false positives, please report
-them to the maintainer, see CHECKPATCH in MAINTAINERS.
-EOM
+		print "$vname has style problems, please review.\n"
 	}
 
 	return $clean;
-- 
1.8.5.6

Re: [PATCH] checkpatch: Make the output better readable

[Joe Perches]

On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> I always have troubles to parse checkpatch.pl output when I check
> the whole patchset. It is hard to say which messages belongs to
> what patch.
> 
> This patch does few small changes to make the output look better
> for me:
> 
>     + delimit each patch from each other with dashes and empty line
>     + remove empty line after the summary

I've no objection about this, but don't much care either.

>     + print message about false positives only once

This bit seems sensible, thanks.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -720,8 +720,14 @@ my @fixed_deleted = ();
>  my $fixlinenr = -1;
>  
>  my $vname;
> +my $filenum = 0;
>  for my $filename (@ARGV) {
>  	my $FILE;
> +
> +	if ($filenum++ && $quiet == 0) {
> +		print "--------------------------------------------------------------------------------\n";

Perhaps more perlish would be print '-' x 81 . '\n\n';
Dunno why you chose 81 though, it seems an unusual number.

Re: [PATCH] checkpatch: Make the output better readable

[Petr Mladek]

On Mon 2015-06-01 08:50:24, Joe Perches wrote:
> On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> > I always have troubles to parse checkpatch.pl output when I check
> > the whole patchset. It is hard to say which messages belongs to
> > what patch.
> > 
> > This patch does few small changes to make the output look better
> > for me:
> > 
> >     + delimit each patch from each other with dashes and empty line
> >     + remove empty line after the summary
> 
> I've no objection about this, but don't much care either.
> 
> >     + print message about false positives only once
> 
> This bit seems sensible, thanks.
> 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -720,8 +720,14 @@ my @fixed_deleted = ();
> >  my $fixlinenr = -1;
> >  
> >  my $vname;
> > +my $filenum = 0;
> >  for my $filename (@ARGV) {
> >  	my $FILE;
> > +
> > +	if ($filenum++ && $quiet == 0) {
> > +		print "--------------------------------------------------------------------------------\n";
> 
> Perhaps more perlish would be print '-' x 81 . '\n\n';
> Dunno why you chose 81 though, it seems an unusual number.

Are you sure, please? I have just counted it again and I see 80
dashes. Is it possible that you counted the initial quotation
mark '"'?

Well, I do not mind about the number of dashes. Feel free to update
it in case you merge it.

Thanks a lot for review.

Best Regards,
Petr

Re: [PATCH] checkpatch: Make the output better readable

[Joe Perches]

On Tue, 2015-06-02 at 11:13 +0200, Petr Mladek wrote:
> On Mon 2015-06-01 08:50:24, Joe Perches wrote:
> > On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> > > I always have troubles to parse checkpatch.pl output when I check
> > > the whole patchset. It is hard to say which messages belongs to
> > > what patch.
> > > 
> > > This patch does few small changes to make the output look better
> > > for me:
> > > 
> > >     + delimit each patch from each other with dashes and empty line
> > >     + remove empty line after the summary
> > 
> > I've no objection about this, but don't much care either.
> > 
> > >     + print message about false positives only once
> > 
> > This bit seems sensible, thanks.
> > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -720,8 +720,14 @@ my @fixed_deleted = ();
> > >  my $fixlinenr = -1;
> > >  
> > >  my $vname;
> > > +my $filenum = 0;
> > >  for my $filename (@ARGV) {
> > >  	my $FILE;
> > > +
> > > +	if ($filenum++ && $quiet == 0) {
> > > +		print "--------------------------------------------------------------------------------\n";
> > 
> > Perhaps more perlish would be print '-' x 81 . '\n\n';
> > Dunno why you chose 81 though, it seems an unusual number.
> 
> Are you sure, please? I have just counted it again and I see 80
> dashes. Is it possible that you counted the initial quotation
> mark '"'?

My mistake, I neglected to account for the cr in echo|wc

> Well, I do not mind about the number of dashes. Feel free to update
> it in case you merge it.

I don't actually merge stuff, I can forward it to
Andrew Morton though, but perhaps it'd be better to
check $#ARGV > 1 and emit something like
	"$filename is being processed\n"
so that there is a delimiter before and after each file

Another option for you is to add --emacs on the command line.
That prefixes patch filename & location before each message.

Re: [PATCH] checkpatch: Make the output better readable

[Petr Mladek]

On Tue 2015-06-02 02:52:03, Joe Perches wrote:
> On Tue, 2015-06-02 at 11:13 +0200, Petr Mladek wrote:
> > On Mon 2015-06-01 08:50:24, Joe Perches wrote:
> > > On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> > > > I always have troubles to parse checkpatch.pl output when I check
> > > > the whole patchset. It is hard to say which messages belongs to
> > > > what patch.
> > > > 
> > > > This patch does few small changes to make the output look better
> > > > for me:
> > > > 
> > > >     + delimit each patch from each other with dashes and empty line
> > > >     + remove empty line after the summary
> > > 
> > > I've no objection about this, but don't much care either.
> > > 
> > > >     + print message about false positives only once
> > > 
> > > This bit seems sensible, thanks.
> > > 
> > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > []
> > > > @@ -720,8 +720,14 @@ my @fixed_deleted = ();
> > > >  my $fixlinenr = -1;
> > > >  
> > > >  my $vname;
> > > > +my $filenum = 0;
> > > >  for my $filename (@ARGV) {
> > > >  	my $FILE;
> > > > +
> > > > +	if ($filenum++ && $quiet == 0) {
> > > > +		print "--------------------------------------------------------------------------------\n";
> > > 
> > > Perhaps more perlish would be print '-' x 81 . '\n\n';
> > > Dunno why you chose 81 though, it seems an unusual number.
> > 
> > Are you sure, please? I have just counted it again and I see 80
> > dashes. Is it possible that you counted the initial quotation
> > mark '"'?
> 
> My mistake, I neglected to account for the cr in echo|wc
> 
> > Well, I do not mind about the number of dashes. Feel free to update
> > it in case you merge it.
> 
> I don't actually merge stuff, I can forward it to
> Andrew Morton though,

T.hat would be nice. Thanks in advance.

> but perhaps it'd be better to
> check $#ARGV > 1 and emit something like
> 	"$filename is being processed\n"
> so that there is a delimiter before and after each file

I personally do not like this idea much. It would create another
long line and kind of hide the warnings and errors. IMHO, the dashes
are better and enough. But I am not UI guy.

But feel free to improve it as you like.

> Another option for you is to add --emacs on the command line.
> That prefixes patch filename & location before each message.

Thanks for the hint. I was not aware of it. Well, it still looks
messy without my patch.

Best Regards,
Petr

Re: [PATCH] checkpatch: Make the output better readable

[Joe Perches]

On Tue, 2015-06-02 at 12:44 +0200, Petr Mladek wrote:
> On Tue 2015-06-02 02:52:03, Joe Perches wrote:
> > On Tue, 2015-06-02 at 11:13 +0200, Petr Mladek wrote:
> > > On Mon 2015-06-01 08:50:24, Joe Perches wrote:
> > > > On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> > > > > I always have troubles to parse checkpatch.pl output when I check
> > > > > the whole patchset. It is hard to say which messages belongs to
> > > > > what patch.
> > > > > 
> > > > > This patch does few small changes to make the output look better
> > > > > for me:
> > > > > 
> > > > >     + delimit each patch from each other with dashes and empty line
> > > > >     + remove empty line after the summary
> > > > 
> > > > I've no objection about this, but don't much care either.
> > > > 
> > > > >     + print message about false positives only once
> > > > 
> > > > This bit seems sensible, thanks.
> > > > 
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > []
> > > > > @@ -720,8 +720,14 @@ my @fixed_deleted = ();
> > > > >  my $fixlinenr = -1;
> > > > >  
> > > > >  my $vname;
> > > > > +my $filenum = 0;
> > > > >  for my $filename (@ARGV) {
> > > > >  	my $FILE;
> > > > > +
> > > > > +	if ($filenum++ && $quiet == 0) {
> > > > > +		print "--------------------------------------------------------------------------------\n";
> > > > 
> > > > Perhaps more perlish would be print '-' x 81 . '\n\n';
> > > > Dunno why you chose 81 though, it seems an unusual number.
> > > 
> > > Are you sure, please? I have just counted it again and I see 80
> > > dashes. Is it possible that you counted the initial quotation
> > > mark '"'?
> > 
> > My mistake, I neglected to account for the cr in echo|wc
> > 
> > > Well, I do not mind about the number of dashes. Feel free to update
> > > it in case you merge it.
> > 
> > I don't actually merge stuff, I can forward it to
> > Andrew Morton though,
> 
> That would be nice. Thanks in advance.
> 
> > but perhaps it'd be better to
> > check $#ARGV > 1 and emit something like
> > 	"$filename is being processed\n"
> > so that there is a delimiter before and after each file
> 
> I personally do not like this idea much. It would create another
> long line and kind of hide the warnings and errors. IMHO, the dashes
> are better and enough. But I am not UI guy.
> 
> But feel free to improve it as you like.
> 
> > Another option for you is to add --emacs on the command line.
> > That prefixes patch filename & location before each message.
> 
> Thanks for the hint. I was not aware of it. Well, it still looks
> messy without my patch.

Maybe this:

If there are multiple patches/files on the command line,
use a prefix before the patch/file message output like:
	--------------
	patch/filename
	--------------
to make the identifying which messages go with which
file/patch a bit easier to parse.

Move the perl version and false positive messages after
all the files have been scanned so that they are emitted
only once.

Standardize the NOTE: <...> form to always emit a blank
line before the NOTE and always use print << "EOM" style.
---
 scripts/checkpatch.pl | 62 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c8032a0..eaa76bd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -197,11 +197,11 @@ sub hash_show_words {
 	my ($hashRef, $prefix) = @_;
 
 	if ($quiet == 0 && keys %$hashRef) {
-		print "NOTE: $prefix message types:";
+		print "\nNOTE: $prefix message types:";
 		foreach my $word (sort keys %$hashRef) {
 			print " $word";
 		}
-		print "\n\n";
+		print "\n";
 	}
 }
 
@@ -741,6 +741,13 @@ for my $filename (@ARGV) {
 		push(@rawlines, $_);
 	}
 	close($FILE);
+
+	if ($#ARGV > 0 && $quiet == 0) {
+		print '-' x length($vname) . "\n";
+		print "$vname\n";
+		print '-' x length($vname) . "\n";
+	}
+
 	if (!process($filename)) {
 		$exit = 1;
 	}
@@ -755,6 +762,23 @@ for my $filename (@ARGV) {
 	build_types();
 }
 
+if (!$quiet) {
+	if ($^V lt 5.10.0) {
+		print << "EOM"
+
+NOTE: perl $^V is not modern enough to detect all possible issues.
+      An upgrade to at least perl v5.10.0 is suggested.
+EOM
+	}
+	if ($exit) {
+		print << "EOM"
+
+NOTE: If any of the errors are false positives, please report
+      them to the maintainer, see CHECKPATCH in MAINTAINERS.
+EOM
+	}
+}
+
 exit($exit);
 
 sub top_of_kernel_tree {
@@ -5578,22 +5602,18 @@ sub process {
 		print "total: $cnt_error errors, $cnt_warn warnings, " .
 			(($check)? "$cnt_chk checks, " : "") .
 			"$cnt_lines lines checked\n";
-		print "\n" if ($quiet == 0);
 	}
 
 	if ($quiet == 0) {
-
-		if ($^V lt 5.10.0) {
-			print("NOTE: perl $^V is not modern enough to detect all possible issues.\n");
-			print("An upgrade to at least perl v5.10.0 is suggested.\n\n");
-		}
-
 		# If there were whitespace errors which cleanpatch can fix
 		# then suggest that.
 		if ($rpt_cleaners) {
-			print "NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or\n";
-			print "      scripts/cleanfile\n\n";
 			$rpt_cleaners = 0;
+			print << "EOM"
+
+NOTE: Whitespace errors detected.
+      You may wish to use scripts/cleanpatch or scripts/cleanfile
+EOM
 		}
 	}
 
@@ -5627,6 +5647,7 @@ sub process {
 
 		if (!$quiet) {
 			print << "EOM";
+
 Wrote EXPERIMENTAL --fix correction(s) to '$newfile'
 
 Do _NOT_ trust the results written to this file.
@@ -5634,22 +5655,17 @@ Do _NOT_ submit these changes without inspecting them for correctness.
 
 This EXPERIMENTAL file is simply a convenience to help rewrite patches.
 No warranties, expressed or implied...
-
 EOM
 		}
 	}
 
-	if ($clean == 1 && $quiet == 0) {
-		print "$vname has no obvious style problems and is ready for submission.\n"
-	}
-	if ($clean == 0 && $quiet == 0) {
-		print << "EOM";
-$vname has style problems, please review.
-
-If any of these errors are false positives, please report
-them to the maintainer, see CHECKPATCH in MAINTAINERS.
-EOM
+	if ($quiet == 0) {
+		print "\n";
+		if ($clean == 1) {
+			print "$vname has no obvious style problems and is ready for submission.\n";
+		} else {
+			print "$vname has style problems, please review.\n";
+		}
 	}
-
 	return $clean;
 }

Re: [PATCH] checkpatch: Make the output better readable

[Joe Perches]

On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> I always have troubles to parse checkpatch.pl output when I check
> the whole patchset. It is hard to say which messages belongs to
> what patch.
> 
> This patch does few small changes to make the output look better
> for me:

As git and other utilities now use color by default, what do
you think about adding color for various message types?

And colorize only to the terminal, not any redirected output.

Maybe something like:

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c8032a0..12c43c6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -9,6 +9,7 @@ use strict;
 use POSIX;
 use File::Basename;
 use Cwd 'abs_path';
+use Term::ANSIColor qw(:constants);
 
 my $P = $0;
 my $D = dirname(abs_path($P));
@@ -1649,10 +1650,24 @@ sub report {
 		return 0;
 	}
 	my $line;
+	my $cprefix = $prefix;
+	my $clevel = $level;
+	my $ctype = $type;
+	if (-t STDOUT) {
+		$cprefix = GREEN . $prefix . RESET;
+		if ($level eq "ERROR") {
+			$clevel = RED . $level . RESET;
+		} elsif ($level eq "WARNING") {
+			$clevel = YELLOW . $level . RESET;
+		} else {
+			$clevel = GREEN . $level . RESET;
+		}
+		$ctype = GREEN . $type . RESET;
+	}
 	if ($show_types) {
-		$line = "$prefix$level:$type: $msg\n";
+		$line = "$cprefix$clevel:$ctype: $msg\n";
 	} else {
-		$line = "$prefix$level: $msg\n";
+		$line = "$cprefix$clevel: $msg\n";
 	}
 	$line = (split('\n', $line))[0] . "\n" if ($terse);
 

Re: [PATCH] checkpatch: Make the output better readable

[Joe Perches]

(adding back cc's)

On Mon, 2015-06-01 at 11:14 -0700, Jim Davis wrote:
> On Mon, Jun 1, 2015 at 11:02 AM, Joe Perches <joe@perches.com> wrote:
> > On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> >> I always have troubles to parse checkpatch.pl output when I check
> >> the whole patchset. It is hard to say which messages belongs to
> >> what patch.
> >>
> >> This patch does few small changes to make the output look better
> >> for me:
> >
> > As git and other utilities now use color by default, what do
> > you think about adding color for various message types?
> >
> > And colorize only to the terminal, not any redirected output.
> >
> > Maybe something like:
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index c8032a0..12c43c6 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -9,6 +9,7 @@ use strict;
> >  use POSIX;
> >  use File::Basename;
> >  use Cwd 'abs_path';
> > +use Term::ANSIColor qw(:constants);
> >
> >  my $P = $0;
> >  my $D = dirname(abs_path($P));
> > @@ -1649,10 +1650,24 @@ sub report {
> >                 return 0;
> >         }
> >         my $line;
> > +       my $cprefix = $prefix;
> > +       my $clevel = $level;
> > +       my $ctype = $type;
> > +       if (-t STDOUT) {
> > +               $cprefix = GREEN . $prefix . RESET;
> > +               if ($level eq "ERROR") {
> > +                       $clevel = RED . $level . RESET;
> > +               } elsif ($level eq "WARNING") {
> > +                       $clevel = YELLOW . $level . RESET;
> > +               } else {
> > +                       $clevel = GREEN . $level . RESET;
> > +               }
> > +               $ctype = GREEN . $type . RESET;
> > +       }
> 
> Could colors be optional?  Not everyone can distinguish green from
> red, and some of us who can still prefer being colorless.  (So to
> speak.)

Just as with git, piping though cat|less would eliminate colors,
I suppose a --nocolor option could be added easily enough too.

btw: it's not a patch, just a possibility/proposal.

Re: [PATCH] checkpatch: Make the output better readable

[Petr Mladek]

On Mon 2015-06-01 11:02:42, Joe Perches wrote:
> On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> > I always have troubles to parse checkpatch.pl output when I check
> > the whole patchset. It is hard to say which messages belongs to
> > what patch.
> > 
> > This patch does few small changes to make the output look better
> > for me:
> 
> As git and other utilities now use color by default, what do
> you think about adding color for various message types?
> 
> And colorize only to the terminal, not any redirected output.

JFYI, the output with this patch looks fine to me but to be honest
I do not mind much about it.

I sent a patch that helped me a lot with parsing. If it is accepted
in some form, it would be great. I am sure that more improvements are
possible.

But all this is matter of personal taste. I am afraid that this theme
is prone for bikeshedding and I do not want to get involved if
possible :-)

Best Regards,
Petr

> Maybe something like:
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index c8032a0..12c43c6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -9,6 +9,7 @@ use strict;
>  use POSIX;
>  use File::Basename;
>  use Cwd 'abs_path';
> +use Term::ANSIColor qw(:constants);
>  
>  my $P = $0;
>  my $D = dirname(abs_path($P));
> @@ -1649,10 +1650,24 @@ sub report {
>  		return 0;
>  	}
>  	my $line;
> +	my $cprefix = $prefix;
> +	my $clevel = $level;
> +	my $ctype = $type;
> +	if (-t STDOUT) {
> +		$cprefix = GREEN . $prefix . RESET;
> +		if ($level eq "ERROR") {
> +			$clevel = RED . $level . RESET;
> +		} elsif ($level eq "WARNING") {
> +			$clevel = YELLOW . $level . RESET;
> +		} else {
> +			$clevel = GREEN . $level . RESET;
> +		}
> +		$ctype = GREEN . $type . RESET;
> +	}
>  	if ($show_types) {
> -		$line = "$prefix$level:$type: $msg\n";
> +		$line = "$cprefix$clevel:$ctype: $msg\n";
>  	} else {
> -		$line = "$prefix$level: $msg\n";
> +		$line = "$cprefix$clevel: $msg\n";
>  	}
>  	$line = (split('\n', $line))[0] . "\n" if ($terse);
>  
> 
> 

Re: [PATCH] checkpatch: Make the output better readable

[Joe Perches]

On Tue, 2015-06-02 at 11:26 +0200, Petr Mladek wrote:
> On Mon 2015-06-01 11:02:42, Joe Perches wrote:
> > As git and other utilities now use color by default, what do
> > you think about adding color for various message types?
[]
> But all this is matter of personal taste. I am afraid that this theme
> is prone for bikeshedding and I do not want to get involved if
> possible :-)

Yeah, but you started it... :}

cheers, Joe

Re: [PATCH] checkpatch: Make the output better readable

[Rasmus Villemoes]

On Tue, Jun 02 2015, Petr Mladek <pmladek@suse.cz> wrote:

> I sent a patch that helped me a lot with parsing.

FWIW, me too. I've always been bothered by the empty line between the
statistics line and the summary, and the lack of vertical separation
before the output for the next file.

Rasmus