Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Ævar Arnfjörð Bjarmason writes: > Ripping out Error.pm for our few internal callers is one thing, trying > to maintain bugwards compatibility with how it throws exceptions for > users expecting Error.pm objects is another. I think at that point it's > easier to just stay with

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Tue, Dec 12 2017, Junio C. Hamano jotted: > Ævar Arnfjörð Bjarmason writes: > >> My "Makefile: replace perl/Makefile.PL with simple make rules" currently >> cooking in pu changes that so that: >> >> * We always at runtime test for the system CPAN module. >> >> * In the

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Ævar Arnfjörð Bjarmason writes: > My "Makefile: replace perl/Makefile.PL with simple make rules" currently > cooking in pu changes that so that: > > * We always at runtime test for the system CPAN module. > > * In the case of Error.pm we happen to ship a fallback, in the case

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Tue, Dec 12 2017, Alex Bennée jotted: > Thomas Adam writes: > >> Hi, >> >> On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote: >>> I.e. we'd just ship a copy of Email::Valid and Mail::Address in >>> perl/Git/FromCPAN/, use a wrapper to load them, and

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Thomas Adam writes: > Hi, > > On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote: >> I.e. we'd just ship a copy of Email::Valid and Mail::Address in >> perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't >> need to if/else this at the

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Tue, Dec 12 2017, Thomas Adam jotted: > Hi, > > On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote: >> I.e. we'd just ship a copy of Email::Valid and Mail::Address in >> perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't >> need to if/else this at the

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Hi, On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote: > I.e. we'd just ship a copy of Email::Valid and Mail::Address in > perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't > need to if/else this at the code level, just always use the module, > and it

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Mon, Dec 11, 2017 at 6:26 PM, Thomas Adam wrote: > On Mon, Dec 11, 2017 at 05:13:53PM +, Alex Bennée wrote: >> So have we come to a consensus about the best solution here? >> >> I'm perfectly happy to send a reversion patch because to be honest >> hacking on a bunch of

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Mon, Dec 11, 2017 at 05:13:53PM +, Alex Bennée wrote: > So have we come to a consensus about the best solution here? > > I'm perfectly happy to send a reversion patch because to be honest > hacking on a bunch of perl to handle special mail cases is not my idea > of fun spare time hacking

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Junio C Hamano writes: > Thomas Adam writes: > >> Trying to come up with a reinvention of regexps for email addresses is asking >> for trouble, not to mention a crappy rod for your own back. Don't do that. >> This is why people use Mail::Address. >> >>

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Matthieu Moy writes: > My point in cc907506 ("send-email: don't use Mail::Address, even if > available", 2017-08-23) was not that Mail::Address was bad, but that > changing our behavior depending on whether it was there or not was > really bad. For example, the issue

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Wed, Nov 22, 2017 at 09:05:41AM +, Alex Bennée wrote: > My hacky guess about GIT's perl use calls is: > >find . -iname "*.perl" -or -iname "*.pm" -or -iname "*.pl" | xargs grep -h > "use .*::" | sort | uniq | wc -l >88 So let us concentrate just on git-send-email.perl for now.

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Matthieu Moy writes: > Junio C Hamano writes: > >> Was there any reason why Mail::Address was _inadequate_? > > I think the main reason was that Mail::Address is not a standard perl > module, and not relying on it avoided one external dependency.

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Junio C Hamano writes: > Was there any reason why Mail::Address was _inadequate_? I think the main reason was that Mail::Address is not a standard perl module, and not relying on it avoided one external dependency. AFAIK, we don't really have a good way to deal with Perl

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Thomas Adam writes: > Trying to come up with a reinvention of regexps for email addresses is asking > for trouble, not to mention a crappy rod for your own back. Don't do that. > This is why people use Mail::Address. > >

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Tue, Nov 21, 2017 at 08:46:59PM +, Alex Bennée wrote: > > Eric Sunshine writes: > > Aside from those observations, it looks like the tokenizer in this > > function is broken. For any input with the address enclosed in "<" and > > ">", the comment is lost entirely;

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Eric Sunshine writes: > A few more comments/observations... > > On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée wrote: >> diff --git a/perl/Git.pm b/perl/Git.pm >> @@ -936,6 +936,9 @@ sub parse_mailboxes { >>

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Eric Sunshine writes: > The more I think about this, the less I consider this a bug in > git-send-email. As noted, people might legitimately use a complex > command (--cc-cmd="myscript--option arg"), so changing git-send-email > to treat cc-cmd as an atomic string seems

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Mon, Nov 20, 2017 at 7:07 PM, Philip Oakley wrote: > From: "Eric Sunshine" > On Sat, Nov 18, 2017 at 9:54 PM, Eric Sunshine > wrote: >> > --- 8< --- >> > diff --git a/git-send-email.perl b/git-send-email.perl >> > @@

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

From: "Eric Sunshine" On Sat, Nov 18, 2017 at 9:54 PM, Eric Sunshine wrote: On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée wrote: +test_expect_success $PREREQ 'cc trailer with get_maintainer output' ' + [...] +

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Mon, Nov 20, 2017 at 5:44 AM, Alex Bennée wrote: > Eric Sunshine writes: >> It is not at all clear, based upon this text, what this is fixing. >> When you re-roll, please provide a description of the regression in >> sufficient detail for

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

A few more comments/observations... On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée wrote: > diff --git a/perl/Git.pm b/perl/Git.pm > @@ -936,6 +936,9 @@ sub parse_mailboxes { > $end_of_addr_seen = 0; > } elsif ($token =~ /^\(/) { >

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Sat, Nov 18, 2017 at 9:54 PM, Eric Sunshine wrote: > On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée wrote: >> +test_expect_success $PREREQ 'cc trailer with get_maintainer output' ' >> + [...] >> + git send-email -1

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Eric Sunshine writes: > On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée wrote: >> Getting rid of Mail::Address regressed behaviour with common >> get_maintainer scripts such as the Linux kernel. Fix the missed corner >> case and add a test for it.

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée wrote: > Getting rid of Mail::Address regressed behaviour with common > get_maintainer scripts such as the Linux kernel. Fix the missed corner > case and add a test for it. It is not at all clear, based upon this text, what

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

Alex Bennée writes: > Getting rid of Mail::Address regressed behaviour with common > get_maintainer scripts such as the Linux kernel. Fix the missed corner > case and add a test for it. > > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > index

[PATCH] git-send-email: fix get_maintainer.pl regression

Getting rid of Mail::Address regressed behaviour with common get_maintainer scripts such as the Linux kernel. Fix the missed corner case and add a test for it. Fixes: cc9075067776ebd34cc08f31bf78bb05f12fd879 Signed-off-by: Alex Bennée --- perl/Git.pm | 3 +++