git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matthieu Moy <git@matthieu-moy.fr>
To: git@vger.kernel.org
Cc: Matthieu Moy <git@matthieu-moy.fr>
Subject: [RFC PATCH 2/2] send-email: don't use Mail::Address, even if available
Date: Wed, 23 Aug 2017 12:21:02 +0200	[thread overview]
Message-ID: <20170823102102.20120-2-git@matthieu-moy.fr> (raw)
In-Reply-To: <20170823102102.20120-1-git@matthieu-moy.fr>

Using Mail::Address made sense when we didn't have a proper parser. We
now have a reasonable address parser, and using Mail::Address
_if available_ causes much more trouble than it gives benefits:

* Developers typically test one version, not both.

* Users may not be aware that installing Mail::Address will change the
  behavior. They may complain about the behavior in one case without
  knowing that Mail::Address is involved.

* Having this optional Mail::Address makes it tempting to anwser "please
  install Mail::Address" to users instead of fixing our own code. We've
  reached the stage where bugs in our parser should be fixed, not worked
  around.

Signed-off-by: Matthieu Moy <git@matthieu-moy.fr>
---
 git-send-email.perl | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 33a69ffe5d..2208dcc213 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -155,7 +155,6 @@ sub format_2822_time {
 }
 
 my $have_email_valid = eval { require Email::Valid; 1 };
-my $have_mail_address = eval { require Mail::Address; 1 };
 my $smtp;
 my $auth;
 my $num_sent = 0;
@@ -490,11 +489,7 @@ my ($repoauthor, $repocommitter);
 ($repocommitter) = Git::ident_person(@repo, 'committer');
 
 sub parse_address_line {
-	if ($have_mail_address) {
-		return map { $_->format } Mail::Address->parse($_[0]);
-	} else {
-		return Git::parse_mailboxes($_[0]);
-	}
+	return Git::parse_mailboxes($_[0]);
 }
 
 sub split_addrs {
-- 
2.14.0.rc0.dirty


  reply	other threads:[~2017-08-23 10:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-22 23:15 git send-email Cc with cruft not working as expected Jacob Keller
2017-08-22 23:18 ` Stefan Beller
2017-08-22 23:30   ` Jacob Keller
2017-08-22 23:36     ` Stefan Beller
2017-08-23 10:02       ` Matthieu Moy
2017-08-23 10:21         ` [RFC PATCH 1/2] send-email: fix garbage removal after address Matthieu Moy
2017-08-23 10:21           ` Matthieu Moy [this message]
2017-08-23 21:59           ` Jacob Keller
2017-08-24 20:32           ` Junio C Hamano
2017-08-25  9:11             ` Matthieu Moy
2017-08-25  9:11               ` [PATCH v2 " Matthieu Moy
2017-08-25  9:12                 ` [PATCH v2 2/2] send-email: don't use Mail::Address, even if available Matthieu Moy
2017-08-23 22:49         ` git send-email Cc with cruft not working as expected Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170823102102.20120-2-git@matthieu-moy.fr \
    --to=git@matthieu-moy.fr \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).