theory / svn-notify

Subversion activity notification
http://search.cpan.org/dist/SVN-Notify/
18 stars 18 forks source link

Use is_valid() to test addresses. Previously, using Email::Address, … #22

Closed mpghf closed 6 years ago

mpghf commented 6 years ago

…$addr was undefined if no object was parsed. Now, using Email::Address:XS (since version 1.01), an empty Email::Address::XS object is returned.

pali commented 6 years ago

Maybe you should add dependency on version 1.01 if you are using method which was introduced in this version.

mpghf commented 6 years ago

pali,

Build.PL already requires: 'Email::Address::XS' => '1.03'

Surely this should do the trick?

pali commented 6 years ago

That should be enough.

pali commented 6 years ago

@mpghf Email::Address::XS->parse() takes one string and in list context returns N objects (N can be also zero). Therefore your logic join ', ', map { my ($addr) = ...; $addr->format } @_ is not correct.

mpghf commented 6 years ago

@pali Thanks. See if you approve of the updated fix.

pali commented 6 years ago

If you are want to take only the first address and drop all remaining, then call Email::Address::XS->parse() in scalar context in which it always returns object (and call to is_valid() would work for validation)

mpghf commented 6 years ago

@pali Ok, thanks. I see where you're going. I'll try and figure out whether we can reasonably expect more than one address.

mpghf commented 6 years ago

@pali It looks like we're passing the To and From addresses individually to $norm for formatting, so I've taken your advice.

pali commented 6 years ago

Ok, seems better.

theory commented 6 years ago

@pali would you like co-maint as well? Seems like you and @mpghf make a good team.

mpghf commented 6 years ago

@pali Regarding squashing the commits - I'll do so when I merge. I tried on my local copy, but I'm not honestly sure if I got it right.

Regarding co-maint - I'd appreciate the assistance. You're far more of a perl expert than I am.