mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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
> 

  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