[PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-16 Thread David Bremner
"W. Trevor King"  writes:

>
> There should be standard Perl syntax for capturing both streams ;).  I
> can add a separate parameter instead, but Perl doesn't seem to have
> keyword-arguments.  At least, nmbug relies on assumptions about
> argument values:
>
>   sub git_pipe {
> my $envref = (ref $_[0] eq 'HASH') ? shift : {};
> my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
> my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : undef;
> ?
>   }
>

Perl supports keyword arguments fine, by passing a single hash 
to the subroutine. This particular sub doesn't do things that way, but
it's only called in three places, so it would be simple enough to change
the calling convention to to

git_pipe (env => {}, lines=>[], 
 command => [  qw/diff-index --cached/,
 "--diff-filter=$filter", qw/--name-only HEAD/ ],
 redirect => [ whatever you want here ] 
 )


>  I'd be happy to rewrite nmbug in Python too, which has (to my eyes)
> much saner kwargs and subprocess handling.

Although I think that's an extreme answer to this particular issue ;), I
do think it's worth discussing. In particular I think it would be
reasonable to consider an updated nmbug that used the python bindings to
notmuch, for possibly more speed. Of the people that currently work on
nmbug (I count myself, yourself, and Tomi), I think we're all ok with
python.  I know at least Jani mentioned he'd be more interested in
contributing to a python version.

d



Re: [PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-16 Thread David Bremner
W. Trevor King wk...@tremily.us writes:


 There should be standard Perl syntax for capturing both streams ;).  I
 can add a separate parameter instead, but Perl doesn't seem to have
 keyword-arguments.  At least, nmbug relies on assumptions about
 argument values:

   sub git_pipe {
 my $envref = (ref $_[0] eq 'HASH') ? shift : {};
 my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
 my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : undef;
 …
   }


Perl supports keyword arguments fine, by passing a single hash 
to the subroutine. This particular sub doesn't do things that way, but
it's only called in three places, so it would be simple enough to change
the calling convention to to

git_pipe (env = {}, lines=[], 
 command = [  qw/diff-index --cached/,
 --diff-filter=$filter, qw/--name-only HEAD/ ],
 redirect = [ whatever you want here ] 
 )


  I'd be happy to rewrite nmbug in Python too, which has (to my eyes)
 much saner kwargs and subprocess handling.

Although I think that's an extreme answer to this particular issue ;), I
do think it's worth discussing. In particular I think it would be
reasonable to consider an updated nmbug that used the python bindings to
notmuch, for possibly more speed. Of the people that currently work on
nmbug (I count myself, yourself, and Tomi), I think we're all ok with
python.  I know at least Jani mentioned he'd be more interested in
contributing to a python version.

d

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-15 Thread David Bremner
"W. Trevor King"  writes:

> +  if ($dir eq '-2|') {
> +$dir = '-|';
> +  }
> +

I think I'd prefer an extra flag, rather than making new syntax.
The existing syntax is not pretty, but it is standard perl



[PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-15 Thread W. Trevor King
On Tue, Jul 15, 2014 at 08:49:58PM -0300, David Bremner wrote:
> "W. Trevor King"  writes:
> 
> > +  if ($dir eq '-2|') {
> > +$dir = '-|';
> > +  }
> > +
> 
> I think I'd prefer an extra flag, rather than making new syntax.
> The existing syntax is not pretty, but it is standard perl

There should be standard Perl syntax for capturing both streams ;).  I
can add a separate parameter instead, but Perl doesn't seem to have
keyword-arguments.  At least, nmbug relies on assumptions about
argument values:

  sub git_pipe {
my $envref = (ref $_[0] eq 'HASH') ? shift : {};
my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : undef;
?
  }

to map positional arguments back into particular semantic meanings.
That seemed ugly enough (can we only have a single boolean argument
with this pattern?) that I'd prefer piggy-backing on Perl's
stream-direction syntax.  But let me know what you like best, and I'll
go that way with v2.  I'd be happy to rewrite nmbug in Python too,
which has (to my eyes) much saner kwargs and subprocess handling.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 



Re: [PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-15 Thread David Bremner
W. Trevor King wk...@tremily.us writes:

 +  if ($dir eq '-2|') {
 +$dir = '-|';
 +  }
 +

I think I'd prefer an extra flag, rather than making new syntax.
The existing syntax is not pretty, but it is standard perl

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-15 Thread W. Trevor King
On Tue, Jul 15, 2014 at 08:49:58PM -0300, David Bremner wrote:
 W. Trevor King wk...@tremily.us writes:
 
  +  if ($dir eq '-2|') {
  +$dir = '-|';
  +  }
  +
 
 I think I'd prefer an extra flag, rather than making new syntax.
 The existing syntax is not pretty, but it is standard perl

There should be standard Perl syntax for capturing both streams ;).  I
can add a separate parameter instead, but Perl doesn't seem to have
keyword-arguments.  At least, nmbug relies on assumptions about
argument values:

  sub git_pipe {
my $envref = (ref $_[0] eq 'HASH') ? shift : {};
my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : undef;
…
  }

to map positional arguments back into particular semantic meanings.
That seemed ugly enough (can we only have a single boolean argument
with this pattern?) that I'd prefer piggy-backing on Perl's
stream-direction syntax.  But let me know what you like best, and I'll
go that way with v2.  I'd be happy to rewrite nmbug in Python too,
which has (to my eyes) much saner kwargs and subprocess handling.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-06 Thread W. Trevor King
Add a '-2|' dir for "execute the command and pipe both stdout and
stderr to us".  Use this to catch stderr from the rev-parse call in
is_unmerged.  We already check the status, so we don't want to confuse
users with stuff like:

  error: No upstream configured for branch 'master'

on nmbug's stderr.
---
 devel/nmbug/nmbug | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
index 998ee6b..c9ac046 100755
--- a/devel/nmbug/nmbug
+++ b/devel/nmbug/nmbug
@@ -56,7 +56,7 @@ my $EMPTYBLOB = git (qw{hash-object -t blob /dev/null});
 sub git_pipe {
   my $envref = (ref $_[0] eq 'HASH') ? shift : {};
   my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
-  my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : undef;
+  my $dir = ($_[0] eq '-|' or $_[0] eq '-2|' or $_[0] eq '|-') ? shift : undef;

   unshift @_, 'git';
   $envref->{GIT_DIR} ||= $NMBGIT;
@@ -83,10 +83,15 @@ sub git {
 sub spawn {
   my $envref = (ref $_[0] eq 'HASH') ? shift : {};
   my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
-  my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : '-|';
+  my $dir = ($_[0] eq '-|' or $_[0] eq '-2|' or $_[0] eq '|-') ? shift : '-|';
+  my $stderr_to_stdout = $dir eq '-2|';

   die unless @_;

+  if ($dir eq '-2|') {
+$dir = '-|';
+  }
+
   if (open my $child, $dir) {
 return $child;
   }
@@ -105,6 +110,9 @@ sub spawn {
   if ($dir ne '|-') {
open STDIN, '<', '/dev/null' or die "reopening stdin: $!"
   }
+  if ($stderr_to_stdout) {
+   open(STDERR, ">");
+  }
   exec @_;
   die "exec @_: $!";
 }
@@ -430,7 +438,7 @@ sub do_status {
 sub is_unmerged {
   my $commit = shift || '@{upstream}';

-  my ($fetch_head, $status) = git_with_status ('rev-parse', $commit);
+  my ($fetch_head, $status) = git_with_status ('-2|', 'rev-parse', $commit);
   if ($status) {
 return 0;
   }
-- 
1.9.1.353.gc66d89d



[PATCH 3/4] nmbug: Catch stderr in is_unmerged

2014-07-06 Thread W. Trevor King
Add a '-2|' dir for execute the command and pipe both stdout and
stderr to us.  Use this to catch stderr from the rev-parse call in
is_unmerged.  We already check the status, so we don't want to confuse
users with stuff like:

  error: No upstream configured for branch 'master'

on nmbug's stderr.
---
 devel/nmbug/nmbug | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
index 998ee6b..c9ac046 100755
--- a/devel/nmbug/nmbug
+++ b/devel/nmbug/nmbug
@@ -56,7 +56,7 @@ my $EMPTYBLOB = git (qw{hash-object -t blob /dev/null});
 sub git_pipe {
   my $envref = (ref $_[0] eq 'HASH') ? shift : {};
   my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
-  my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : undef;
+  my $dir = ($_[0] eq '-|' or $_[0] eq '-2|' or $_[0] eq '|-') ? shift : undef;
 
   unshift @_, 'git';
   $envref-{GIT_DIR} ||= $NMBGIT;
@@ -83,10 +83,15 @@ sub git {
 sub spawn {
   my $envref = (ref $_[0] eq 'HASH') ? shift : {};
   my $ioref  = (ref $_[0] eq 'ARRAY') ? shift : undef;
-  my $dir = ($_[0] eq '-|' or $_[0] eq '|-') ? shift : '-|';
+  my $dir = ($_[0] eq '-|' or $_[0] eq '-2|' or $_[0] eq '|-') ? shift : '-|';
+  my $stderr_to_stdout = $dir eq '-2|';
 
   die unless @_;
 
+  if ($dir eq '-2|') {
+$dir = '-|';
+  }
+
   if (open my $child, $dir) {
 return $child;
   }
@@ -105,6 +110,9 @@ sub spawn {
   if ($dir ne '|-') {
open STDIN, '', '/dev/null' or die reopening stdin: $!
   }
+  if ($stderr_to_stdout) {
+   open(STDERR, STDOUT);
+  }
   exec @_;
   die exec @_: $!;
 }
@@ -430,7 +438,7 @@ sub do_status {
 sub is_unmerged {
   my $commit = shift || '@{upstream}';
 
-  my ($fetch_head, $status) = git_with_status ('rev-parse', $commit);
+  my ($fetch_head, $status) = git_with_status ('-2|', 'rev-parse', $commit);
   if ($status) {
 return 0;
   }
-- 
1.9.1.353.gc66d89d

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch