Re: [PATCH 5/5] git-send-email: allow edit invalid email address

2012-11-27 Thread Krzysztof Mazur
On Mon, Nov 26, 2012 at 03:50:30PM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  On Mon, Nov 26, 2012 at 02:58:58PM -0800, Junio C Hamano wrote:
  Krzysztof Mazur krzys...@podlesie.net writes:
  
   Not having this new code inside elsif (/^e/) { } feels somewhat
   sloppy, even though it is not *too* bad.  Also do we know this
  
   ok, I will fix that.
  
   function will never be used for addresses other than recipients' (I
   gave a cursory look to see what is done to the $sender and it does
   not seem to go through this function, tho)?
  
   Yes, this function is called only from validate_address_just()
   to filter @initial_to, @initial_cc, @bcc_list as early as possible,
   and filter @to and @cc added in each email.
  
  Thanks; when merged to 'pu', this series seems to break t9001.  I'll
  push the result out with breakages but could you take a look?
  
 
  Sorry, I tested final version only on an ancient perl 5.8.8 and it really
  worked there. The third patch is broken:
 
  diff --git a/git-send-email.perl b/git-send-email.perl
  index 9996735..f3bbc16 100755
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -1472,7 +1472,7 @@ sub unique_email_list {
  my @emails;
   
  foreach my $entry (@_) {
  -   my $clean = extract_valid_address_or_die($entry))
  +   my $clean = extract_valid_address_or_die($entry);
 
 Ah, ok, I wasn't looking closely enough.  Thanks for a quick
 turnaround.  Will requeue and push out.

I rechecked that and I've just sent some older broken version. The
patch that I've sent had date Date: Thu, 22 Nov 2012 19:00:25 +0100,
but on my tree I have commit from Date: Thu Nov 22 19:01:55 2012 +0100,
which is exactly the same as the fixed version in your tree.

Thanks,

Krzysiek
--
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 5/5] git-send-email: allow edit invalid email address

2012-11-26 Thread Krzysztof Mazur
On Mon, Nov 26, 2012 at 09:08:34AM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  In some cases the user may want to send email with Cc: line with
  email address we cannot extract. Now we allow user to extract
  such email address for us.
 
  Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
  ---
   git-send-email.perl | 9 ++---
   1 file changed, 6 insertions(+), 3 deletions(-)
 
  diff --git a/git-send-email.perl b/git-send-email.perl
  index d42dca2..9996735 100755
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -851,10 +851,10 @@ sub extract_valid_address_or_die {
   
   sub validate_address {
  my $address = shift;
  -   if (!extract_valid_address($address)) {
  +   while (!extract_valid_address($address)) {
  print STDERR error: unable to extract a valid address from: 
  $address\n;
  -   $_ = ask(What to do with this address? ([q]uit|[d]rop): ,
  -   valid_re = qr/^(?:quit|q|drop|d)/i,
  +   $_ = ask(What to do with this address? ([q]uit|[d]rop|[e]dit): 
  ,
  +   valid_re = qr/^(?:quit|q|drop|d|edit|e)/i,
  default = 'q');
  if (/^d/i) {
  return undef;
  @@ -862,6 +862,9 @@ sub validate_address {
  cleanup_compose_files();
  exit(0);
  }
  +   $address = ask(Who should the email be sent to (if any)? ,
  +   default = ,
  +   valid_re = qr/\@.*\./, confirm_only = 1);
 
 Not having this new code inside elsif (/^e/) { } feels somewhat
 sloppy, even though it is not *too* bad.  Also do we know this

ok, I will fix that.

 function will never be used for addresses other than recipients' (I
 gave a cursory look to see what is done to the $sender and it does
 not seem to go through this function, tho)?

Yes, this function is called only from validate_address_just()
to filter @initial_to, @initial_cc, @bcc_list as early as possible,
and filter @to and @cc added in each email.

Krzysiek
--
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 5/5] git-send-email: allow edit invalid email address

2012-11-26 Thread Junio C Hamano
Krzysztof Mazur krzys...@podlesie.net writes:

 In some cases the user may want to send email with Cc: line with
 email address we cannot extract. Now we allow user to extract
 such email address for us.

 Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
 ---
  git-send-email.perl | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

 diff --git a/git-send-email.perl b/git-send-email.perl
 index d42dca2..9996735 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -851,10 +851,10 @@ sub extract_valid_address_or_die {
  
  sub validate_address {
   my $address = shift;
 - if (!extract_valid_address($address)) {
 + while (!extract_valid_address($address)) {
   print STDERR error: unable to extract a valid address from: 
 $address\n;
 - $_ = ask(What to do with this address? ([q]uit|[d]rop): ,
 - valid_re = qr/^(?:quit|q|drop|d)/i,
 + $_ = ask(What to do with this address? ([q]uit|[d]rop|[e]dit): 
 ,
 + valid_re = qr/^(?:quit|q|drop|d|edit|e)/i,
   default = 'q');
   if (/^d/i) {
   return undef;
 @@ -862,6 +862,9 @@ sub validate_address {
   cleanup_compose_files();
   exit(0);
   }
 + $address = ask(Who should the email be sent to (if any)? ,
 + default = ,
 + valid_re = qr/\@.*\./, confirm_only = 1);

Not having this new code inside elsif (/^e/) { } feels somewhat
sloppy, even though it is not *too* bad.  Also do we know this
function will never be used for addresses other than recipients' (I
gave a cursory look to see what is done to the $sender and it does
not seem to go through this function, tho)?
--
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 5/5] git-send-email: allow edit invalid email address

2012-11-26 Thread Junio C Hamano
Krzysztof Mazur krzys...@podlesie.net writes:

 Not having this new code inside elsif (/^e/) { } feels somewhat
 sloppy, even though it is not *too* bad.  Also do we know this

 ok, I will fix that.

 function will never be used for addresses other than recipients' (I
 gave a cursory look to see what is done to the $sender and it does
 not seem to go through this function, tho)?

 Yes, this function is called only from validate_address_just()
 to filter @initial_to, @initial_cc, @bcc_list as early as possible,
 and filter @to and @cc added in each email.

Thanks; when merged to 'pu', this series seems to break t9001.  I'll
push the result out with breakages but could you take a look?


Test Summary Report
---
t9001-send-email.sh  (Wstat: 256 Tests: 102 Failed: 
77)
  Failed tests:  4-7, 9-10, 12-13, 15, 17-21, 23-29, 31-33
35, 37, 39, 41, 43, 45, 47, 49, 51-58, 61-88
91, 93-95, 98-102
  Non-zero exit status: 1
--
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


[PATCH 5/5] git-send-email: allow edit invalid email address

2012-11-22 Thread Krzysztof Mazur
In some cases the user may want to send email with Cc: line with
email address we cannot extract. Now we allow user to extract
such email address for us.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d42dca2..9996735 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -851,10 +851,10 @@ sub extract_valid_address_or_die {
 
 sub validate_address {
my $address = shift;
-   if (!extract_valid_address($address)) {
+   while (!extract_valid_address($address)) {
print STDERR error: unable to extract a valid address from: 
$address\n;
-   $_ = ask(What to do with this address? ([q]uit|[d]rop): ,
-   valid_re = qr/^(?:quit|q|drop|d)/i,
+   $_ = ask(What to do with this address? ([q]uit|[d]rop|[e]dit): 
,
+   valid_re = qr/^(?:quit|q|drop|d|edit|e)/i,
default = 'q');
if (/^d/i) {
return undef;
@@ -862,6 +862,9 @@ sub validate_address {
cleanup_compose_files();
exit(0);
}
+   $address = ask(Who should the email be sent to (if any)? ,
+   default = ,
+   valid_re = qr/\@.*\./, confirm_only = 1);
}
return $address;
 }
-- 
1.8.0.393.gcc9701d

--
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