Re: [PATCH] send-email: validate reconfirm interactive responses
(Sorry sending this from web interface)
On Wed, Sep 5, 2012 at 8:29 PM, Junio C Hamano gits...@pobox.com wrote:
Stephen Boyd bebar...@gmail.com writes:
This is now bugging me if I just hit enter and don't want to specify
anything for
these headers (I want the defaults or what's in the files already).
Can we allow
the empty string to be valid as well so I don't have to go through
these prompts?
That indeed was the intention, and if it is not behaving, you found
a bug.
The relevant code in sub ask does this:
...
$resp = $term-readline($prompt);
if (!defined $resp) { # EOF
print \n;
return defined $default ? $default : undef;
}
if ($resp eq '' and defined $default) {
return $default;
}
if (!defined $valid_re or $resp =~ /$valid_re/) {
return $resp;
}
I am scratching my head wondering why your just hit enter does not
trigger the if response is empty and we have default, just return it
codepath we can see above. It shouldn't even trigger the regexp
based validation codepath in the first place.
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?
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) {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: validate reconfirm interactive responses
Stephen Boyd bebar...@gmail.com writes:
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.
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) {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: validate reconfirm interactive responses
On Thu, Sep 6, 2012 at 1:03 PM, Junio C Hamano gits...@pobox.com wrote: 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. I almost never type anything and just use the To header in the patch I want to send. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: validate reconfirm interactive responses
On Tue, Aug 14, 2012 at 3:25 PM, Junio C Hamano gits...@pobox.com wrote:
@@ -745,13 +752,15 @@ sub file_declares_8bit_cte {
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
$sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender);
+ default = $sender,
+ valid_re = qr/\@.*\./, confirm_only = 1);
This is now bugging me if I just hit enter and don't want to specify
anything for
these headers (I want the defaults or what's in the files already).
Can we allow
the empty string to be valid as well so I don't have to go through
these prompts?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: validate reconfirm interactive responses
Stephen Boyd bebar...@gmail.com writes:
On Tue, Aug 14, 2012 at 3:25 PM, Junio C Hamano gits...@pobox.com wrote:
@@ -745,13 +752,15 @@ sub file_declares_8bit_cte {
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
$sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender);
+ default = $sender,
+ valid_re = qr/\@.*\./, confirm_only = 1);
This is now bugging me if I just hit enter and don't want to specify
anything for
these headers (I want the defaults or what's in the files already).
Can we allow
the empty string to be valid as well so I don't have to go through
these prompts?
That indeed was the intention, and if it is not behaving, you found
a bug.
The relevant code in sub ask does this:
...
$resp = $term-readline($prompt);
if (!defined $resp) { # EOF
print \n;
return defined $default ? $default : undef;
}
if ($resp eq '' and defined $default) {
return $default;
}
if (!defined $valid_re or $resp =~ /$valid_re/) {
return $resp;
}
I am scratching my head wondering why your just hit enter does not
trigger the if response is empty and we have default, just return it
codepath we can see above. It shouldn't even trigger the regexp
based validation codepath in the first place.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: validate reconfirm interactive responses
Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes:
On Tue, Aug 14, 2012 at 3:25 PM, Junio C Hamano gits...@pobox.com wrote:
People answer 'y' to Who should the emails appear to be from? and
'n' to Message-ID to be used as In-Reply-To for the first email?
for some unknown reason.
Yeah, I know :-(. I did feel stupid already. Thanks for improving.
Actually, it is a very understandable mistake and I do not think it
is a user stupidity. It is a UI bug in the prompter that gives:
Who should the emails appear to be from? [Junio C Hamano gits...@pobox.com]
and does *not* tell the user that the way to accept the default is
to just press RETURN. It makes it look as if it is asking Is it OK
to use this?, and it is a natural response to say Yes to the
prompt.
We would want to do something like the following pseudo-patch, I
think, but I do not know what is the best way to show both $prompt
and the press return suggestion to the user, so I am not going to
do this myself.
A tested patch to improve this is very much welcomed.
Thanks.
diff --git a/git-send-email.perl b/git-send-email.perl
index 607137b..2ec0ce8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -688,6 +688,9 @@ sub ask {
unless defined $term-IN and defined fileno($term-IN) and
defined $term-OUT and defined fileno($term-OUT);
while ($i++ 10) {
+ if (defined $default) {
+ SAY (press RETURN to accept the default);
+ }
$resp = $term-readline($prompt);
if (!defined $resp) { # EOF
print \n;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
