* [PATCH] Change a WARN message in checkpatch
@ 2008-01-14 22:29 Paolo Ciarrocchi
2008-01-28 14:56 ` Andy Whitcroft
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Ciarrocchi @ 2008-01-14 22:29 UTC (permalink / raw)
To: apw; +Cc: Linux Kernel
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.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Change a WARN message in checkpatch
2008-01-14 22:29 [PATCH] Change a WARN message in checkpatch Paolo Ciarrocchi
@ 2008-01-28 14:56 ` Andy Whitcroft
2008-01-28 15:13 ` Paolo Ciarrocchi
0 siblings, 1 reply; 11+ messages in thread
From: Andy Whitcroft @ 2008-01-28 14:56 UTC (permalink / raw)
To: Paolo Ciarrocchi; +Cc: Linux Kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Change a WARN message in checkpatch
2008-01-28 14:56 ` Andy Whitcroft
@ 2008-01-28 15:13 ` Paolo Ciarrocchi
2008-02-23 13:38 ` Paolo Ciarrocchi
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Ciarrocchi @ 2008-01-28 15:13 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Linux Kernel
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/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Change a WARN message in checkpatch
2008-01-28 15:13 ` Paolo Ciarrocchi
@ 2008-02-23 13:38 ` Paolo Ciarrocchi
2008-02-24 15:18 ` Benny Halevy
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Ciarrocchi @ 2008-02-23 13:38 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Linux Kernel
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/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Change a WARN message in checkpatch
2008-02-23 13:38 ` Paolo Ciarrocchi
@ 2008-02-24 15:18 ` Benny Halevy
2008-02-24 18:14 ` Paolo Ciarrocchi
0 siblings, 1 reply; 11+ messages in thread
From: Benny Halevy @ 2008-02-24 15:18 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Paolo Ciarrocchi, Linux Kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Change a WARN message in checkpatch
2008-02-24 15:18 ` Benny Halevy
@ 2008-02-24 18:14 ` Paolo Ciarrocchi
2008-02-25 3:29 ` Andy Whitcroft
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Ciarrocchi @ 2008-02-24 18:14 UTC (permalink / raw)
To: Benny Halevy; +Cc: Andy Whitcroft, Linux Kernel
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/
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Change a WARN message in checkpatch
2008-02-24 18:14 ` Paolo Ciarrocchi
@ 2008-02-25 3:29 ` Andy Whitcroft
2008-02-25 5:47 ` Benny Halevy
2008-02-25 6:51 ` Paolo Ciarrocchi
0 siblings, 2 replies; 11+ messages in thread
From: Andy Whitcroft @ 2008-02-25 3:29 UTC (permalink / raw)
To: Paolo Ciarrocchi; +Cc: Benny Halevy, Linux Kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Change a WARN message in checkpatch
2008-02-25 3:29 ` Andy Whitcroft
@ 2008-02-25 5:47 ` Benny Halevy
2008-02-25 6:51 ` Paolo Ciarrocchi
1 sibling, 0 replies; 11+ messages in thread
From: Benny Halevy @ 2008-02-25 5:47 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Paolo Ciarrocchi, Linux Kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Change a WARN message in checkpatch
2008-02-25 3:29 ` Andy Whitcroft
2008-02-25 5:47 ` Benny Halevy
@ 2008-02-25 6:51 ` Paolo Ciarrocchi
2008-02-25 10:53 ` Andy Whitcroft
1 sibling, 1 reply; 11+ messages in thread
From: Paolo Ciarrocchi @ 2008-02-25 6:51 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Benny Halevy, Linux Kernel
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/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Change a WARN message in checkpatch
2008-02-25 6:51 ` Paolo Ciarrocchi
@ 2008-02-25 10:53 ` Andy Whitcroft
0 siblings, 0 replies; 11+ messages in thread
From: Andy Whitcroft @ 2008-02-25 10:53 UTC (permalink / raw)
To: Paolo Ciarrocchi; +Cc: Benny Halevy, Linux Kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Change a WARN message in checkpatch
@ 2008-01-04 12:45 Paolo Ciarrocchi
0 siblings, 0 replies; 11+ messages in thread
From: Paolo Ciarrocchi @ 2008-01-04 12:45 UTC (permalink / raw)
To: andyw, Linux Kernel
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2008-02-25 10:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-14 22:29 [PATCH] Change a WARN message in checkpatch Paolo Ciarrocchi
2008-01-28 14:56 ` Andy Whitcroft
2008-01-28 15:13 ` Paolo Ciarrocchi
2008-02-23 13:38 ` Paolo Ciarrocchi
2008-02-24 15:18 ` Benny Halevy
2008-02-24 18:14 ` Paolo Ciarrocchi
2008-02-25 3:29 ` Andy Whitcroft
2008-02-25 5:47 ` Benny Halevy
2008-02-25 6:51 ` Paolo Ciarrocchi
2008-02-25 10:53 ` Andy Whitcroft
-- strict thread matches above, loose matches on Subject: below --
2008-01-04 12:45 Paolo Ciarrocchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox