From: Petr Mladek <pmladek@suse.cz>
To: Joe Perches <joe@perches.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Andy Whitcroft <apw@canonical.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] checkpatch: Improve output with multiple command-line files
Date: Thu, 4 Jun 2015 14:03:28 +0200 [thread overview]
Message-ID: <20150604120328.GN3135@pathway.suse.cz> (raw)
In-Reply-To: <d5a2b71b4ec735b70c326ed2554d8c627f13c248.1433346576.git.joe@perches.com>
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
>
next prev parent reply other threads:[~2015-06-04 12:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-03 15:53 [PATCH 0/3] checkpatch: output style changes Joe Perches
2015-06-03 15:53 ` [PATCH 1/3] checkpatch: Improve output with multiple command-line files Joe Perches
2015-06-04 12:03 ` Petr Mladek [this message]
2015-06-04 12:14 ` Joe Perches
2015-06-04 12:29 ` Petr Mladek
2015-06-04 12:36 ` Joe Perches
2015-06-03 15:53 ` [PATCH 2/3] checkpatch: colorize output to terminal Joe Perches
2015-06-04 12:05 ` Petr Mladek
2015-06-03 15:53 ` [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames Joe Perches
2015-06-04 12:18 ` Petr Mladek
2015-06-04 12:40 ` Joe Perches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150604120328.GN3135@pathway.suse.cz \
--to=pmladek@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=apw@canonical.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome