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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 23, 2010 at 17:17, Joe Perches <joe@xxxxxxxxxxx> wrote:
> On Thu, 2010-09-23 at 17:58 +0200, Julia Lawall wrote:
>> On Thu, 23 Sep 2010, Joe Perches wrote:
>> > On Thu, 2010-09-23 at 16:00 +0400, Vasiliy Kulikov wrote:
>> > > On Thu, Sep 23, 2010 at 13:09 +0400, Vasiliy Kulikov wrote:
>> > > > On Thu, Sep 23, 2010 at 10:55 +0200, Julia Lawall wrote:
>> > > > > I made some changes to git-send-email to get it to send mail to different
>> > > > > people, ie a different set of addresses for each patch. ÂIs that now
>> > > > > possible with the standard version? ÂIf not I can submit a patch with my
>> > > > > changes at some point.
>> > > > I use git-send-email --cc-cmd=script_to_form_cc_list.
>> > I believe that Julia means some mechanism to vary the
>> > "to" addresses for each patch, ie: some "--to-cmd=cmd".
>> Yes, sort of. ÂI took the strategy of precomputing the To addresses, so I
>> just have a collection of files that have different To and Cc addresses.
>> But a --to-cmd option seems like a good idea too.
>
> Perhaps something like this?
>
> Lightly tested only.
>
> 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?

> Signed-off-by: Joe Perches <joe@xxxxxxxxxxx>
> ---
> Âgit-send-email.perl | Â 25 +++++++++++++++++++++++--
> Â1 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 6dab3bf..8e8e4c4 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -70,6 +70,7 @@ git send-email [options] <file | directory | rev-list options >
>
> Â Automating:
>   --identity       Â<str> Â* Use the sendemail.<id> options.
> +  Â--to-cmd        Â<str> Â* Email To: via `<str> \$patch_path`
>   --cc-cmd        Â<str> Â* Email Cc: via `<str> \$patch_path`
>   --suppress-cc      <str> Â* author, self, sob, cc, cccmd, body, bodycc, all.
>   --[no-]signed-off-by-cc    Â* Send to Signed-off-by: addresses. Default on.
> @@ -187,7 +188,8 @@ sub do_edit {
> Â}
>
> Â# Variables with corresponding config settings
> -my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
> +my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
> +my ($to_cmd, $cc_cmd);
> Âmy ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
> Âmy ($identity, $aliasfiletype, @alias_files, @smtp_host_parts, $smtp_domain);
> Âmy ($validate, $confirm);
> @@ -214,6 +216,7 @@ my %config_settings = (
> Â Â "smtppass" => \$smtp_authpass,
> Â Â Â Â"smtpdomain" => \$smtp_domain,
> Â Â "to" => \@to,
> + Â Â"tocmd" => \$to_cmd,
> Â Â "cc" => \@initial_cc,
> Â Â "cccmd" => \$cc_cmd,
> Â Â "aliasfiletype" => \$aliasfiletype,
> @@ -272,6 +275,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
> Â Â Â Â Â Â Â Â Â Â "in-reply-to=s" => \$initial_reply_to,
> Â Â Â Â Â Â Â Â Â Â"subject=s" => \$initial_subject,
> Â Â Â Â Â Â Â Â Â Â"to=s" => \@to,
> + Â Â Â Â Â Â Â Â Â "to-cmd=s" => \$to_cmd,
> Â Â Â Â Â Â Â Â Â Â"no-to" => \$no_to,
> Â Â Â Â Â Â Â Â Â Â"cc=s" => \@initial_cc,
> Â Â Â Â Â Â Â Â Â Â"no-cc" => \$no_cc,
> @@ -711,7 +715,7 @@ if (!defined $sender) {
> Â Â Â Â$prompting++;
> Â}
>
> -if (!@to) {
> +if (!@to && $to_cmd eq "") {

Why compare $to_cmd to "" instead of checking definedness?

> Â Â Â Âmy $to = ask("Who should the emails be sent to? ");
> Â Â Â Âpush @to, parse_address_line($to) if defined $to; # sanitized/validated later
> Â Â Â Â$prompting++;
> @@ -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.

Also "open my $f" is better, but I see the existing code uses glob
filehandles (urghl).

> + Â Â Â Â Â Â Â Â Â Â Â or die "(to-cmd) Could not execute '$to_cmd'";
> + Â Â Â Â Â Â Â while(<F>) {
> + Â Â Â Â Â Â Â Â Â Â Â my $t = $_;
> + Â Â Â Â Â Â Â Â Â Â Â $t =~ s/^\s*//g;
> + Â Â Â Â Â Â Â Â Â Â Â $t =~ s/\n$//g;

Shouldn't this just be:

    while (my $address = <$f>) {
        chomp $address;
        ...

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

> + Â Â Â Â Â Â Â Â Â Â Â next if ($t eq $sender and $suppress_from);
> + Â Â Â Â Â Â Â Â Â Â Â push @to, parse_address_line($t)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â if defined $t; # sanitized/validated later
> + Â Â Â Â Â Â Â Â Â Â Â printf("(to-cmd) Adding To: %s from: '%s'\n",
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â $t, $to_cmd) unless $quiet;
> + Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â close F
> + Â Â Â Â Â Â Â Â Â Â Â or die "(to-cmd) failed to close pipe to '$to_cmd'";
> + Â Â Â }

close F could be skipped if we used lexical handes, but see urghl
above.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]