Messages in this thread Patch in this message | | | Subject | Re: [PATCH] checkpatch: Make the output better readable | From | Joe Perches <> | Date | Tue, 02 Jun 2015 13:49:19 -0700 |
| |
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; }
| |