Use is_valid() to test addresses. Previously, using Email::Address, … #22
Conversation
…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.
|
Maybe you should add dependency on version 1.01 if you are using method which was introduced in this version. |
|
pali, Build.PL already requires: 'Email::Address::XS' => '1.03' Surely this should do the trick? |
|
That should be enough. |
|
@mpghf |
Check that $addr is defined before trying to use it. For empty strings as input (at least), $addr can be undef.
|
@pali Thanks. See if you approve of the updated fix. |
|
If you are want to take only the first address and drop all remaining, then call |
|
@pali Ok, thanks. I see where you're going. I'll try and figure out whether we can reasonably expect more than one address. |
Email::Address::XS->parse will always return a valid scalar object, and is->valid() can be used to test if the result is empty or not. john@example.com -> not empty john -> empty '' -> empty The addresses are passed individually to $norm, so there will never be multiple addresses.
|
@pali It looks like we're passing the To and From addresses individually to $norm for formatting, so I've taken your advice. |
|
Ok, seems better. |
lib/SVN/Notify.pm
Outdated
| @@ -1475,7 +1475,7 @@ sub output_headers { | |||
| $norm = sub { | |||
| return join ', ' => map { | |||
| my ($addr) = Email::Address::XS->parse($_); | |||
| if ($addr) { | |||
| if ($addr->is_valid()) { | |||
There was a problem hiding this comment.
Do we know for sure that $addr is never undef? Looks like it could be. Maybe this should be:
my $addr = Email::Address::XS->parse($_);
if ($addr && $addr->is_valid) {Though reading the docs for parse linked above, it seems as though is_valid could return false if there were more than one object parsed? Kinda weird.
There was a problem hiding this comment.
In scalar context since 1.01 it always returns Email::Address::XS object.
In scalar context this function returns just first parsed object. If more then one object was parsed then is_valid method on returned object returns false. If no object was parsed then empty Email::Address::XS object is returned.
@theory you are looking at old version of patches in this pull request
@mpghf Ideally squash changes, so there would not be any reverts or old versions
There was a problem hiding this comment.
Running if($addr) on an empty address caused the warnings - I'm not familiar enough with perl's behaviour in that case to understand why.
Using if(defined $addr) instead worked, but @pali has given a better option where $addr will never be undef.
There was a problem hiding this comment.
Running if($addr) on an empty address caused the warnings
That is probably because of stringification operator which calls format() method? (Now just guessing).
There was a problem hiding this comment.
@pali Thanks. Good to know.
Email::Address::XS->parse will always return a valid scalar object, and is->valid() can be used to test if the result is empty or not. 'john@example.com' -> not empty 'john' -> empty '' -> empty The addresses are passed individually to $norm, so there will never be multiple addresses.
|
@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. |
…$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.