Skip Menu |
 

This queue is for tickets about the Keyword-Declare CPAN distribution.

Report information
The Basics
Id: 131661
Status: resolved
Priority: 0/
Queue: Keyword-Declare

People
Owner: Nobody in particular
Requestors: CXW [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.001015
Fixed in: 0.001016



Subject: Use of K::D::Arg for optional arguments breaks API and is not documented as such
Download (untitled) / with headers
text/plain 1.5k
Damian, thanks for this module, which I have been using in my own dists for a year or two. I also greatly enjoyed your Inception conference-talk video. :) In v0.001015, my Test::OnlySome breaks: https://rt.cpan.org/Ticket/Display.html?id=131643 (thanks to ANDK for the report). The keyword declaration is: keyword os(String? $debug_var, Var? $opts_name, Num? $N, Block|Statement $controlled) (e.g., to be used as `os ok(1, "this test passed (if it was run)")`). In <= v0.001014, optional $debug_var, $opts_name, and $N are undef if the corresponding syntactic element is not present where the keyword is used. However, in v0.001015, all three are K::D::Arg instances, thus defined, regardless of whether the element is present. This contradicts the examples in the K::D POD, which still use `defined` to check whether an optional parameter is present. (E.g., https://metacpan.org/pod/release/DCONWAY/Keyword-Declare-0.001015/lib/Keyword/Declare.pm#DESCRIPTION ). - If you still intend `defined` to be usable to check whether an optional parameter is present, would you please not pass a K::D::Arg instance? - If K::D::Arg is the desired API change even for this situation, would you please update the documentation to show how to check for presence of the argument? If you wish, please feel free to use my current hack: https://github.com/cxw42/Test-OnlySome/commit/1986bfb40f04e240f0c2645fd82126a9bb719c4e#diff-cbf0d9a93b7e8b6018777e7e108fbe18R521-R541 . (However, there's almost certainly a better way!) Thank you for considering this request!
Subject: Re: [rt.cpan.org #131661] Use of K::D::Arg for optional arguments breaks API and is not documented as such
Date: Wed, 5 Feb 2020 00:10:37 +0000
To: bug-Keyword-Declare [...] rt.cpan.org
From: Damian Conway <damian [...] conway.org>
Hi Christopher, Thanks for the report. I am sincerely sorry for having changed the API out from under you. Show quoted text
> - If K::D::Arg is the desired API change even for this situation, > would you please update the documentation to show how to check for > presence of the argument?
The API change was intentional (and permanent) so I have updated the docs to show the correct way to test for a missing optional parameter. In fact I added an entire subsection on that topic. That change has just been uploaded to CPAN as Keyword::Declare version 0.001016. The TLDR is that you should use length($optional_param) instead of defined($optional_param) to check whether an optional parameter was found. Happily, using length() instead of defined() is backwards compatible to previous (i.e pre-K::D::Arg) versions of K::D, so updating that internal test within Test::OnlySome will not break your module for those who are still using an older version of K::D. Once again, I apologize for the breakage to your module, and sincerely hope that this will be the last time that happens. All the best, Damian
Download (untitled) / with headers
text/plain 1.4k
Damian, No worries! Thanks very much for your quick response. I have modified Test::OnlySome to use length() and it works fine. If you're going to be in Houston for TPaRCiH, I'll thank you in person; either way, best regards! Chris On Tue Feb 04 19:11:20 2020, damian@conway.org wrote: Show quoted text
> Hi Christopher, > > Thanks for the report. > I am sincerely sorry for having changed the API out from under you. > >
> > - If K::D::Arg is the desired API change even for this situation, > > would you please update the documentation to show how to check for > > presence of the argument?
> > The API change was intentional (and permanent) so I have updated the > docs to show the correct way to test for a missing optional parameter. > In fact I added an entire subsection on that topic. That change has just > been uploaded to CPAN as Keyword::Declare version 0.001016. > > The TLDR is that you should use length($optional_param) > instead of defined($optional_param) to check whether > an optional parameter was found. > > Happily, using length() instead of defined() is backwards compatible > to previous (i.e pre-K::D::Arg) versions of K::D, so updating that > internal test within Test::OnlySome will not break your module for > those who are still using an older version of K::D. > > Once again, I apologize for the breakage to your module, > and sincerely hope that this will be the last time that happens. > > All the best, > > Damian
Subject: Re: [rt.cpan.org #131661] Use of K::D::Arg for optional arguments breaks API and is not documented as such
Date: Sat, 15 Feb 2020 05:13:34 +0000
To: bug-Keyword-Declare [...] rt.cpan.org
From: Damian Conway <damian [...] conway.org>
Glad to have been able to sort it out, Chris. Thanks again for the report. Damian


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.