On Wed, 2019-05-08 at 19:43 +0200, Antonio Borneo wrote: > The size of 8 characters used for both TAB and indentation is > embedded as magic value allover the checkpatch script, and this > makes the script less readable.
I doubt this bit of the commit message is proper. Tabs _are_ 8 in the linux-kernel sources and checkpatch was written for the linux-kernel. Using a variable _could_ reasonably be described as an improvement, but readability wasn't and isn't really an issue here. Other than that, the patch seems fine. thanks, Joe > Replace the magic value 8 with a variable. > From the context of the code it's clear if it is used for > indentation or tabulation, so no need to use two separate > variables. > > Add a command-line option "--tab-size" to let the user select a > TAB size value other than 8. > This makes easy to reuse this script by other projects with > different requirements in their coding style (e.g. OpenOCD [1] > requires TAB size of 4 characters [2]). > > [1] http://openocd.org/ > [2] http://openocd.org/doc/doxygen/html/stylec.html#styleformat > > Signed-off-by: Antonio Borneo <[email protected]> > Signed-off-by: Erik Ahlén <[email protected]> > Signed-off-by: Spencer Oliver <[email protected]> > --- > V1 -> V2 > add the command line option > > scripts/checkpatch.pl | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 916a3fbd4d47..90f641bf1895 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -62,6 +62,7 @@ my $conststructsfile = "$D/const_structs.checkpatch"; > my $typedefsfile = ""; > my $color = "auto"; > my $allow_c99_comments = 1; # Can be overridden by --ignore > C99_COMMENT_TOLERANCE > +my $tabsize = 8; > > sub help { > my ($exitcode) = @_; > @@ -96,6 +97,7 @@ Options: > --show-types show the specific message type in the output > --max-line-length=n set the maximum line length, if exceeded, warn > --min-conf-desc-length=n set the min description length, if shorter, warn > + --tab-size=n set the number of spaces for tab (default 8) > --root=PATH PATH to the kernel tree root > --no-summary suppress the per-file summary > --mailback only produce a report in case of warnings/errors > @@ -213,6 +215,7 @@ GetOptions( > 'list-types!' => \$list_types, > 'max-line-length=i' => \$max_line_length, > 'min-conf-desc-length=i' => \$min_conf_desc_length, > + 'tab-size=i' => \$tabsize, > 'root=s' => \$root, > 'summary!' => \$summary, > 'mailback!' => \$mailback, > @@ -1211,7 +1214,7 @@ sub expand_tabs { > if ($c eq "\t") { > $res .= ' '; > $n++; > - for (; ($n % 8) != 0; $n++) { > + for (; ($n % $tabsize) != 0; $n++) { > $res .= ' '; > } > next; > @@ -2224,7 +2227,7 @@ sub string_find_replace { > sub tabify { > my ($leading) = @_; > > - my $source_indent = 8; > + my $source_indent = $tabsize; > my $max_spaces_before_tab = $source_indent - 1; > my $spaces_to_tab = " " x $source_indent; > > @@ -3153,7 +3156,7 @@ sub process { > next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/); > > # at the beginning of a line any tabs must come first and anything > -# more than 8 must use tabs. > +# more than $tabsize must use tabs. > if ($rawline =~ /^\+\s* \t\s*\S/ || > $rawline =~ /^\+\s* \s*/) { > my $herevet = "$here\n" . cat_vet($rawline) . "\n"; > @@ -3172,7 +3175,7 @@ sub process { > "please, no space before tabs\n" . $herevet) && > $fix) { > while ($fixed[$fixlinenr] =~ > - s/(^\+.*) {8,8}\t/$1\t\t/) {} > + s/(^\+.*) > {$tabsize,$tabsize}\t/$1\t\t/) {} > while ($fixed[$fixlinenr] =~ > s/(^\+.*) +\t/$1\t/) {} > } > @@ -3194,11 +3197,11 @@ sub process { > if ($perl_version_ok && > $sline =~ /^\+\t+( > +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=])/) > { > my $indent = length($1); > - if ($indent % 8) { > + if ($indent % $tabsize) { > if (WARN("TABSTOP", > "Statements should start on a > tabstop\n" . $herecurr) && > $fix) { > - $fixed[$fixlinenr] =~ s@(^\+\t+) +@$1 . > "\t" x ($indent/8)@e; > + $fixed[$fixlinenr] =~ s@(^\+\t+) +@$1 . > "\t" x ($indent/$tabsize)@e; > } > } > } > @@ -3216,8 +3219,8 @@ sub process { > my $newindent = $2; > > my $goodtabindent = $oldindent . > - "\t" x ($pos / 8) . > - " " x ($pos % 8); > + "\t" x ($pos / $tabsize) . > + " " x ($pos % $tabsize); > my $goodspaceindent = $oldindent . " " x $pos; > > if ($newindent ne $goodtabindent && > @@ -3688,11 +3691,11 @@ sub process { > #print "line<$line> prevline<$prevline> indent<$indent> > sindent<$sindent> check<$check> continuation<$continuation> s<$s> > cond_lines<$cond_lines> stat_real<$stat_real> stat<$stat>\n"; > > if ($check && $s ne '' && > - (($sindent % 8) != 0 || > + (($sindent % $tabsize) != 0 || > ($sindent < $indent) || > ($sindent == $indent && > ($s !~ /^\s*(?:\}|\{|else\b)/)) || > - ($sindent > $indent + 8))) { > + ($sindent > $indent + $tabsize))) { > WARN("SUSPECT_CODE_INDENT", > "suspect code indent for conditional > statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n"); > }
