Re: [PATCH] send-email: validate & reconfirm interactive responses

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 6, 2012, 20:03 UTC
Message-ID: <7vipbq3omb.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALaEz9W_RR70KEzxk7GxjOu1_yv4UudckcsKAP2C_39Nc2yLbw@mail.gmail.com>

Stephen Boyd <bebarino@gmail.com> writes:

Show 6 quoted lines

> It works fine for "Who should the emails appear to be from?" but
> beyond that we have "Who should the emails be sent to?" and
> "Message-ID to be used as In-Reply-To for the first email?" which I
> typically just hit enter to. It seems that they have no "default"
> argument so that second if fails. I suppose we can add a default => ""
> to these two asks?

For $initial_reply_to, I think "empty" means "I do not want to make this message reply to anything", so I think it is OK to either give a default "", or extendign valid_re to also catch an empty string. In either case, the prompt message may want to clarify what happens when you give an empty input (e.g. "leave this empty to start a new thread", or something).

If you let $to to go empty with the first hunk of your patch, where does the mail eventually go? Does anybody later in the code decide to add some recipient? If there is a reason why an empty input is a valid here, I think there is a stronger need (that is, stronger than the above ase for $initial_reply_to) to explain when the user wants to leave this empty.

Show 22 quoted lines

> ----8<-----
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 607137b..13d813e 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -760,6 +760,7 @@ if (!defined $sender) {
>
>  if (!@initial_to && !defined $to_cmd) {
>         my $to = ask("Who should the emails be sent to? ",
> +                    default => "",
>                      valid_re => qr/\@.*\./, confirm_only => 1);
>         push @initial_to, parse_address_line($to) if defined $to; #
> sanitized/validated later
>         $prompting++;
> @@ -787,6 +788,7 @@ sub expand_one_alias {
>  if ($thread && !defined $initial_reply_to && $prompting) {
>         $initial_reply_to = ask(
>                 "Message-ID to be used as In-Reply-To for the first email? ",
> +               default => "",
>                 valid_re => qr/\@.*\./, confirm_only => 1);
>  }
>  if (defined $initial_reply_to) {