git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH 2/2] Remove now useless email-address parsing code

From
Alex Bennée <alex.bennee@linaro.org>
Date
Jan 4, 2018, 22:11 UTC
Message-ID
<87po6pcm08.fsf@linaro.org>
In-Reply-To
<1515092151-14423-2-git-send-email-git@matthieu-moy.fr>
Matthieu Moy <git@matthieu-moy.fr> writes:
Show 11 quoted lines
> We now use Mail::Address unconditionaly, hence parse_mailboxes is now
> dead code. Remove it and its tests.
>
> Signed-off-by: Matthieu Moy <git@matthieu-moy.fr>
> ---
>  perl/Git.pm          | 71 ----------------------------------------------------
>  t/t9000-addresses.sh | 27 --------------------
>  t/t9000/test.pl      | 67 -------------------------------------------------
>  3 files changed, 165 deletions(-)
>  delete mode 100755 t/t9000-addresses.sh
>  delete mode 100755 t/t9000/test.pl
Should we add the tests for t9001-send-email.sh to guard against regressions?
Show 189 quoted lines
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index 02a3871..9d60d79 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -880,77 +880,6 @@ sub ident_person {
>  	return "$ident[0] <$ident[1]>";
>  }
>
> -=item parse_mailboxes
> -
> -Return an array of mailboxes extracted from a string.
> -
> -=cut
> -
> -# Very close to Mail::Address's parser, but we still have minor
> -# differences in some cases (see t9000 for examples).
> -sub parse_mailboxes {
> -	my $re_comment = qr/\((?:[^)]*)\)/;
> -	my $re_quote = qr/"(?:[^\"\\]|\\.)*"/;
> -	my $re_word = qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;
> -
> -	# divide the string in tokens of the above form
> -	my $re_token = qr/(?:$re_quote|$re_word|$re_comment|\S)/;
> -	my @tokens = map { $_ =~ /\s*($re_token)\s*/g } @_;
> -	my $end_of_addr_seen = 0;
> -
> -	# add a delimiter to simplify treatment for the last mailbox
> -	push @tokens, ",";
> -
> -	my (@addr_list, @phrase, @address, @comment, @buffer) = ();
> -	foreach my $token (@tokens) {
> -		if ($token =~ /^[,;]$/) {
> -			# if buffer still contains undeterminated strings
> -			# append it at the end of @address or @phrase
> -			if ($end_of_addr_seen) {
> -				push @phrase, @buffer;
> -			} else {
> -				push @address, @buffer;
> -			}
> -
> -			my $str_phrase = join ' ', @phrase;
> -			my $str_address = join '', @address;
> -			my $str_comment = join ' ', @comment;
> -
> -			# quote are necessary if phrase contains
> -			# special characters
> -			if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) {
> -				$str_phrase =~ s/(^|[^\\])"/$1/g;
> -				$str_phrase = qq["$str_phrase"];
> -			}
> -
> -			# add "<>" around the address if necessary
> -			if ($str_address ne "" && $str_phrase ne "") {
> -				$str_address = qq[<$str_address>];
> -			}
> -
> -			my $str_mailbox = "$str_phrase $str_address $str_comment";
> -			$str_mailbox =~ s/^\s*|\s*$//g;
> -			push @addr_list, $str_mailbox if ($str_mailbox);
> -
> -			@phrase = @address = @comment = @buffer = ();
> -			$end_of_addr_seen = 0;
> -		} elsif ($token =~ /^\(/) {
> -			push @comment, $token;
> -		} elsif ($token eq "<") {
> -			push @phrase, (splice @address), (splice @buffer);
> -		} elsif ($token eq ">") {
> -			$end_of_addr_seen = 1;
> -			push @address, (splice @buffer);
> -		} elsif ($token eq "@" && !$end_of_addr_seen) {
> -			push @address, (splice @buffer), "@";
> -		} else {
> -			push @buffer, $token;
> -		}
> -	}
> -
> -	return @addr_list;
> -}
> -
>  =item hash_object ( TYPE, FILENAME )
>
>  Compute the SHA1 object id of the given C<FILENAME> considering it is
> diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh
> deleted file mode 100755
> index a1ebef6..0000000
> --- a/t/t9000-addresses.sh
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -#!/bin/sh
> -
> -test_description='compare address parsing with and without Mail::Address'
> -. ./test-lib.sh
> -
> -if ! test_have_prereq PERL; then
> -	skip_all='skipping perl interface tests, perl not available'
> -	test_done
> -fi
> -
> -perl -MTest::More -e 0 2>/dev/null || {
> -	skip_all="Perl Test::More unavailable, skipping test"
> -	test_done
> -}
> -
> -perl -MMail::Address -e 0 2>/dev/null || {
> -	skip_all="Perl Mail::Address unavailable, skipping test"
> -	test_done
> -}
> -
> -test_external_has_tap=1
> -
> -test_external_without_stderr \
> -	'Perl address parsing function' \
> -	perl "$TEST_DIRECTORY"/t9000/test.pl
> -
> -test_done
> diff --git a/t/t9000/test.pl b/t/t9000/test.pl
> deleted file mode 100755
> index dfeaa9c..0000000
> --- a/t/t9000/test.pl
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -#!/usr/bin/perl
> -use lib (split(/:/, $ENV{GITPERLLIB}));
> -
> -use 5.008;
> -use warnings;
> -use strict;
> -
> -use Test::More qw(no_plan);
> -use Mail::Address;
> -
> -BEGIN { use_ok('Git') }
> -
> -my @success_list = (q[Jane],
> -	q[jdoe@example.com],
> -	q[<jdoe@example.com>],
> -	q[Jane <jdoe@example.com>],
> -	q[Jane Doe <jdoe@example.com>],
> -	q["Jane" <jdoe@example.com>],
> -	q["Doe, Jane" <jdoe@example.com>],
> -	q["Jane@:;\>.,()<Doe" <jdoe@example.com>],
> -	q[Jane!#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>],
> -	q["<jdoe@example.com>"],
> -	q["Jane jdoe@example.com"],
> -	q[Jane Doe <jdoe    @   example.com  >],
> -	q[Jane       Doe <  jdoe@example.com  >],
> -	q[Jane @ Doe @ Jane @ Doe],
> -	q["Jane, 'Doe'" <jdoe@example.com>],
> -	q['Doe, "Jane' <jdoe@example.com>],
> -	q["Jane" "Do"e <jdoe@example.com>],
> -	q["Jane' Doe" <jdoe@example.com>],
> -	q["Jane Doe <jdoe@example.com>" <jdoe@example.com>],
> -	q["Jane\" Doe" <jdoe@example.com>],
> -	q[Doe, jane <jdoe@example.com>],
> -	q["Jane Doe <jdoe@example.com>],
> -	q['Jane 'Doe' <jdoe@example.com>],
> -	q[Jane@:;\.,()<>Doe <jdoe@example.com>],
> -	q[Jane <jdoe@example.com> Doe],
> -	q[<jdoe@example.com> Jane Doe]);
> -
> -my @known_failure_list = (q[Jane\ Doe <jdoe@example.com>],
> -	q["Doe, Ja"ne <jdoe@example.com>],
> -	q["Doe, Katarina" Jane <jdoe@example.com>],
> -	q[Jane jdoe@example.com],
> -	q["Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com>],
> -	q[Jane Doe],
> -	q[Jane "Doe <jdoe@example.com>"],
> -	q[\"Jane Doe <jdoe@example.com>],
> -	q[Jane\"\" Doe <jdoe@example.com>],
> -	q['Jane "Katarina\" \' Doe' <jdoe@example.com>]);
> -
> -foreach my $str (@success_list) {
> -	my @expected = map { $_->format } Mail::Address->parse("$str");
> -	my @actual = Git::parse_mailboxes("$str");
> -	is_deeply(\@expected, \@actual, qq[same output : $str]);
> -}
> -
> -TODO: {
> -	local $TODO = "known breakage";
> -	foreach my $str (@known_failure_list) {
> -		my @expected = map { $_->format } Mail::Address->parse("$str");
> -		my @actual = Git::parse_mailboxes("$str");
> -		is_deeply(\@expected, \@actual, qq[same output : $str]);
> -	}
> -}
> -
> -my $is_passing = eval { Test::More->is_passing };
> -exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;

-- Alex Bennée

Previous: Matthieu MoyNext: Matthieu Moy
Message 3 of 22 in “add a local copy of Mail::Address from CPAN”
  1. 1/2 add a local copy of Mail::Address from CPANMatthieu Moy, Jan 4, 2018
  2. 2/2 Remove now useless email-address parsing codeMatthieu Moy, Jan 4, 2018
  3. Alex BennéeJan 4, 2018
  4. Matthieu MoyJan 5, 2018
  5. send-email: add test for Linux's get_maintainer.plMatthieu Moy, Jan 5, 2018
  6. Alex BennéeJan 5, 2018
  7. Matthieu MoyJan 5, 2018
  8. Junio C HamanoJan 5, 2018
  9. Eric SunshineJan 4, 2018
  10. Ævar Arnfjörð BjarmasonJan 5, 2018
  11. 1/3 send-email: add and use a local copy of Mail::AddressMatthieu Moy, Jan 5, 2018
  12. 2/3 Remove now useless email-address parsing codeMatthieu Moy, Jan 5, 2018
  13. 3/3 send-email: add test for Linux's get_maintainer.plMatthieu Moy, Jan 5, 2018
  14. Eric SunshineJan 5, 2018
  15. Matthieu MoyJan 8, 2018
  16. 1/3 send-email: add and use a local copy of Mail::AddressMatthieu Moy, Jan 8, 2018
  17. 3/3 send-email: add test for Linux's get_maintainer.plMatthieu Moy, Jan 8, 2018
  18. Junio C HamanoJan 8, 2018
  19. 2/3 Remove now useless email-address parsing codeMatthieu Moy, Jan 8, 2018
  20. Alex BennéeJan 8, 2018
  21. Alex BennéeJan 8, 2018
  22. Ævar Arnfjörð BjarmasonFeb 14, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.