Re: [PATCH v6 1/2] checkpatch: add verbose mode
On Mon, Feb 22, 2021 at 8:14 AM Joe Perches wrote: > > On Mon, 2021-02-22 at 00:05 +0530, Dwaipayan Ray wrote: > > On Sun, Feb 21, 2021 at 11:36 PM Joe Perches wrote: > > > > > > On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote: > > > > Add a new verbose mode to checkpatch.pl to emit additional verbose > > > > test descriptions. The verbose mode is optional and can be enabled > > > > by the flag -v or --verbose. > > > > > > OK, maybe add color coding to the list_types output. > > Okay, nice idea! > [] > > Sure, I will do something like this. > > Are there any other improvements you can see right now > > or will the coloring thing suffice? > > A lot more descriptive output for the .rst file and > of course a lot more of the types documented... > Sure I was hoping to do that over time after getting this series in... Nevertheless I will send the v7 in right away with the color code changes. Thanks, Dwaipayan.
Re: [PATCH v6 1/2] checkpatch: add verbose mode
On Mon, 2021-02-22 at 00:05 +0530, Dwaipayan Ray wrote: > On Sun, Feb 21, 2021 at 11:36 PM Joe Perches wrote: > > > > On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote: > > > Add a new verbose mode to checkpatch.pl to emit additional verbose > > > test descriptions. The verbose mode is optional and can be enabled > > > by the flag -v or --verbose. > > > > OK, maybe add color coding to the list_types output. > Okay, nice idea! [] > Sure, I will do something like this. > Are there any other improvements you can see right now > or will the coloring thing suffice? A lot more descriptive output for the .rst file and of course a lot more of the types documented...
Re: [PATCH v6 1/2] checkpatch: add verbose mode
On Sun, Feb 21, 2021 at 11:36 PM Joe Perches wrote: > > On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote: > > Add a new verbose mode to checkpatch.pl to emit additional verbose > > test descriptions. The verbose mode is optional and can be enabled > > by the flag -v or --verbose. > > OK, maybe add color coding to the list_types output. Okay, nice idea! > Something like: > --- > scripts/checkpatch.pl | 61 > --- > 1 file changed, 43 insertions(+), 18 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index bbff13e0ca7e..ccd4a1bd73cb 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -163,14 +163,39 @@ sub list_types { > my $text = <$script>; > close($script); > > - my @types = (); > + my %types = (); > # Also catch when type or level is passed through a variable > - for ($text =~ > /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) > { > - push (@types, $_); > + while ($text =~ > /\b(CHK|WARN|ERROR|&\{\$msg_level})\s*\(\s*"([^"]+)"/g) { > + if (exists($types{$2})) { > + $types{$2} .= ",$1" if ($types{$2} ne $1); > + } else { > + $types{$2} = $1; > + } > + } > + print("#\tMessage type"); > + if ($color) { > + print(" (color coding: "); > + print(RED . "ERROR" . RESET); > + print("|"); > + print(YELLOW . "WARNING" . RESET); > + print("|"); > + print(GREEN . "CHECK" . RESET); > + print("|"); > + print("Multiple levels" . RESET); > + print(")"); > } > - @types = sort(uniq(@types)); > - print("#\tMessage type\n\n"); > - foreach my $type (@types) { > + print("\n\n"); > + foreach my $type (sort keys %types) { > + if ($color) { > + my $level = $types{$type}; > + if ($level eq 'ERROR') { > + $type = RED . $type . RESET; > + } elsif ($level eq 'WARN') { > + $type = YELLOW . $type . RESET; > + } elsif ($level eq 'CHK') { > + $type = GREEN . $type . RESET; > + } > + } > print(++$count . "\t" . $type . "\n"); > if ($verbose && exists($verbose_messages{$type})) { > my $message = $verbose_messages{$type}; > @@ -301,6 +326,18 @@ help(0) if ($help); > die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || > $fix)); > die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse); > > +if ($color =~ /^[01]$/) { > + $color = !$color; > +} elsif ($color =~ /^always$/i) { > + $color = 1; > +} elsif ($color =~ /^never$/i) { > + $color = 0; > +} elsif ($color =~ /^auto$/i) { > + $color = (-t STDOUT); > +} else { > + die "$P: Invalid color mode: $color\n"; > +} > + > load_docs() if ($verbose); > list_types(0) if ($list_types); > > @@ -321,18 +358,6 @@ if ($#ARGV < 0) { > push(@ARGV, '-'); > } > > -if ($color =~ /^[01]$/) { > - $color = !$color; > -} elsif ($color =~ /^always$/i) { > - $color = 1; > -} elsif ($color =~ /^never$/i) { > - $color = 0; > -} elsif ($color =~ /^auto$/i) { > - $color = (-t STDOUT); > -} else { > - die "$P: Invalid color mode: $color\n"; > -} > - > # skip TAB size 1 to avoid additional checks on $tabsize - 1 > die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2); > > Sure, I will do something like this. Are there any other improvements you can see right now or will the coloring thing suffice? Thanks, Dwaipayan
Re: [PATCH v6 1/2] checkpatch: add verbose mode
On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote: > Add a new verbose mode to checkpatch.pl to emit additional verbose > test descriptions. The verbose mode is optional and can be enabled > by the flag -v or --verbose. OK, maybe add color coding to the list_types output. Something like: --- scripts/checkpatch.pl | 61 --- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index bbff13e0ca7e..ccd4a1bd73cb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -163,14 +163,39 @@ sub list_types { my $text = <$script>; close($script); - my @types = (); + my %types = (); # Also catch when type or level is passed through a variable - for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) { - push (@types, $_); + while ($text =~ /\b(CHK|WARN|ERROR|&\{\$msg_level})\s*\(\s*"([^"]+)"/g) { + if (exists($types{$2})) { + $types{$2} .= ",$1" if ($types{$2} ne $1); + } else { + $types{$2} = $1; + } + } + print("#\tMessage type"); + if ($color) { + print(" (color coding: "); + print(RED . "ERROR" . RESET); + print("|"); + print(YELLOW . "WARNING" . RESET); + print("|"); + print(GREEN . "CHECK" . RESET); + print("|"); + print("Multiple levels" . RESET); + print(")"); } - @types = sort(uniq(@types)); - print("#\tMessage type\n\n"); - foreach my $type (@types) { + print("\n\n"); + foreach my $type (sort keys %types) { + if ($color) { + my $level = $types{$type}; + if ($level eq 'ERROR') { + $type = RED . $type . RESET; + } elsif ($level eq 'WARN') { + $type = YELLOW . $type . RESET; + } elsif ($level eq 'CHK') { + $type = GREEN . $type . RESET; + } + } print(++$count . "\t" . $type . "\n"); if ($verbose && exists($verbose_messages{$type})) { my $message = $verbose_messages{$type}; @@ -301,6 +326,18 @@ help(0) if ($help); die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse); +if ($color =~ /^[01]$/) { + $color = !$color; +} elsif ($color =~ /^always$/i) { + $color = 1; +} elsif ($color =~ /^never$/i) { + $color = 0; +} elsif ($color =~ /^auto$/i) { + $color = (-t STDOUT); +} else { + die "$P: Invalid color mode: $color\n"; +} + load_docs() if ($verbose); list_types(0) if ($list_types); @@ -321,18 +358,6 @@ if ($#ARGV < 0) { push(@ARGV, '-'); } -if ($color =~ /^[01]$/) { - $color = !$color; -} elsif ($color =~ /^always$/i) { - $color = 1; -} elsif ($color =~ /^never$/i) { - $color = 0; -} elsif ($color =~ /^auto$/i) { - $color = (-t STDOUT); -} else { - die "$P: Invalid color mode: $color\n"; -} - # skip TAB size 1 to avoid additional checks on $tabsize - 1 die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2);
[PATCH v6 1/2] checkpatch: add verbose mode
Add a new verbose mode to checkpatch.pl to emit additional verbose test descriptions. The verbose mode is optional and can be enabled by the flag -v or --verbose. The test descriptions are parsed from the checkpatch documentation file at `Documentation/dev-tools/checkpatch.rst`. The test descriptions in the docs are kept in a fixed format grouped by usage. Some examples of this format are: **LINE_SPACING** Vertical space is wasted given the limited number of lines an editor window can display when multiple blank lines are used. **MISSING_SIGN_OFF** The patch is missing a Signed-off-by line. A signed-off-by line should be added according to Developer's certificate of Origin. To avoid lengthy output, the verbose description is printed only for the first instance of a particular message type. The --verbose option cannot be used along with the --terse option. Verbose mode can also be used with the --list-types option, which will then output the verbose descriptions along with the checkpatch message types. Signed-off-by: Dwaipayan Ray --- scripts/checkpatch.pl | 68 +-- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9a549b009d2f..b44fcf13dec1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -23,6 +23,9 @@ my $V = '0.32'; use Getopt::Long qw(:config no_auto_abbrev); my $quiet = 0; +my $verbose = 0; +my %verbose_messages = (); +my %verbose_emitted = (); my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; @@ -61,6 +64,7 @@ my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $conststructsfile = "$D/const_structs.checkpatch"; +my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst"; my $typedefsfile; my $color = "auto"; my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE @@ -78,6 +82,7 @@ Version: $V Options: -q, --quietquiet + -v, --verbose verbose mode --no-tree run without a kernel tree --no-signoff do not check for 'Signed-off-by' line --patchtreat FILE as patchfile (default) @@ -167,6 +172,11 @@ sub list_types { print("#\tMessage type\n\n"); foreach my $type (@types) { print(++$count . "\t" . $type . "\n"); + if ($verbose && exists($verbose_messages{$type})) { + my $message = $verbose_messages{$type}; + $message =~ s/\n/\n\t/g; + print("\t" . $message . "\n\n"); + } } exit($exitcode); @@ -198,6 +208,46 @@ if (-f $conf) { unshift(@ARGV, @conf_args) if @conf_args; } +sub load_docs { + open(my $docs, '<', "$docsfile") + or warn "$P: Can't read the documentation file $docsfile $!\n"; + + my $type = ''; + my $desc = ''; + my $in_desc = 0; + + while (<$docs>) { + chomp; + my $line = $_; + $line =~ s/\s+$//; + + if ($line =~ /^\s*\*\*(.+)\*\*$/) { + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + $type = $1; + $desc = ''; + $in_desc = 1; + } elsif ($in_desc) { + if ($line =~ /^(?:\s{4,}|$)/) { + $line =~ s/^\s{4}//; + $desc .= $line; + $desc .= "\n"; + } else { + $verbose_messages{$type} = trim($desc); + $type = ''; + $desc = ''; + $in_desc = 0; + } + } + } + + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + close($docs); +} + # Perl's Getopt::Long allows options to take optional arguments after a space. # Prevent --color by itself from consuming other arguments foreach (@ARGV) { @@ -208,6 +258,7 @@ foreach (@ARGV) { GetOptions( 'q|quiet+' => \$quiet, + 'v|verbose!'=> \$verbose, 'tree!' => \$tree, 'signoff!' => \$chk_signoff, 'patch!'=> \$chk_patch, @@ -247,13 +298,15 @@ GetOptions( help(0) if ($help); +die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); +die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse); + +load_docs() if ($verbose); list_types(0) if ($list_types); $fix = 1 if ($fix_inplace); $check_orig = $check; -die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); - my $exit = 0; my $perl_version_ok = 1; @@