[PATCH] Change a WARN message in checkpatch

[PATCH] Change a WARN message in checkpatch

[Paolo Ciarrocchi]

Hi Andy,
When I started using checkpatch I was confused by the following WARN message:

	no space between function name and open parenthesis

I thought the problem was that a space was missing while the truth is the opposite.

How about the following patch?


--- checkpatch.pl.old	2008-01-04 13:37:51.000000000 +0100
+++ checkpatch.pl	2008-01-04 13:37:24.000000000 +0100
@@ -1117,7 +1117,7 @@
 		while ($line =~ /($Ident)\s+\(/g) {
 			if ($1 !~ /^(?:if|for|while|switch|return|volatile|__volatile__|__attribute__|format|__extension__|Copyright|case)$/ &&
 		            $line !~ /$Type\s+\(/ && $line !~ /^.\#\s*define\b/) {
-				WARN("no space between function name and open parenthesis '('\n" . $herecurr);
+				WARN("don't put a space between function name and open parenthesis '('\n" . $herecurr);
 			}
 		}
 # Check operator spacing.

Re: [PATCH] Change a WARN message in checkpatch

[Andy Whitcroft]

On Mon, Jan 14, 2008 at 11:29:13PM +0100, Paolo Ciarrocchi wrote:
> Hi Andy,
> When I started using checkpatch I was confused by the following WARN message:
> 
> 	no space between function name and open parenthesis
> 
> I thought the problem was that a space was missing while the truth is the opposite.
> 
> How about the following patch?

I can see how that language would be confusing.  Your patch makes the
english clearer, but somehow seems clumsy.  There must be a better way
to say this.  I will think on it and see what I can come up with.

-apw

Re: [PATCH] Change a WARN message in checkpatch

[Paolo Ciarrocchi]

On Jan 28, 2008 3:56 PM, Andy Whitcroft <apw@shadowen.org> wrote:
> On Mon, Jan 14, 2008 at 11:29:13PM +0100, Paolo Ciarrocchi wrote:
> > Hi Andy,
> > When I started using checkpatch I was confused by the following WARN message:
> >
> >       no space between function name and open parenthesis
> >
> > I thought the problem was that a space was missing while the truth is the opposite.
> >
> > How about the following patch?
>
> I can see how that language would be confusing.  Your patch makes the
> english clearer, but somehow seems clumsy.  There must be a better way
> to say this.  I will think on it and see what I can come up with.

Fair enought, I'm not English mother tongue and I'm sure you can come
up with a better
sentence :-)

BTW, is there a public repo I can track to look at all the new patches?

Thanks.

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

Re: [PATCH] Change a WARN message in checkpatch

[Paolo Ciarrocchi]

On Mon, Jan 28, 2008 at 4:13 PM, Paolo Ciarrocchi
<paolo.ciarrocchi@gmail.com> wrote:
>
> On Jan 28, 2008 3:56 PM, Andy Whitcroft <apw@shadowen.org> wrote:
>  > On Mon, Jan 14, 2008 at 11:29:13PM +0100, Paolo Ciarrocchi wrote:
>  > > Hi Andy,
>  > > When I started using checkpatch I was confused by the following WARN message:
>  > >
>  > >       no space between function name and open parenthesis
>  > >
>  > > I thought the problem was that a space was missing while the truth is the opposite.
>  > >
>  > > How about the following patch?
>  >
>  > I can see how that language would be confusing.  Your patch makes the
>  > english clearer, but somehow seems clumsy.  There must be a better way
>  > to say this.  I will think on it and see what I can come up with.
>
>  Fair enought, I'm not English mother tongue and I'm sure you can come
>  up with a better
>  sentence :-)

AFAICS the problem is still present.

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

Re: [PATCH] Change a WARN message in checkpatch

[Benny Halevy]

On Feb. 23, 2008, 5:38 -0800, "Paolo Ciarrocchi" <paolo.ciarrocchi@gmail.com> wrote:
> On Mon, Jan 28, 2008 at 4:13 PM, Paolo Ciarrocchi
> <paolo.ciarrocchi@gmail.com> wrote:
>> On Jan 28, 2008 3:56 PM, Andy Whitcroft <apw@shadowen.org> wrote:
>>  > On Mon, Jan 14, 2008 at 11:29:13PM +0100, Paolo Ciarrocchi wrote:
>>  > > Hi Andy,
>>  > > When I started using checkpatch I was confused by the following WARN message:
>>  > >
>>  > >       no space between function name and open parenthesis
>>  > >
>>  > > I thought the problem was that a space was missing while the truth is the opposite.
>>  > >
>>  > > How about the following patch?
>>  >
>>  > I can see how that language would be confusing.  Your patch makes the
>>  > english clearer, but somehow seems clumsy.  There must be a better way
>>  > to say this.  I will think on it and see what I can come up with.
>>
>>  Fair enought, I'm not English mother tongue and I'm sure you can come
>>  up with a better
>>  sentence :-)
> 
> AFAICS the problem is still present.
> 
> Ciao,

How about:
-				WARN("no space between function name and open parenthesis '('\n" . $herecurr);
+				WARN("there should be no space between function name and open parenthesis '('\n" . $herecurr);

The original phrasing can be interpreted like "there is no space between ..." and the correct
interpretation should be "there should be no space between ..."

Benny

Re: [PATCH] Change a WARN message in checkpatch

[Paolo Ciarrocchi]

On Sun, Feb 24, 2008 at 4:18 PM, Benny Halevy <bhalevy@panasas.com> wrote:
[...]
>  How about:
>  -                               WARN("no space between function name and open parenthesis '('\n" . $herecurr);
>  +                               WARN("there should be no space between function name and open parenthesis '('\n" . $herecurr);

I originally suggested:
+                               WARN("don't put a space between
function name and open parenthesis '('\n" . $herecurr);
but I really prefer your version.

>  The original phrasing can be interpreted like "there is no space between ..." and the correct
>  interpretation should be "there should be no space between ..."

Indeed.

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

Re: [PATCH] Change a WARN message in checkpatch

[Andy Whitcroft]

On Sun, Feb 24, 2008 at 07:14:13PM +0100, Paolo Ciarrocchi wrote:
> On Sun, Feb 24, 2008 at 4:18 PM, Benny Halevy <bhalevy@panasas.com> wrote:
> [...]
> >  How about:
> >  -                               WARN("no space between function name and open parenthesis '('\n" . $herecurr);
> >  +                               WARN("there should be no space between function name and open parenthesis '('\n" . $herecurr);
> 
> I originally suggested:
> +                               WARN("don't put a space between
> function name and open parenthesis '('\n" . $herecurr);
> but I really prefer your version.
> 
> >  The original phrasing can be interpreted like "there is no space between ..." and the correct
> >  interpretation should be "there should be no space between ..."
> 
> Indeed.

As we want the messages to be as short as possible, I am leaning towards
standardising on:

	spaces prohibited <where>
	spaces required <where>

-apw

Re: [PATCH] Change a WARN message in checkpatch

[Benny Halevy]

On Feb. 24, 2008, 19:29 -0800, Andy Whitcroft <apw@shadowen.org> wrote:
> On Sun, Feb 24, 2008 at 07:14:13PM +0100, Paolo Ciarrocchi wrote:
>> On Sun, Feb 24, 2008 at 4:18 PM, Benny Halevy <bhalevy@panasas.com> wrote:
>> [...]
>>>  How about:
>>>  -                               WARN("no space between function name and open parenthesis '('\n" . $herecurr);
>>>  +                               WARN("there should be no space between function name and open parenthesis '('\n" . $herecurr);
>> I originally suggested:
>> +                               WARN("don't put a space between
>> function name and open parenthesis '('\n" . $herecurr);
>> but I really prefer your version.
>>
>>>  The original phrasing can be interpreted like "there is no space between ..." and the correct
>>>  interpretation should be "there should be no space between ..."
>> Indeed.
> 
> As we want the messages to be as short as possible, I am leaning towards
> standardising on:
> 
> 	spaces prohibited <where>
> 	spaces required <where>

Sounds good to me.

Benny

> 
> -apw

Re: [PATCH] Change a WARN message in checkpatch

[Paolo Ciarrocchi]

On 2/25/08, Andy Whitcroft
> As we want the messages to be as short as possible, I am leaning towards
> standardising on:
>
> spaces prohibited <where>
> spaces required <where>
>
in that case i would prefer:
 space not required <where>
 space required <where>

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

Re: [PATCH] Change a WARN message in checkpatch

[Andy Whitcroft]

On Mon, Feb 25, 2008 at 10:21:01AM +0330, Paolo Ciarrocchi wrote:
> On 2/25/08, Andy Whitcroft
> > As we want the messages to be as short as possible, I am leaning towards
> > standardising on:
> >
> > spaces prohibited <where>
> > spaces required <where>
> >
> in that case i would prefer:
>  space not required <where>
>  space required <where>

"not required" implies that you don't have to have the space, but you
can can have it if you want.  So I don't think thats quite right either.

-apw

[PATCH] Change a WARN message in checkpatch

[Paolo Ciarrocchi]

Hi Andy,
I just started using checkpatch and I was confused by the following WARN message:

	no space between function name and open parenthesis

I thought the probelm was that a space was missing while the truth is the opposite.

How about the following patch?


--- checkpatch.pl.old	2008-01-04 13:37:51.000000000 +0100
+++ checkpatch.pl	2008-01-04 13:37:24.000000000 +0100
@@ -1117,7 +1117,7 @@
 		while ($line =~ /($Ident)\s+\(/g) {
 			if ($1 !~ /^(?:if|for|while|switch|return|volatile|__volatile__|__attribute__|format|__extension__|Copyright|case)$/ &&
 		            $line !~ /$Type\s+\(/ && $line !~ /^.\#\s*define\b/) {
-				WARN("no space between function name and open parenthesis '('\n" . $herecurr);
+				WARN("don't put a space between function name and open parenthesis '('\n" . $herecurr);
 			}
 		}
 # Check operator spacing.