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(-)
--
2.1.2
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.
Suggested-by: Petr Mladek <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
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;
}
--
2.1.2
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(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index eaa76bd..cef2cd4 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));
@@ -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;
}
--
2.1.2
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(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index cef2cd4..1241f99d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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");
}
--
2.1.2
On Wed 2015-06-03 08:53:38, Joe Perches wrote:
> 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.
>
> 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.
> ---
> 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
It would make sense to write this message only once as well.
Best Regards,
Petr
> }
> }
>
> @@ -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;
> }
> --
> 2.1.2
>
On Wed 2015-06-03 08:53:39, Joe Perches wrote:
> 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
> ---
> scripts/checkpatch.pl | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index eaa76bd..cef2cd4 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));
> @@ -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;
> }
> --
> 2.1.2
>
On Thu, 2015-06-04 at 14:03 +0200, Petr Mladek wrote:
> On Wed 2015-06-03 08:53:38, Joe Perches wrote:
> > 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.
[]
> Tested-by: Petr Mladek <[email protected]>
[]
> > +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.
On Wed 2015-06-03 08:53:40, Joe Perches wrote:
> 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.
> ---
> scripts/checkpatch.pl | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index cef2cd4..1241f99d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -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) {
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
> ERROR("MISSING_SIGN_OFF",
> "Missing Signed-off-by: line(s)\n");
> }
> --
> 2.1.2
>
On Thu 2015-06-04 05:14:39, Joe Perches wrote:
> On Thu, 2015-06-04 at 14:03 +0200, Petr Mladek wrote:
> > On Wed 2015-06-03 08:53:38, Joe Perches wrote:
> > > 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.
> []
> > Tested-by: Petr Mladek <[email protected]>
> []
> > > +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
On Thu, 2015-06-04 at 14:29 +0200, Petr Mladek wrote:
> On Thu 2015-06-04 05:14:39, Joe Perches wrote:
> > On Thu, 2015-06-04 at 14:03 +0200, Petr Mladek wrote:
> > > On Wed 2015-06-03 08:53:38, Joe Perches wrote:
[]
> > > > +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.
> 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
On Thu, 2015-06-04 at 14:18 +0200, Petr Mladek wrote:
> On Wed 2015-06-03 08:53:40, Joe Perches wrote:
> > 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.
[]
> > @@ -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.
> 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.
> 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