Here are the relevant docs:
=head2 capability
...
The B<capability> method returns an array of capabilities as returned
by the CAPABILITY IMAP Client command, or a reference to an array of
capabilities if called in scalar context. If the CAPABILITY IMAP
Client command fails for any reason then the B<capability> method will
return C<undef>. Supported capabilities are cached by the client,
...
=head2 has_capability
...
Returns true if the IMAP server to which the IMAPClient object is
connected has the capability specified as an argument to
B<has_capability>. If the server does not have the capability then
the empty string "" is returned, if the underlying L</capability>
calls fails then undef is returned.
On Thu Oct 11 02:21:31 2018, solutions@overmeer.net wrote:
Show quoted text> * Phil Pearl (Lobbes) via RT (bug-Mail-IMAPClient@rt.cpan.org) [181011
> 03:57]:
[snip]
Show quoted text> Hum, my message of 5 oct did not reach RT or you. I'll bounce it
> to you now
[snip]
Show quoted text> > + if ( $#kv == 1 ) {
>
> if(@kv==2) # showing '2' is closer to two ()-pairs
agreed
Show quoted text> > + $self->{CAPABILITY}->{ $kv[0] } ||= [];
> > + push ( @{ $self->{CAPABILITY}->{ $kv[0] } }, $kv[1] );
>
> Now you loose backwards compatibility with the ugly $imap->{PARAM}.
> That construct is documented, hence must be kept.
I don't believe that is documented or supported, but maybe I missed something?
Show quoted text> You do not need the first of the pair by autovivification.
fixed
Show quoted text> + my $field = $self->{CAPABILITY}->{$cap} ||= [];
> + my ($name, $value) = split /\=/, $cap, 2;
> + push @{$field->{$name}}, $value
> + if defined $value;
>
> But the latter still breaks compatibility. Maybe add:
>
> $self->{CAPABILITY}->{$name} = $value; # backwards compat
I defer to my first comment that I believe this is not documented, and not supported.
This is a place that no one should be poking around in.
Show quoted text>
> - push( @addc, $kv[0] ) unless exists $self->{CAPABILITY}->{ $kv[0]
> - push ( @caps, @addc );
> - return wantarray ? @caps : \@caps;
>
> + my @names = keys %{$self->{CAPABILITY}};
> + wantarray ? @names : \@names;
>
> > @@ -2846,7 +2855,17 @@ sub capability {
> - my $aref = $self->{CAPABILITY}{ uc $which };
> - if (wantarray) {
> - return @{ $aref || [] };
> - }
> - elsif ($aref) {
> - return @$aref ? $aref->[0] : $which;
> ^^^^^^^ interface change
The interface is documented to return true, and any non-zero/empty value is true in perl so I feel this keeps with the documentation and the spirit of original intention.
Show quoted text> But I strongly advice a new method to return lists, for situations
> like this:
>
> call(count => $imap->has_capability{PARAM});
>
> In LIST content, this may now produce an
> "Odd number of elements in hash assignment"
Agreed, that could be problematic, even if likely rare for today's usage.
If we really are concerned about this, then I suppose we might do this:
- capability stays as is - return undef/array/arrayref depending upon context
- has_capability stays as is - return undef/true(capability name)/false("")
- get_capability - similar to has_capability but returns undef or an array ref always?
- gathers:
1. an array of values (val1, val2, ...)
2. single element array (capability) when no values are present
- returns:
a. undef - underlying error
b. array ref regardless of list/scalar context
Is this preferred? This could be a simple wrapper around has_capabililty as proposed above.
Or what if we changed this return in has_capability to always return an array reference?
+ elsif ($aref) {
+ return @$aref ? $aref->[0] : $which;
+ }
becomes:
+ return @$aref ? $aref : [$which];
Then document/suggest the caller use 'scalar' before any arguments in a list to ensure proper context is used to get the desired results.
The result evaluates to true still, but now the caller needs to deal with an array ref instead of a string. The docs would need to be updated to note that "true" is now perhaps more useful than before, but is a reference and not just a string which may not have provided the full information desired in the past.
Given the docs only stated returning "true", we may have leeway, but do we want true to be more useful/correct or should true really be just a 1 (one) and give other actual info in a separate/new call?
I'm not sold on a specific approach yet.
