2008-03-05 20:49:44

by Paolo Ciarrocchi

[permalink] [raw]
Subject: [PATCH] checkpatch: Make some text more consinstent and informative

Hi Andy,

Now messages about missing spaces or not needed spaces are in the format:
space is required
space is not required

Signed-off-by: Paolo Ciarrocchi <[email protected]>
---
checkpatch.pl | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/checkpatch.pl b/checkpatch.pl
index 2a7cef9..dfaa5f0 100755
--- a/checkpatch.pl
+++ b/checkpatch.pl
@@ -1005,7 +1005,7 @@ sub process {
$herecurr);
}
if ($line =~ /^\s*signed-off-by:\S/i) {
- WARN("need space after Signed-off-by:\n" .
+ WARN("space is required after Signed-off-by:\n" .
$herecurr);
}
}
@@ -1340,7 +1340,7 @@ sub process {
} elsif ("$ctx$name" =~ /$Type$/) {

} else {
- WARN("no space between function name and open parenthesis '('\n" . $herecurr);
+ WARN("space is not required between function name and open parenthesis '('\n" . $herecurr);
}
}
# Check operator spacing.
@@ -1424,7 +1424,7 @@ sub process {
} elsif ($op eq ';') {
if ($ctx !~ /.x[WEBC]/ &&
$cc !~ /^\\/ && $cc !~ /^;/) {
- ERROR("need space after that '$op' $at\n" . $hereptr);
+ ERROR("space is reuired after that '$op' $at\n" . $hereptr);
}

# // is a comment
@@ -1433,13 +1433,13 @@ sub process {
# -> should have no spaces
} elsif ($op eq '->') {
if ($ctx =~ /Wx.|.xW/) {
- ERROR("no spaces around that '$op' $at\n" . $hereptr);
+ ERROR("space are not required around that '$op' $at\n" . $hereptr);
}

# , must have a space on the right.
} elsif ($op eq ',') {
if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) {
- ERROR("need space after that '$op' $at\n" . $hereptr);
+ ERROR("space is required after that '$op' $at\n" . $hereptr);
}

# '*' as part of a type definition -- reported already.
@@ -1452,10 +1452,10 @@ sub process {
} elsif ($op eq '!' || $op eq '~' ||
($is_unary && ($op eq '*' || $op eq '-' || $op eq '&'))) {
if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
- ERROR("need space before that '$op' $at\n" . $hereptr);
+ ERROR("space is required before that '$op' $at\n" . $hereptr);
}
if ($ctx =~ /.xW/) {
- ERROR("no space after that '$op' $at\n" . $hereptr);
+ ERROR("space is not required after that '$op' $at\n" . $hereptr);
}

# unary ++ and unary -- are allowed no space on one side.
@@ -1464,7 +1464,7 @@ sub process {
ERROR("need space one side of that '$op' $at\n" . $hereptr);
}
if ($ctx =~ /WxB/ || ($ctx =~ /Wx./ && $cc =~ /^;/)) {
- ERROR("no space before that '$op' $at\n" . $hereptr);
+ ERROR("space is not required before that '$op' $at\n" . $hereptr);
}

# << and >> may either have or not have spaces both sides
@@ -1514,31 +1514,31 @@ sub process {
#need space before brace following if, while, etc
if (($line =~ /\(.*\){/ && $line !~ /\($Type\){/) ||
$line =~ /do{/) {
- ERROR("need a space before the open brace '{'\n" . $herecurr);
+ ERROR("space ia required before the open brace '{'\n" . $herecurr);
}

# closing brace should have a space following it when it has anything
# on the line
if ($line =~ /}(?!(?:,|;|\)))\S/) {
- ERROR("need a space after that close brace '}'\n" . $herecurr);
+ ERROR("space is required after that close brace '}'\n" . $herecurr);
}

# check spacing on square brackets
if ($line =~ /\[\s/ && $line !~ /\[\s*$/) {
- ERROR("no space after that open square bracket '['\n" . $herecurr);
+ ERROR("space is not required after that open square bracket '['\n" . $herecurr);
}
if ($line =~ /\s\]/) {
- ERROR("no space before that close square bracket ']'\n" . $herecurr);
+ ERROR("space is not required before that close square bracket ']'\n" . $herecurr);
}

# check spacing on paretheses
if ($line =~ /\(\s/ && $line !~ /\(\s*(?:\\)?$/ &&
$line !~ /for\s*\(\s+;/) {
- ERROR("no space after that open parenthesis '('\n" . $herecurr);
+ ERROR("space is not required after that open parenthesis '('\n" . $herecurr);
}
if ($line =~ /(\s+)\)/ && $line !~ /^.\s*\)/ &&
$line !~ /for\s*\(.*;\s+\)/) {
- ERROR("no space before that close parenthesis ')'\n" . $herecurr);
+ ERROR("space ia not required before that close parenthesis ')'\n" . $herecurr);
}

#goto labels aren't indented, allow a single space however
@@ -1549,7 +1549,7 @@ sub process {

# Need a space before open parenthesis after if, while etc
if ($line=~/\b(if|while|for|switch)\(/) {
- ERROR("need a space before the open parenthesis '('\n" . $herecurr);
+ ERROR("space is required before the open parenthesis '('\n" . $herecurr);
}

# Check for illegal assignment in if conditional.
--
1.5.4.950.ga176


2008-03-06 10:20:42

by Andy Whitcroft

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Make some text more consinstent and informative

On Wed, Mar 05, 2008 at 09:49:12PM +0100, Paolo Ciarrocchi wrote:
> Hi Andy,
>
> Now messages about missing spaces or not needed spaces are in the format:
> space is required
> space is not required
>
> Signed-off-by: Paolo Ciarrocchi <[email protected]>

The current version (which your patch has crossed with) has these changed
to "required" and "prohibited" respectivly, to remove the ambiguity introduced by
tehe "no spaces X" form.

I did consider and reject the "is not required" form. In english this
does not correct carry the "must not be present" meaning, it more means
"may or may not be present at your option". Which is not what we are
trying to say either.

Hopefully the new wording is clearer to native and non-native english
speakers alike.

Thanks for doing the patch even so.

-apw

2008-03-06 22:33:27

by Paolo Ciarrocchi

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Make some text more consinstent and informative

On 3/6/08, Andy Whitcroft <[email protected]> wrote:
> On Wed, Mar 05, 2008 at 09:49:12PM +0100, Paolo Ciarrocchi wrote:
> > Hi Andy,
> >
> > Now messages about missing spaces or not needed spaces are in the format:
> > space is required
> > space is not required
> >
> > Signed-off-by: Paolo Ciarrocchi <[email protected]>
>
> The current version (which your patch has crossed with) has these changed
> to "required" and "prohibited" respectivly, to remove the ambiguity
> introduced by
> tehe "no spaces X" form.
>
> I did consider and reject the "is not required" form. In english this
> does not correct carry the "must not be present" meaning, it more means
> "may or may not be present at your option". Which is not what we are
> trying to say either.

yes, i tend to agree.

> Hopefully the new wording is clearer to native and non-native english
> speakers alike.

i had a look at the next branch, i really like the new output. Very nice work.

> Thanks for doing the patch even so.

you are welcome, i'll keep tracking the next branch.

ciao,
--
Paolo
http://paolo.ciarrocchi.googlepages.com/