LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] checkpatch.pl: allow piping
       [not found] <20080111041120.085610726@mvista.com>
@ 2008-01-11  4:11 ` Daniel Walker
  2008-01-11  8:52 ` Jiri Slaby
  1 sibling, 0 replies; 25+ messages in thread
From: Daniel Walker @ 2008-01-11  4:11 UTC (permalink / raw)
  To: apw; +Cc: linux-kernel, rdunlap, jschopp

A little feature addition to allow checkpatch.pl to check patches piped
into it, in addition to specific file arguments.

Signed-off-by: Daniel Walker <dwalker@mvista.com>
---
 scripts/checkpatch.pl |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Index: linux-2.6.23/scripts/checkpatch.pl
===================================================================
--- linux-2.6.23.orig/scripts/checkpatch.pl
+++ linux-2.6.23/scripts/checkpatch.pl
@@ -24,6 +24,7 @@ my $file = 0;
 my $check = 0;
 my $summary = 1;
 my $mailback = 0;
+my $piped = (-t STDIN) ? 0 : 1;
 my $root;
 GetOptions(
 	'q|quiet+'	=> \$quiet,
@@ -43,7 +44,7 @@ GetOptions(
 
 my $exit = 0;
 
-if ($#ARGV < 0) {
+if ($#ARGV < 0 && !$piped) {
 	print "usage: $P [options] patchfile\n";
 	print "version: $V\n";
 	print "options: -q           => quiet\n";
@@ -181,6 +182,18 @@ if ($tree && -f "$root/$removal") {
 }
 
 my @rawlines = ();
+
+if ($piped) {
+	while (<STDIN>) {
+		chomp;
+		push(@rawlines, $_);
+	}
+	if (!process("", @rawlines)) {
+		$exit = 1;
+	}
+	@rawlines = ();
+}
+
 for my $filename (@ARGV) {
 	if ($file) {
 		open(FILE, "diff -u /dev/null $filename|") ||
-- 

-- 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
       [not found] <20080111041120.085610726@mvista.com>
  2008-01-11  4:11 ` [PATCH] checkpatch.pl: allow piping Daniel Walker
@ 2008-01-11  8:52 ` Jiri Slaby
  2008-01-11  9:17   ` Daniel Walker
  1 sibling, 1 reply; 25+ messages in thread
From: Jiri Slaby @ 2008-01-11  8:52 UTC (permalink / raw)
  To: Daniel Walker; +Cc: apw, linux-kernel, rdunlap, jschopp

On 01/11/2008 05:10 AM, Daniel Walker wrote:
> A little feature addition to allow checkpatch.pl to check patches piped
> into it, in addition to specific file arguments.

You can still add - as an argument to check stdin. In which way is this better?

regards,
-- 
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  8:52 ` Jiri Slaby
@ 2008-01-11  9:17   ` Daniel Walker
  2008-01-11  9:21     ` Jiri Slaby
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Walker @ 2008-01-11  9:17 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: apw, linux-kernel, rdunlap, jschopp


On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> On 01/11/2008 05:10 AM, Daniel Walker wrote:
> > A little feature addition to allow checkpatch.pl to check patches piped
> > into it, in addition to specific file arguments.
> 
> You can still add - as an argument to check stdin. In which way is this better?

There's was no reason to limit the arguments ..

I was using it to do something like the following ,

git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl

Which I think is useful .

Dainel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:17   ` Daniel Walker
@ 2008-01-11  9:21     ` Jiri Slaby
  2008-01-11  9:23       ` Bernd Petrovitsch
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Slaby @ 2008-01-11  9:21 UTC (permalink / raw)
  To: Daniel Walker; +Cc: apw, linux-kernel, rdunlap, jschopp

On 01/11/2008 10:17 AM, Daniel Walker wrote:
> On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
>> On 01/11/2008 05:10 AM, Daniel Walker wrote:
>>> A little feature addition to allow checkpatch.pl to check patches piped
>>> into it, in addition to specific file arguments.
>> You can still add - as an argument to check stdin. In which way is this better?
> 
> There's was no reason to limit the arguments ..
> 
> I was using it to do something like the following ,
> 
> git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl

Ok, and if you add a - there, it should have the same effect, but for free, 
doesn't it:
git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl -

regards,
-- 
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:21     ` Jiri Slaby
@ 2008-01-11  9:23       ` Bernd Petrovitsch
  2008-01-11  9:30         ` Daniel Walker
  0 siblings, 1 reply; 25+ messages in thread
From: Bernd Petrovitsch @ 2008-01-11  9:23 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Daniel Walker, apw, linux-kernel, rdunlap, jschopp

On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> On 01/11/2008 10:17 AM, Daniel Walker wrote:
> > On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> >> On 01/11/2008 05:10 AM, Daniel Walker wrote:
> >>> A little feature addition to allow checkpatch.pl to check patches piped
> >>> into it, in addition to specific file arguments.
> >> You can still add - as an argument to check stdin. In which way is this better?
> > 
> > There's was no reason to limit the arguments ..
> > 
> > I was using it to do something like the following ,
> > 
> > git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl
> 
> Ok, and if you add a - there, it should have the same effect, but for free, 
> doesn't it:
> git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl -

JftSoC:
git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl /dev/stdin
(and there a a few others) should also do the trick.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:23       ` Bernd Petrovitsch
@ 2008-01-11  9:30         ` Daniel Walker
  2008-01-11  9:34           ` Jiri Slaby
  2008-01-11 10:08           ` [PATCH] checkpatch.pl: allow piping Bernd Petrovitsch
  0 siblings, 2 replies; 25+ messages in thread
From: Daniel Walker @ 2008-01-11  9:30 UTC (permalink / raw)
  To: Bernd Petrovitsch; +Cc: Jiri Slaby, apw, linux-kernel, rdunlap, jschopp


On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> > On 01/11/2008 10:17 AM, Daniel Walker wrote:
> > > On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> > >> On 01/11/2008 05:10 AM, Daniel Walker wrote:
> > >>> A little feature addition to allow checkpatch.pl to check
> patches piped
> > >>> into it, in addition to specific file arguments.
> > >> You can still add - as an argument to check stdin. In which way
> is this better?
> > > 
> > > There's was no reason to limit the arguments ..
> > > 
> > > I was using it to do something like the following ,
> > > 
> > > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> | ./scripts/checkpatch.pl
> > 
> > Ok, and if you add a - there, it should have the same effect, but
> for free, 
> > doesn't it:
> > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> | ./scripts/checkpatch.pl -
> 
> JftSoC:
> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> | ./scripts/checkpatch.pl /dev/stdin
> (and there a a few others) should also do the trick.

Not a particularly attractive command line .. Might still be a good idea
to add this since these two forms alluded me, and are likely to allude
people new to unix all together (who is more likely to be using this
particular tool)..

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:30         ` Daniel Walker
@ 2008-01-11  9:34           ` Jiri Slaby
  2008-01-11  9:36             ` Daniel Walker
  2008-01-11 10:08           ` [PATCH] checkpatch.pl: allow piping Bernd Petrovitsch
  1 sibling, 1 reply; 25+ messages in thread
From: Jiri Slaby @ 2008-01-11  9:34 UTC (permalink / raw)
  To: Daniel Walker; +Cc: Bernd Petrovitsch, apw, linux-kernel, rdunlap, jschopp

On 01/11/2008 10:30 AM, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
>> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
>>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>> | ./scripts/checkpatch.pl -
>>
>> JftSoC:
>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>> | ./scripts/checkpatch.pl /dev/stdin
>> (and there a a few others) should also do the trick.
> 
> Not a particularly attractive command line .. Might still be a good idea
> to add this since these two forms alluded me, and are likely to allude
> people new to unix all together (who is more likely to be using this
> particular tool)..

If somebody is hacking kernel, I think he should know the - trick used in many 
programs, but do not consider this as a nack.

thanks,
-- 
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:34           ` Jiri Slaby
@ 2008-01-11  9:36             ` Daniel Walker
  2008-01-11  9:41               ` Jiri Slaby
  2008-01-11 11:16               ` Stefan Richter
  0 siblings, 2 replies; 25+ messages in thread
From: Daniel Walker @ 2008-01-11  9:36 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Bernd Petrovitsch, apw, linux-kernel, rdunlap, jschopp


On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
> On 01/11/2008 10:30 AM, Daniel Walker wrote:
> > On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
> >> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> >>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> >> | ./scripts/checkpatch.pl -
> >>
> >> JftSoC:
> >> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> >> | ./scripts/checkpatch.pl /dev/stdin
> >> (and there a a few others) should also do the trick.
> > 
> > Not a particularly attractive command line .. Might still be a good idea
> > to add this since these two forms alluded me, and are likely to allude
> > people new to unix all together (who is more likely to be using this
> > particular tool)..
> 
> If somebody is hacking kernel, I think he should know the - trick used in many 
> programs, but do not consider this as a nack.

I'm hacking the kernel, and I didn't know the - trick .. So you have
your testing case all in one with the patch submitter ..

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:36             ` Daniel Walker
@ 2008-01-11  9:41               ` Jiri Slaby
  2008-01-11  9:47                 ` Daniel Walker
  2008-01-11 11:16               ` Stefan Richter
  1 sibling, 1 reply; 25+ messages in thread
From: Jiri Slaby @ 2008-01-11  9:41 UTC (permalink / raw)
  To: Daniel Walker; +Cc: Bernd Petrovitsch, apw, linux-kernel, rdunlap, jschopp

On 01/11/2008 10:36 AM, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
>> If somebody is hacking kernel, I think he should know the - trick used in many 
>> programs, but do not consider this as a nack.
> 
> I'm hacking the kernel, and I didn't know the - trick .. So you have
> your testing case all in one with the patch submitter ..

OK, maybe we should base it in doc for patch submitting then?

regards,
-- 
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:41               ` Jiri Slaby
@ 2008-01-11  9:47                 ` Daniel Walker
  2008-01-11 10:11                   ` Bernd Petrovitsch
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Walker @ 2008-01-11  9:47 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Bernd Petrovitsch, apw, linux-kernel, rdunlap, jschopp


On Fri, 2008-01-11 at 10:41 +0100, Jiri Slaby wrote:
> On 01/11/2008 10:36 AM, Daniel Walker wrote:
> > On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
> >> If somebody is hacking kernel, I think he should know the - trick used in many 
> >> programs, but do not consider this as a nack.
> > 
> > I'm hacking the kernel, and I didn't know the - trick .. So you have
> > your testing case all in one with the patch submitter ..
> 
> OK, maybe we should base it in doc for patch submitting then?

It's in the bash docs I'm sure, it's just a matter of who ends up
reading that part of the docs.. I still think my patch is a good one ,
we could change the name to "Allow piping with out tricks" I suppose ..

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:30         ` Daniel Walker
  2008-01-11  9:34           ` Jiri Slaby
@ 2008-01-11 10:08           ` Bernd Petrovitsch
  1 sibling, 0 replies; 25+ messages in thread
From: Bernd Petrovitsch @ 2008-01-11 10:08 UTC (permalink / raw)
  To: Daniel Walker; +Cc: Jiri Slaby, apw, linux-kernel, rdunlap, jschopp

On Fre, 2008-01-11 at 01:30 -0800, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
> > On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> > > On 01/11/2008 10:17 AM, Daniel Walker wrote:
> > > > On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> > > >> On 01/11/2008 05:10 AM, Daniel Walker wrote:
[...]
> > > > I was using it to do something like the following ,
> > > > 
> > > > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> > | ./scripts/checkpatch.pl
> > > 
> > > Ok, and if you add a - there, it should have the same effect, but
> > for free, 
> > > doesn't it:
> > > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> > | ./scripts/checkpatch.pl -
> > 
> > JftSoC:
> > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> > | ./scripts/checkpatch.pl /dev/stdin
> > (and there a a few others) should also do the trick.
> 
> Not a particularly attractive command line .. Might still be a good idea

Might be.
But it works for (almost?) all programs (which do not need seekable
input) which absolutely want the input file specified by a parameter.
And if it's an proprietary program, you can't even patch it.

> to add this since these two forms alluded me, and are likely to allude
> people new to unix all together (who is more likely to be using this
> particular tool)..

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:47                 ` Daniel Walker
@ 2008-01-11 10:11                   ` Bernd Petrovitsch
  0 siblings, 0 replies; 25+ messages in thread
From: Bernd Petrovitsch @ 2008-01-11 10:11 UTC (permalink / raw)
  To: Daniel Walker; +Cc: Jiri Slaby, apw, linux-kernel, rdunlap, jschopp

On Fre, 2008-01-11 at 01:47 -0800, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:41 +0100, Jiri Slaby wrote:
> > On 01/11/2008 10:36 AM, Daniel Walker wrote:
> > > On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
> > >> If somebody is hacking kernel, I think he should know the - trick used in many 
> > >> programs, but do not consider this as a nack.
> > > 
> > > I'm hacking the kernel, and I didn't know the - trick .. So you have
> > > your testing case all in one with the patch submitter ..
> > 
> > OK, maybe we should base it in doc for patch submitting then?
> 
> It's in the bash docs I'm sure, it's just a matter of who ends up

Yes, too. And "awk" also supports that.
But "/dev/stdin" is also since ages a symlink to
"/proc/self/fd/0" (which is another equivalent "file").

> reading that part of the docs.. I still think my patch is a good one ,
> we could change the name to "Allow piping with out tricks" I suppose ..

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11  9:36             ` Daniel Walker
  2008-01-11  9:41               ` Jiri Slaby
@ 2008-01-11 11:16               ` Stefan Richter
  2008-01-11 11:50                 ` Jiri Slaby
  1 sibling, 1 reply; 25+ messages in thread
From: Stefan Richter @ 2008-01-11 11:16 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Jiri Slaby, Bernd Petrovitsch, apw, linux-kernel, rdunlap, jschopp

Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
>> >> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
>> >>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>> >> | ./scripts/checkpatch.pl -

>> If somebody is hacking kernel, I think he should know the - trick used in many 
>> programs, but do not consider this as a nack.
> 
> I'm hacking the kernel, and I didn't know the - trick .. So you have
> your testing case all in one with the patch submitter ..

How about

 if ($#ARGV < 0) {
 	print "usage: $P [options] patchfile\n";
 	print "version: $V\n";
 	print "options: -q           => quiet\n";
 	print "         --no-tree    => run without a kernel tree\n";
 	print "         --terse      => one line per report\n";
 	print "         --emacs      => emacs compile window format\n";
 	print "         --file       => check a source file\n";
 	print "         --strict     => enable more subjective tests\n";
 	print "         --root       => path to the kernel tree root\n";
+	print "When patchfile is -, read standard input.\n";
 	exit(1);
 }

-- 
Stefan Richter
-=====-==--- ---= -=-==
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: allow piping
  2008-01-11 11:16               ` Stefan Richter
@ 2008-01-11 11:50                 ` Jiri Slaby
  2008-01-11 17:06                   ` [PATCH] checkpatch.pl: show how to read from stdin Stefan Richter
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Slaby @ 2008-01-11 11:50 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Daniel Walker, Bernd Petrovitsch, apw, linux-kernel, rdunlap, jschopp

On 01/11/2008 12:16 PM, Stefan Richter wrote:
> Daniel Walker wrote:
>> On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
>>>>> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
>>>>>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>>>>> | ./scripts/checkpatch.pl -
> 
>>> If somebody is hacking kernel, I think he should know the - trick used in many 
>>> programs, but do not consider this as a nack.
>> I'm hacking the kernel, and I didn't know the - trick .. So you have
>> your testing case all in one with the patch submitter ..
> 
> How about
> 
>  if ($#ARGV < 0) {
>  	print "usage: $P [options] patchfile\n";
>  	print "version: $V\n";
>  	print "options: -q           => quiet\n";
>  	print "         --no-tree    => run without a kernel tree\n";
>  	print "         --terse      => one line per report\n";
>  	print "         --emacs      => emacs compile window format\n";
>  	print "         --file       => check a source file\n";
>  	print "         --strict     => enable more subjective tests\n";
>  	print "         --root       => path to the kernel tree root\n";
> +	print "When patchfile is -, read standard input.\n";
>  	exit(1);
>  }
> 

My ACK.

regards,
-- 
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-11 11:50                 ` Jiri Slaby
@ 2008-01-11 17:06                   ` Stefan Richter
  2008-01-11 17:09                     ` Daniel Walker
  2008-01-14 17:17                     ` Andy Whitcroft
  0 siblings, 2 replies; 25+ messages in thread
From: Stefan Richter @ 2008-01-11 17:06 UTC (permalink / raw)
  To: Andy Whitcroft
  Cc: Daniel Walker, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Acked-by: Jiri Slaby <jirislaby@gmail.com>
---
 scripts/checkpatch.pl |    1 +
 1 file changed, 1 insertion(+)

Index: linux/scripts/checkpatch.pl
===================================================================
--- linux.orig/scripts/checkpatch.pl
+++ linux/scripts/checkpatch.pl
@@ -53,6 +53,7 @@ if ($#ARGV < 0) {
 	print "         --file       => check a source file\n";
 	print "         --strict     => enable more subjective tests\n";
 	print "         --root       => path to the kernel tree root\n";
+	print "When patchfile is -, read standard input.\n";
 	exit(1);
 }
 

-- 
Stefan Richter
-=====-==--- ---= -=-==
http://arcgraph.de/sr/


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-11 17:06                   ` [PATCH] checkpatch.pl: show how to read from stdin Stefan Richter
@ 2008-01-11 17:09                     ` Daniel Walker
  2008-01-11 17:29                       ` Stefan Richter
  2008-01-14 17:17                     ` Andy Whitcroft
  1 sibling, 1 reply; 25+ messages in thread
From: Daniel Walker @ 2008-01-11 17:09 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Andy Whitcroft, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby


On Fri, 2008-01-11 at 18:06 +0100, Stefan Richter wrote:
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> Acked-by: Jiri Slaby <jirislaby@gmail.com>
> ---
>  scripts/checkpatch.pl |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux/scripts/checkpatch.pl
> ===================================================================
> --- linux.orig/scripts/checkpatch.pl
> +++ linux/scripts/checkpatch.pl
> @@ -53,6 +53,7 @@ if ($#ARGV < 0) {
>  	print "         --file       => check a source file\n";
>  	print "         --strict     => enable more subjective tests\n";
>  	print "         --root       => path to the kernel tree root\n";
> +	print "When patchfile is -, read standard input.\n";
>  	exit(1);
>  }
>  

Naww .. Why add documentation when you can just make it do the right
thing ..

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-11 17:09                     ` Daniel Walker
@ 2008-01-11 17:29                       ` Stefan Richter
  2008-01-11 17:39                         ` Daniel Walker
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Richter @ 2008-01-11 17:29 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Andy Whitcroft, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby

Daniel Walker wrote:
> On Fri, 2008-01-11 at 18:06 +0100, Stefan Richter wrote:
>> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
>> +	print "When patchfile is -, read standard input.\n";

> Naww .. Why add documentation when you can just make it do the right
> thing ..

Why add code for a feature which already exists?
-- 
Stefan Richter
-=====-==--- ---= -=-==
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-11 17:29                       ` Stefan Richter
@ 2008-01-11 17:39                         ` Daniel Walker
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Walker @ 2008-01-11 17:39 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Andy Whitcroft, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby


On Fri, 2008-01-11 at 18:29 +0100, Stefan Richter wrote:
> Daniel Walker wrote:
> > On Fri, 2008-01-11 at 18:06 +0100, Stefan Richter wrote:
> >> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> >> +	print "When patchfile is -, read standard input.\n";
> 
> > Naww .. Why add documentation when you can just make it do the right
> > thing ..
> 
> Why add code for a feature which already exists?

To make it simpler to use .. A good percentage (if not all) command line
unix data processing utils accept piped data directly, without the need
for the "-" .. That's the expect usage .. checkpatch.pl doesn't need to
deviate from the norm, there is no size constraint, there is no line
limit .. So we're not duplicating functionality, we're making the tool
conform ..

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-11 17:06                   ` [PATCH] checkpatch.pl: show how to read from stdin Stefan Richter
  2008-01-11 17:09                     ` Daniel Walker
@ 2008-01-14 17:17                     ` Andy Whitcroft
  2008-01-14 17:35                       ` Daniel Walker
  1 sibling, 1 reply; 25+ messages in thread
From: Andy Whitcroft @ 2008-01-14 17:17 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Daniel Walker, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby

On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote:
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> Acked-by: Jiri Slaby <jirislaby@gmail.com>

As an absolute minimum this seems reasonable to me.  I guess we could
make no arguments default to '-' also.  There are up and downsides to
doing that, as currently no arguments currently tell you the usage and
with this patch would point clearly out the '-' option.  Just assuming
stding would lose easy access to usage, which may actually be more
confusing for the beginner.  Hmmm.  Cirtainly will include this
documentation change if nothing else.

> ---
>  scripts/checkpatch.pl |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux/scripts/checkpatch.pl
> ===================================================================
> --- linux.orig/scripts/checkpatch.pl
> +++ linux/scripts/checkpatch.pl
> @@ -53,6 +53,7 @@ if ($#ARGV < 0) {
>  	print "         --file       => check a source file\n";
>  	print "         --strict     => enable more subjective tests\n";
>  	print "         --root       => path to the kernel tree root\n";
> +	print "When patchfile is -, read standard input.\n";
>  	exit(1);
>  }

-apw

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-14 17:17                     ` Andy Whitcroft
@ 2008-01-14 17:35                       ` Daniel Walker
  2008-01-14 19:12                         ` Andy Whitcroft
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Walker @ 2008-01-14 17:35 UTC (permalink / raw)
  To: Andy Whitcroft
  Cc: Stefan Richter, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby


On Mon, 2008-01-14 at 17:17 +0000, Andy Whitcroft wrote:
> On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote:
> > Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> > Acked-by: Jiri Slaby <jirislaby@gmail.com>
> 
> As an absolute minimum this seems reasonable to me.  I guess we could
> make no arguments default to '-' also.  There are up and downsides to
> doing that, as currently no arguments currently tell you the usage and
> with this patch would point clearly out the '-' option.  Just assuming
> stding would lose easy access to usage, which may actually be more
> confusing for the beginner.  Hmmm.  Cirtainly will include this
> documentation change if nothing else.
> 

The patch that I submitted checks STDIN for piped data, and if there is
any it will default to checking that incoming data .. That's regardless
of the number of arguments given .. 

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-14 17:35                       ` Daniel Walker
@ 2008-01-14 19:12                         ` Andy Whitcroft
  2008-01-14 19:17                           ` Daniel Walker
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Whitcroft @ 2008-01-14 19:12 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Stefan Richter, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby

On Mon, Jan 14, 2008 at 09:35:15AM -0800, Daniel Walker wrote:
> 
> On Mon, 2008-01-14 at 17:17 +0000, Andy Whitcroft wrote:
> > On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote:
> > > Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> > > Acked-by: Jiri Slaby <jirislaby@gmail.com>
> > 
> > As an absolute minimum this seems reasonable to me.  I guess we could
> > make no arguments default to '-' also.  There are up and downsides to
> > doing that, as currently no arguments currently tell you the usage and
> > with this patch would point clearly out the '-' option.  Just assuming
> > stding would lose easy access to usage, which may actually be more
> > confusing for the beginner.  Hmmm.  Cirtainly will include this
> > documentation change if nothing else.
> > 
> 
> The patch that I submitted checks STDIN for piped data, and if there is
> any it will default to checking that incoming data .. That's regardless
> of the number of arguments given .. 

So it does, however that of itself differs from the unix norm; as with
this I cannot run checkpatch and "type" (ie paste) a patch fragment to
check it.  So I don't think we want the semantics as you have there,
as its confusing to the experienced user and inconsistent with the norm.
Either we should document the standard '-' usage as has been suggested
elsewhere or always assume stdin with no parameters.

-apw

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-14 19:12                         ` Andy Whitcroft
@ 2008-01-14 19:17                           ` Daniel Walker
  2008-01-14 19:31                             ` Stefan Richter
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Walker @ 2008-01-14 19:17 UTC (permalink / raw)
  To: Andy Whitcroft
  Cc: Stefan Richter, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby


On Mon, 2008-01-14 at 19:12 +0000, Andy Whitcroft wrote:
> So it does, however that of itself differs from the unix norm; as with
> this I cannot run checkpatch and "type" (ie paste) a patch fragment to
> check it.  So I don't think we want the semantics as you have there,
> as its confusing to the experienced user and inconsistent with the
> norm.
> Either we should document the standard '-' usage as has been suggested
> elsewhere or always assume stdin with no parameters.

I'm not sure I understand what you mean .. Can you give an example?

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-14 19:17                           ` Daniel Walker
@ 2008-01-14 19:31                             ` Stefan Richter
  2008-01-14 19:44                               ` Jiri Slaby
  2008-01-14 19:51                               ` Daniel Walker
  0 siblings, 2 replies; 25+ messages in thread
From: Stefan Richter @ 2008-01-14 19:31 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Andy Whitcroft, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby

Daniel Walker wrote:
> On Mon, 2008-01-14 at 19:12 +0000, Andy Whitcroft wrote:
>> with this I cannot run checkpatch and "type" (ie paste) a patch
>> fragment to check it.
...
> I'm not sure I understand what you mean .. Can you give an example?

AFAIU Daniel's patch still leaves the possibility to use the '-' syntax,
doesn't it?

(The program 'cat' is of the kind which always reads from stdin if no
file name is given, or if '-' is given instead of a file name.  So,
'cat' allows Andy to start it and then type something in for cat to
process in either case, while checkpatch supports this only with '-' as
argument.  OTOH it's easier to get checkpatch to print a usage note than
it is with cat.  There you have to type no less than 'cat --help', or at
least with GNU cat.)
-- 
Stefan Richter
-=====-==--- ---= -===-
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-14 19:31                             ` Stefan Richter
@ 2008-01-14 19:44                               ` Jiri Slaby
  2008-01-14 19:51                               ` Daniel Walker
  1 sibling, 0 replies; 25+ messages in thread
From: Jiri Slaby @ 2008-01-14 19:44 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Daniel Walker, Andy Whitcroft, Bernd Petrovitsch, linux-kernel,
	Randy Dunlap, Joel Schopp

On 01/14/2008 08:31 PM, Stefan Richter wrote:
> it is with cat.  There you have to type no less than 'cat --help', or at
> least with GNU cat.)

--h (--he --hel --help) suffices due to getopt_long gnu implementation

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] checkpatch.pl: show how to read from stdin
  2008-01-14 19:31                             ` Stefan Richter
  2008-01-14 19:44                               ` Jiri Slaby
@ 2008-01-14 19:51                               ` Daniel Walker
  1 sibling, 0 replies; 25+ messages in thread
From: Daniel Walker @ 2008-01-14 19:51 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Andy Whitcroft, Bernd Petrovitsch, linux-kernel, Randy Dunlap,
	Joel Schopp, Jiri Slaby


On Mon, 2008-01-14 at 20:31 +0100, Stefan Richter wrote:
> AFAIU Daniel's patch still leaves the possibility to use the '-'
> syntax,
> doesn't it?
> 
> (The program 'cat' is of the kind which always reads from stdin if no
> file name is given, or if '-' is given instead of a file name.  So,
> 'cat' allows Andy to start it and then type something in for cat to
> process in either case, while checkpatch supports this only with '-'
> as
> argument.  OTOH it's easier to get checkpatch to print a usage note
> than
> it is with cat.  There you have to type no less than 'cat --help', or
> at
> least with GNU cat.)

I tried the following with my changes,

./scripts/checkpatch.pl -
cat | ./scripts/checkpatch.pl

Both allowed pasting patches and pressing Ctrl-D , then it processed the
patch..

and

cat <patchname> | ./scripts/checkpatch.pl

Also processed the patch ..

Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2008-01-14 19:54 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20080111041120.085610726@mvista.com>
2008-01-11  4:11 ` [PATCH] checkpatch.pl: allow piping Daniel Walker
2008-01-11  8:52 ` Jiri Slaby
2008-01-11  9:17   ` Daniel Walker
2008-01-11  9:21     ` Jiri Slaby
2008-01-11  9:23       ` Bernd Petrovitsch
2008-01-11  9:30         ` Daniel Walker
2008-01-11  9:34           ` Jiri Slaby
2008-01-11  9:36             ` Daniel Walker
2008-01-11  9:41               ` Jiri Slaby
2008-01-11  9:47                 ` Daniel Walker
2008-01-11 10:11                   ` Bernd Petrovitsch
2008-01-11 11:16               ` Stefan Richter
2008-01-11 11:50                 ` Jiri Slaby
2008-01-11 17:06                   ` [PATCH] checkpatch.pl: show how to read from stdin Stefan Richter
2008-01-11 17:09                     ` Daniel Walker
2008-01-11 17:29                       ` Stefan Richter
2008-01-11 17:39                         ` Daniel Walker
2008-01-14 17:17                     ` Andy Whitcroft
2008-01-14 17:35                       ` Daniel Walker
2008-01-14 19:12                         ` Andy Whitcroft
2008-01-14 19:17                           ` Daniel Walker
2008-01-14 19:31                             ` Stefan Richter
2008-01-14 19:44                               ` Jiri Slaby
2008-01-14 19:51                               ` Daniel Walker
2008-01-11 10:08           ` [PATCH] checkpatch.pl: allow piping Bernd Petrovitsch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).