* [PATCH] checkpatch: Better --fix of SPACING errors.
@ 2013-08-07 2:55 Joe Perches
2013-08-07 4:00 ` Joe Perches
0 siblings, 1 reply; 2+ messages in thread
From: Joe Perches @ 2013-08-07 2:55 UTC (permalink / raw)
To: Andrew Morton; +Cc: Phil Carmody, Andy Whitcroft, linux-kernel
Previous attempt at fixing SPACING errors could
make a hash of several defects.
This patch should make --fix be a lot better at
correcting these defects.
Trim left and right sides of these defects
appropriately instead of a somewhat random attempt
at it.
Trim left spaces from any following bit of the
modified line when only a single space is required
around an operator.
Signed-off-by: Joe Perches <joe@perches.com>
---
scripts/checkpatch.pl | 64 +++++++++++++++++++++++++++++++++------------------
1 file changed, 42 insertions(+), 22 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ca5079e..c25af23 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1472,7 +1472,23 @@ sub check_absolute_file {
sub trim {
my ($string) = @_;
- $string =~ s/(^\s+|\s+$)//g;
+ $string =~ s/^\s+|\s+$//g;
+
+ return $string;
+}
+
+sub ltrim {
+ my ($string) = @_;
+
+ $string =~ s/^\s+//;
+
+ return $string;
+}
+
+sub rtrim {
+ my ($string) = @_;
+
+ $string =~ s/\s+$//;
return $string;
}
@@ -2821,6 +2837,7 @@ sub process {
$off = 0;
my $blank = copy_spacing($opline);
+ my $last_after = -1;
for (my $n = 0; $n < $#elements; $n += 2) {
@@ -2886,7 +2903,7 @@ sub process {
$cc !~ /^\\/ && $cc !~ /^;/) {
if (ERROR("SPACING",
"space required after that '$op' $at\n" . $hereptr)) {
- $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
+ $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " ";
$line_fixed = 1;
}
}
@@ -2901,11 +2918,11 @@ sub process {
if ($ctx =~ /Wx.|.xW/) {
if (ERROR("SPACING",
"spaces prohibited around that '$op' $at\n" . $hereptr)) {
- $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
- $line_fixed = 1;
+ $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
if (defined $fix_elements[$n + 2]) {
$fix_elements[$n + 2] =~ s/^\s+//;
}
+ $line_fixed = 1;
}
}
@@ -2914,8 +2931,9 @@ sub process {
if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) {
if (ERROR("SPACING",
"space required after that '$op' $at\n" . $hereptr)) {
- $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]) . " ";
+ $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " ";
$line_fixed = 1;
+ $last_after = $n;
}
}
@@ -2932,8 +2950,10 @@ sub process {
if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
if (ERROR("SPACING",
"space required before that '$op' $at\n" . $hereptr)) {
- $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]);
- $line_fixed = 1;
+ if ($n != $last_after + 2) {
+ $good = $fix_elements[$n] . " " . ltrim($fix_elements[$n + 1]);
+ $line_fixed = 1;
+ }
}
}
if ($op eq '*' && $cc =~/\s*$Modifier\b/) {
@@ -2942,12 +2962,11 @@ sub process {
} elsif ($ctx =~ /.xW/) {
if (ERROR("SPACING",
"space prohibited after that '$op' $at\n" . $hereptr)) {
- $fixed_line =~ s/\s+$//;
- $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
- $line_fixed = 1;
+ $good = $fix_elements[$n] . rtrim($fix_elements[$n + 1]);
if (defined $fix_elements[$n + 2]) {
$fix_elements[$n + 2] =~ s/^\s+//;
}
+ $line_fixed = 1;
}
}
@@ -2956,8 +2975,7 @@ sub process {
if ($ctx !~ /[WEOBC]x[^W]/ && $ctx !~ /[^W]x[WOBEC]/) {
if (ERROR("SPACING",
"space required one side of that '$op' $at\n" . $hereptr)) {
- $fixed_line =~ s/\s+$//;
- $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]) . " ";
+ $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " ";
$line_fixed = 1;
}
}
@@ -2965,20 +2983,18 @@ sub process {
($ctx =~ /Wx./ && $cc =~ /^;/)) {
if (ERROR("SPACING",
"space prohibited before that '$op' $at\n" . $hereptr)) {
- $fixed_line =~ s/\s+$//;
- $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
+ $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
$line_fixed = 1;
}
}
if ($ctx =~ /ExW/) {
if (ERROR("SPACING",
"space prohibited after that '$op' $at\n" . $hereptr)) {
- $fixed_line =~ s/\s+$//;
- $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
- $line_fixed = 1;
+ $good = $fix_elements[$n] . trim($fix_elements[$n + 1]);
if (defined $fix_elements[$n + 2]) {
$fix_elements[$n + 2] =~ s/^\s+//;
}
+ $line_fixed = 1;
}
}
@@ -2992,8 +3008,10 @@ sub process {
if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) {
if (ERROR("SPACING",
"need consistent spacing around '$op' $at\n" . $hereptr)) {
- $fixed_line =~ s/\s+$//;
- $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
+ $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
+ if (defined $fix_elements[$n + 2]) {
+ $fix_elements[$n + 2] =~ s/^\s+//;
+ }
$line_fixed = 1;
}
}
@@ -3004,7 +3022,7 @@ sub process {
if ($ctx =~ /Wx./) {
if (ERROR("SPACING",
"space prohibited before that '$op' $at\n" . $hereptr)) {
- $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
+ $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
$line_fixed = 1;
}
}
@@ -3031,8 +3049,10 @@ sub process {
if ($ok == 0) {
if (ERROR("SPACING",
"spaces required around that '$op' $at\n" . $hereptr)) {
- $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
- $good = $fix_elements[$n] . " " . trim($fix_elements[$n + 1]) . " ";
+ $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
+ if (defined $fix_elements[$n + 2]) {
+ $fix_elements[$n + 2] =~ s/^\s+//;
+ }
$line_fixed = 1;
}
}
--
1.8.1.2.459.gbcd45b4.dirty
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] checkpatch: Better --fix of SPACING errors.
2013-08-07 2:55 [PATCH] checkpatch: Better --fix of SPACING errors Joe Perches
@ 2013-08-07 4:00 ` Joe Perches
0 siblings, 0 replies; 2+ messages in thread
From: Joe Perches @ 2013-08-07 4:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Phil Carmody, Andy Whitcroft, linux-kernel, devel, kernel-janitors
On Tue, 2013-08-06 at 19:55 -0700, Joe Perches wrote:
> Previous attempt at fixing SPACING errors could
> make a hash of several defects.
I have run this against drivers/staging/...
It works rather better than the older version.
It does have an issue with c99 style comments
when it needs to rewrite spacing as well.
If you use this with -f, please convert the
c99 // comments to /* ... */ first using the
--types c99_comments option,
Maybe use:
$ ./scripts/checkpatch.pl -f --fix --types=c99_comments <file>
$ mv <file>.EXPERIMENTAL-checkpatch-fixes <file>
$ ./scripts/checkpatch.pl -f --fix --types=spacing <file>
$ mv <file>.EXPERIMENTAL-checkpatch-fixes <file>
$ git diff <file>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-08-07 4:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-07 2:55 [PATCH] checkpatch: Better --fix of SPACING errors Joe Perches
2013-08-07 4:00 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®