Re: [RFC PATCH] sit-send-email.pl: Add --to-cmd

From: Joe Perches <joe@perches.com>
Date: Sep 23, 2010, 17:46 UTC
Message-ID: <1285263993.31572.25.camel@Joe-Laptop>
In-Reply-To: <AANLkTin_Y8w4ujNGTqGJPNDNfYz7hcjBVLcOG0emBjYn@mail.gmail.com>

On Thu, 2010-09-23 at 17:29 +0000, Ævar Arnfjörð Bjarmason wrote:

Show 6 quoted lines

> On Thu, Sep 23, 2010 at 17:17, Joe Perches <joe@perches.com> wrote:
> > I know there's a test harness in git, but
> > I don't know how to wire up the new options.
> You'd add the tests to t9001-send-email.sh and --tocmd out to some
> program you create. Is there anything in particular you need help
> with?

Just the doing.  I was (am) being lazy.

> > -if (!@to) {
> > +if (!@to && $to_cmd eq "") {
> 
> Why compare $to_cmd to "" instead of checking definedness?

No real reason. Using define is the style used in the rest of the file and it should be changed.

Show 10 quoted lines

> > @@ -1238,6 +1242,23 @@ foreach my $t (@files) {
> >        }
> >        close F;
> >
> > +       if (defined $to_cmd) {
> > +               open(F, "$to_cmd \Q$t\E |")
> 
> quotemeta() is for escaping regexes, not shell syntax. You probably
> want IPC::Open2 or PC::Open3's functions which'll escape arguments for
> you.

I just copied the style from the equivalent cc_cmd section below, so if it's necessary, it should be changed there too.

> I.e. do you need to strip whitespace from the beginning of the string?

I think so.