checkpatch: output style changes

PATCH In review Joe Perches 2 participants 11 messages Jun 3, 2015 – Jun 4, 2015 Checkpatch

Share & tools

Joe Perches

author maintainer

2015-06-03 15:53 UTC #0 lore raw

cover checkpatch: output style changes

Some think that checkpatch output is not easily parsed by people. Try some things to make it easier to visually find messages.

Joe Perches (3):
checkpatch: Improve output with multiple command-line files
checkpatch: colorize output to terminal
checkpatch: Add --showfile to allow input via pipe to show filenames

 scripts/checkpatch.pl | 111 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 77 insertions(+), 34 deletions(-)

Signature · 1 line

2.1.2

Joe Perches

author maintainer

2015-06-03 15:54 UTC #8 lore raw

PATCH 3/3 checkpatch: Add --showfile to allow input via pipe to show filenames

Using "git diff | ./scripts/checkpatch -" does not have an easy mechanism to see the files and lines actually modified.

Add --showfile to see the file and line specified in the diff.

When --showfile is used without --terse, the second line of each message output is redundant, so it is removed.

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

scripts/checkpatch.pl +17 -5

index cef2cd4..1241f99d 100755
@@ -25,6 +25,7 @@ my $chk_patch = 1;
 my $tst_only;
 my $emacs = 0;
 my $terse = 0;
+my $showfile = 0;
 my $file = 0;
 my $check = 0;
 my $check_orig = 0;

@@ -66,6 +67,7 @@ Options:
   --patch                    treat FILE as patchfile (default)
   --emacs                    emacs compile window format
   --terse                    one line per report
+  --showfile                 emit diffed file position, not input file position
   -f, --file                 treat FILE as regular source file
   --subjective, --strict     enable more subjective tests
   --types TYPE(,TYPE2...)    show only these comma separated message types

@@ -137,6 +139,7 @@ GetOptions(
 	'patch!'	=> \$chk_patch,
 	'emacs!'	=> \$emacs,
 	'terse!'	=> \$terse,
+	'showfile!'	=> \$showfile,
 	'f|file!'	=> \$file,
 	'subjective!'	=> \$check,
 	'strict!'	=> \$check,

@@ -1693,6 +1696,12 @@ sub report {
 	}
 	$output .= RESET if (-t STDOUT && $color);
 	$output .= ' ' . $msg . "\n";
+
+	if ($showfile) {
+		my @lines = split("\n", $output, -1);
+		splice(@lines, 1, 1);
+		$output = join("\n", @lines);
+	}
 	$output = (split('\n', $output))[0] . "\n" if ($terse);
 
 	push(our @report, $output);

@@ -2119,10 +2128,6 @@ sub process {
 
 		my $hunk_line = ($realcnt != 0);
 
-#make up the handle for any error we report on this line
-		$prefix = "$filename:$realline: " if ($emacs && $file);
-		$prefix = "$filename:$linenr: " if ($emacs && !$file);
-
 		$here = "#$linenr: " if (!$file);
 		$here = "#$realline: " if ($file);

@@ -2152,6 +2157,13 @@ sub process {
 			$found_file = 1;
 		}
 
+#make up the handle for any error we report on this line
+		if ($showfile) {
+			$prefix = "$realfile:$realline: "
+		} elsif ($emacs) {
+			$prefix = "$filename:$linenr: ";
+		}
+
 		if ($found_file) {
 			if ($realfile =~ m@^(drivers/net/|net/)@) {
 				$check = 1;

@@ -5606,7 +5618,7 @@ sub process {
 		ERROR("NOT_UNIFIED_DIFF",
 		      "Does not appear to be a unified-diff format patch\n");
 	}
-	if ($is_patch && $chk_signoff && $signoff == 0) {
+	if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) {
 		ERROR("MISSING_SIGN_OFF",
 		      "Missing Signed-off-by: line(s)\n");
 	}

Signature · 1 line

2.1.2

Joe Perches

author maintainer

2015-06-03 15:54 UTC #6 lore raw

PATCH 2/3 checkpatch: colorize output to terminal

In reply to Joe Perches

Add optional colors to make seeing message types a bit easier.

Add --color command line switch, default:on

Error is RED, warning is YELLOW, check is GREEN.
The message type, if shown, is BLUE.

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +21 -6

index eaa76bd..cef2cd4 100755
@@ -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));

@@ -49,6 +50,7 @@ my $min_conf_desc_length = 4;
 my $spelling_file = "$D/spelling.txt";
 my $codespell = 0;
 my $codespellfile = "/usr/local/share/codespell/dictionary.txt";
+my $color = 1;
 
 sub help {
 	my ($exitcode) = @_;

@@ -93,6 +95,7 @@ Options:
   --codespell                Use the codespell dictionary for spelling/typos
                              (default:/usr/local/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
+  --color                    Use colors when output is STDOUT (default: on)
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.

@@ -153,6 +156,7 @@ GetOptions(
 	'test-only=s'	=> \$tst_only,
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
+	'color!'	=> \$color,
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

@@ -1672,15 +1676,26 @@ sub report {
 	    (defined $tst_only && $msg !~ /\Q$tst_only\E/)) {
 		return 0;
 	}
-	my $line;
+	my $output = '';
+	if (-t STDOUT && $color) {
+		if ($level eq 'ERROR') {
+			$output .= RED;
+		} elsif ($level eq 'WARNING') {
+			$output .= YELLOW;
+		} else {
+			$output .= GREEN;
+		}
+	}
+	$output .= $prefix . $level . ':';
 	if ($show_types) {
-		$line = "$prefix$level:$type: $msg\n";
-	} else {
-		$line = "$prefix$level: $msg\n";
+		$output .= BLUE if (-t STDOUT && $color);
+		$output .= "$type:";
 	}
-	$line = (split('\n', $line))[0] . "\n" if ($terse);
+	$output .= RESET if (-t STDOUT && $color);
+	$output .= ' ' . $msg . "\n";
+	$output = (split('\n', $output))[0] . "\n" if ($terse);
 
-	push(our @report, $line);
+	push(our @report, $output);
 
 	return 1;
 }

Signature · 1 line

2.1.2

Joe Perches

author maintainer

2015-06-03 15:54 UTC #1 lore raw

PATCH 1/3 checkpatch: Improve output with multiple command-line files

In reply to Joe Perches

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.

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 62 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 23 deletions(-)

scripts/checkpatch.pl +39 ~~-23~~

index c8032a0..eaa76bd 100755
@@ -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;
 }

Signature · 1 line

2.1.2

Petr Mladek

Tested-by on v1 1/3

2015-06-04 12:03 UTC #2 lore raw

Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files

Quoted 12 lines · Joe Perches

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.

Quoted 5 lines

Standardize the NOTE: <...> form to always emit a blank line before the NOTE and always use print << "EOM" style.
Suggested-by Petr Mladek <[email protected]>
Signed-off-by Joe Perches <[email protected]>

Tested-by Petr Mladek <[email protected]>

One has to get used to the output but it is better readable, definitely.

Quoted 75 lines

---
 scripts/checkpatch.pl | 62 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 23 deletions(-)

scripts/checkpatch.pl +26 -9

index c8032a0..eaa76bd 100755
@@ -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

scripts/checkpatch.pl sub process { · line 5616

Quoted 10 lines

scripts/checkpatch.pl +5 -2

 		# 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

It would make sense to write this message only once as well.

Best Regards,
Petr

Quoted 44 lines

scripts/checkpatch.pl +8 ~~-12~~

 		}
 	}

@@ -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;
 }

Signature · 1 line

2.1.2

Petr Mladek

Tested-by on v1 2/3

2015-06-04 12:05 UTC #7 lore raw

Re: [PATCH 2/3] checkpatch: colorize output to terminal

In reply to Joe Perches

Quoted 8 lines · Joe Perches

Add optional colors to make seeing message types a bit easier.
Add --color command line switch, default:on
Error is RED, warning is YELLOW, check is GREEN.
The message type, if shown, is BLUE.
Signed-off-by Joe Perches <[email protected]>

Tested-by Petr Mladek <[email protected]>

I like it. It really helps noticing the problems.

Best Regards,
Petr

Quoted 76 lines

---
 scripts/checkpatch.pl | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +21 -6

index eaa76bd..cef2cd4 100755
@@ -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));

@@ -49,6 +50,7 @@ my $min_conf_desc_length = 4;
 my $spelling_file = "$D/spelling.txt";
 my $codespell = 0;
 my $codespellfile = "/usr/local/share/codespell/dictionary.txt";
+my $color = 1;
 
 sub help {
 	my ($exitcode) = @_;

@@ -93,6 +95,7 @@ Options:
   --codespell                Use the codespell dictionary for spelling/typos
                              (default:/usr/local/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
+  --color                    Use colors when output is STDOUT (default: on)
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.

@@ -153,6 +156,7 @@ GetOptions(
 	'test-only=s'	=> \$tst_only,
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
+	'color!'	=> \$color,
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

@@ -1672,15 +1676,26 @@ sub report {
 	    (defined $tst_only && $msg !~ /\Q$tst_only\E/)) {
 		return 0;
 	}
-	my $line;
+	my $output = '';
+	if (-t STDOUT && $color) {
+		if ($level eq 'ERROR') {
+			$output .= RED;
+		} elsif ($level eq 'WARNING') {
+			$output .= YELLOW;
+		} else {
+			$output .= GREEN;
+		}
+	}
+	$output .= $prefix . $level . ':';
 	if ($show_types) {
-		$line = "$prefix$level:$type: $msg\n";
-	} else {
-		$line = "$prefix$level: $msg\n";
+		$output .= BLUE if (-t STDOUT && $color);
+		$output .= "$type:";
 	}
-	$line = (split('\n', $line))[0] . "\n" if ($terse);
+	$output .= RESET if (-t STDOUT && $color);
+	$output .= ' ' . $msg . "\n";
+	$output = (split('\n', $output))[0] . "\n" if ($terse);
 
-	push(our @report, $line);
+	push(our @report, $output);
 
 	return 1;
 }

Signature · 1 line

2.1.2

Joe Perches

author maintainer

2015-06-04 12:14 UTC #3 lore raw

Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files

In reply to Petr Mladek

Quoted 8 lines · Petr Mladek

Quoted 7 lines · Joe Perches
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.

[]

Quoted 1 line

Tested-by Petr Mladek <[email protected]>

[]

Quoted 5 lines

scripts/checkpatch.pl sub process { · line 5616
Quoted 3 lines
scripts/checkpatch.pl +3 -0
+NOTE: Whitespace errors detected.
+      You may wish to use scripts/cleanpatch or scripts/cleanfile
+EOM
It would make sense to write this message only once as well.

I don't think so as it applies to each patch/file separately.

Petr Mladek

2015-06-04 12:18 UTC #9 lore raw

Re: [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames

In reply to Joe Perches

Quoted 9 lines · Joe Perches

Using "git diff | ./scripts/checkpatch -" does not have an easy mechanism to see the files and lines actually modified.
Add --showfile to see the file and line specified in the diff.
When --showfile is used without --terse, the second line of each message output is redundant, so it is removed.
Signed-off-by Joe Perches <[email protected]>

I like idea but there is a problem, see below.

Quoted 71 lines

---
 scripts/checkpatch.pl | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

scripts/checkpatch.pl +16 -4

index cef2cd4..1241f99d 100755
@@ -25,6 +25,7 @@ my $chk_patch = 1;
 my $tst_only;
 my $emacs = 0;
 my $terse = 0;
+my $showfile = 0;
 my $file = 0;
 my $check = 0;
 my $check_orig = 0;

@@ -66,6 +67,7 @@ Options:
   --patch                    treat FILE as patchfile (default)
   --emacs                    emacs compile window format
   --terse                    one line per report
+  --showfile                 emit diffed file position, not input file position
   -f, --file                 treat FILE as regular source file
   --subjective, --strict     enable more subjective tests
   --types TYPE(,TYPE2...)    show only these comma separated message types

@@ -137,6 +139,7 @@ GetOptions(
 	'patch!'	=> \$chk_patch,
 	'emacs!'	=> \$emacs,
 	'terse!'	=> \$terse,
+	'showfile!'	=> \$showfile,
 	'f|file!'	=> \$file,
 	'subjective!'	=> \$check,
 	'strict!'	=> \$check,

@@ -1693,6 +1696,12 @@ sub report {
 	}
 	$output .= RESET if (-t STDOUT && $color);
 	$output .= ' ' . $msg . "\n";
+
+	if ($showfile) {
+		my @lines = split("\n", $output, -1);
+		splice(@lines, 1, 1);
+		$output = join("\n", @lines);
+	}
 	$output = (split('\n', $output))[0] . "\n" if ($terse);
 
 	push(our @report, $output);

@@ -2119,10 +2128,6 @@ sub process {
 
 		my $hunk_line = ($realcnt != 0);
 
-#make up the handle for any error we report on this line
-		$prefix = "$filename:$realline: " if ($emacs && $file);
-		$prefix = "$filename:$linenr: " if ($emacs && !$file);
-
 		$here = "#$linenr: " if (!$file);
 		$here = "#$realline: " if ($file);

@@ -2152,6 +2157,13 @@ sub process {
 			$found_file = 1;
 		}
 
+#make up the handle for any error we report on this line
+		if ($showfile) {
+			$prefix = "$realfile:$realline: "
+		} elsif ($emacs) {
+			$prefix = "$filename:$linenr: ";
+		}
+
 		if ($found_file) {
 			if ($realfile =~ m@^(drivers/net/|net/)@) {
 				$check = 1;

scripts/checkpatch.pl sub process { · line 5621

Quoted 5 lines

scripts/checkpatch.pl +1 -1

@@ -5606,7 +5618,7 @@ sub process {
 		ERROR("NOT_UNIFIED_DIFF",
 		      "Does not appear to be a unified-diff format patch\n");
 	}
-	if ($is_patch && $chk_signoff && $signoff == 0) {
+	if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) {

You might use also "cat $patch | ./scripts/checkpatch -" and in this case you would want to print the warning.

It still prints invalid filename:linenum when you call:
./scripts/checkpatch 0001-my-commit.patch

A better solution would be to omit the filename:linenum information for this patch-specific messages or print the patchname instead of the filename.

Best Regards,
Petr

Quoted 6 lines

scripts/checkpatch.pl +0 -0

 		ERROR("MISSING_SIGN_OFF",
 		      "Missing Signed-off-by: line(s)\n");
 	}

Signature · 1 line

2.1.2

Petr Mladek

2015-06-04 12:29 UTC #4 lore raw

Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files

In reply to Joe Perches

Quoted 11 lines · Joe Perches

Quoted 8 lines · Petr Mladek
Quoted 7 lines · Joe Perches
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.
[]
Quoted 1 line
Tested-by Petr Mladek <[email protected]>

Quoted 8 lines

[]
Quoted 5 lines
scripts/checkpatch.pl sub process { · line 5616
Quoted 3 lines
scripts/checkpatch.pl +3 -0
+NOTE: Whitespace errors detected.
+      You may wish to use scripts/cleanpatch or scripts/cleanfile
+EOM
It would make sense to write this message only once as well.
I don't think so as it applies to each patch/file separately.

IMHO, it advertises scripts/cleanpatch or scripts/cleanfile. I think that we do not need to repeat it for each affected patch.

In fact, using scripts/cleanfile is questionable. IMHO, it does not make sense to fix indentation just because it looks better. It makes problems when backporting fixes. But it might make sense to fix it when you do some real change on that line.

Best Regards,
Petr

Joe Perches

author maintainer

2015-06-04 12:36 UTC #5 lore raw

Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files

Quoted 3 lines · Petr Mladek

Quoted 2 lines · Joe Perches
Quoted 1 line · Petr Mladek
On Wed 2015-06-03 08:53:38, Joe Perches wrote:

[]

Quoted 10 lines

Quoted 7 lines
Quoted 5 lines
scripts/checkpatch.pl sub process { · line 5616
Quoted 3 lines
scripts/checkpatch.pl +3 -0
+NOTE: Whitespace errors detected.
+      You may wish to use scripts/cleanpatch or scripts/cleanfile
+EOM
It would make sense to write this message only once as well.
I don't think so as it applies to each patch/file separately.
IMHO, it advertises scripts/cleanpatch or scripts/cleanfile. I think that we do not need to repeat it for each affected patch.

No worries, we just disagree.

Quoted 4 lines

In fact, using scripts/cleanfile is questionable. IMHO, it does not make sense to fix indentation just because it looks better. It makes problems when backporting fixes. But it might make sense to fix it when you do some real change on that line.

Whitespace at EOL backporting is trivially handled by git am.

cheers, Joe

Joe Perches

author maintainer

2015-06-04 12:40 UTC #10 lore raw

Re: [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames

In reply to Petr Mladek

Quoted 4 lines · Petr Mladek

Quoted 3 lines · Joe Perches
Using "git diff | ./scripts/checkpatch -" does not have an easy mechanism to see the files and lines actually modified.

Quoted 8 lines

Quoted 6 lines
Add --showfile to see the file and line specified in the diff.
When --showfile is used without --terse, the second line of each message output is redundant, so it is removed.
Signed-off-by Joe Perches <[email protected]>
I like idea but there is a problem, see below.

[]

Quoted 9 lines

scripts/checkpatch.pl sub process { · line 5621
Quoted 6 lines
scripts/checkpatch.pl +1 -1
@@ -5606,7 +5618,7 @@ sub process {
 		ERROR("NOT_UNIFIED_DIFF",
 		      "Does not appear to be a unified-diff format patch\n");
 	}
-	if ($is_patch && $chk_signoff && $signoff == 0) {
+	if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) {
You might use also "cat $patch | ./scripts/checkpatch -" and in this case you would want to print the warning.

Yeah, but I'm not straining for likely unusual use-cases.

Quoted 2 lines

It still prints invalid filename:linenum when you call:
./scripts/checkpatch 0001-my-commit.patch

I don't see this.
It prints the file/line of the patch without -terse just as before.

Quoted 3 lines

A better solution would be to omit the filename:linenum information for this patch-specific messages or print the patchname instead of the filename.

Anyway, I think the patch appropriate to apply
for now and I'll ponder and listen to suggestions
for improvement later.

cheers, Joe