Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1157795 > unrolled thread
| Started by | Joe Perches <joe@perches.com> |
|---|---|
| First post | 2015-06-03 18:00 +0200 |
| Last post | 2015-06-04 14:20 +0200 |
| Articles | 8 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] checkpatch: output style changes Joe Perches <joe@perches.com> - 2015-06-03 18:00 +0200
[PATCH 2/3] checkpatch: colorize output to terminal Joe Perches <joe@perches.com> - 2015-06-03 18:00 +0200
Re: [PATCH 2/3] checkpatch: colorize output to terminal Petr Mladek <pmladek@suse.cz> - 2015-06-04 14:10 +0200
[PATCH 1/3] checkpatch: Improve output with multiple command-line files Joe Perches <joe@perches.com> - 2015-06-03 18:00 +0200
Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files Petr Mladek <pmladek@suse.cz> - 2015-06-04 14:10 +0200
Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files Joe Perches <joe@perches.com> - 2015-06-04 14:20 +0200
[PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames Joe Perches <joe@perches.com> - 2015-06-03 18:00 +0200
Re: [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames Petr Mladek <pmladek@suse.cz> - 2015-06-04 14:20 +0200
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-06-03 18:00 +0200 |
| Subject | [PATCH 0/3] checkpatch: output style changes |
| Message-ID | <pxjfb-5DK-1@gated-at.bofh.it> |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-06-03 18:00 +0200 |
| Subject | [PATCH 2/3] checkpatch: colorize output to terminal |
| Message-ID | <pxjfc-5DK-19@gated-at.bofh.it> |
| In reply to | #1157795 |
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 <joe@perches.com>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Petr Mladek <pmladek@suse.cz> |
|---|---|
| Date | 2015-06-04 14:10 +0200 |
| Subject | Re: [PATCH 2/3] checkpatch: colorize output to terminal |
| Message-ID | <pxC8a-lr-11@gated-at.bofh.it> |
| In reply to | #1157800 |
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 <joe@perches.com>
Tested-by: Petr Mladek <pmladek@suse.cz>
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-06-03 18:00 +0200 |
| Subject | [PATCH 1/3] checkpatch: Improve output with multiple command-line files |
| Message-ID | <pxjfc-5DK-23@gated-at.bofh.it> |
| In reply to | #1157795 |
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 <pmladek@suse.cz>
Signed-off-by: Joe Perches <joe@perches.com>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Petr Mladek <pmladek@suse.cz> |
|---|---|
| Date | 2015-06-04 14:10 +0200 |
| Subject | Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files |
| Message-ID | <pxC8b-lr-21@gated-at.bofh.it> |
| In reply to | #1157802 |
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 <pmladek@suse.cz>
> Signed-off-by: Joe Perches <joe@perches.com>
Tested-by: Petr Mladek <pmladek@suse.cz>
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-06-04 14:20 +0200 |
| Subject | Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files |
| Message-ID | <pxChQ-xc-17@gated-at.bofh.it> |
| In reply to | #1158499 |
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 <pmladek@suse.cz> [] > > +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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-06-03 18:00 +0200 |
| Subject | [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames |
| Message-ID | <pxjfd-5DK-27@gated-at.bofh.it> |
| In reply to | #1157795 |
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 <joe@perches.com>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Petr Mladek <pmladek@suse.cz> |
|---|---|
| Date | 2015-06-04 14:20 +0200 |
| Subject | Re: [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames |
| Message-ID | <pxChQ-xc-7@gated-at.bofh.it> |
| In reply to | #1157803 |
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 <joe@perches.com>
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web