On Nov 28, 2009, at 11:50 AM, BUCHMULLER Norbert wrote:
On Fri, 27 Nov 2009 21:21:53 -0500 Stevan Little wrote:
- Would you expect this work with Native traits? If so how would you
expect it to work?

No, but that's no problem here - the Native traits do not work with
arrayrefs or regexes, so it's no much difference that they do not work
with these enhanced arrayrefs. (Here 'do not work' means that it asserts
that the value of 'handles' is a hashref.)

Yeah I realized that after I sent the mail.

- What is the order of evaluation? Is that even relevant? What might
be some issues that would bring up?

No, it's irrelevant to my knowledge. (No more than the order of the
element is relevant for the arrayref form.)

Yup, I agree, it is only (possibly) relevant if you don't just accept all matches and add some oddness there, but I think we can all agree that is not a good idea so therefore, yes totally irrelevant.

- Would you support having multiple regexprs in the list? Again how
would order of evaluation be handled? What if it matches for one
regexpr and not for another? Is this a conflict? or do we accept both?

Yes, I would. A method is included if it's present in the list as a
string, or if at least one regex matches it.

Yeah, thats about what I was thinking too.

- How do you handle methods which match the strings and also match the
regexpr? Is the method list made unique before applying? Same for
multiple regexprs?

First see my answer for the previous question. The list is not made
unique, though probably it should be. Anyways, the user can already pass
non-unique lists with the arrayref form:

has foo => (
   isa => 'HashRef[Foo]',
   is => 'rw',
   handles => [
       'bar',
       'quux',
       'bar',
   ],
);

If we demand that the arrayref+regex form always results in an unique
list, then we also have to make sure that the arrayref results in an
unique list (as the arrayref is a special form of the arrayref+regex
form - when the number of regexes is 0).

Yeah, I doubt we ever tested for multiples in the array ref cause it would surely be user-error and the end result of the operation is still the same as if there was not multiples (it just adds extra work to it). I suspect that running List::Util::uniq on the arrayref list would be a smart fix.

I would like to make sure these items are thought through before we
consider putting this feature in the core. I will also point out that
it is the policy (in Moose::Manual::Contributing) that something like
this be first tested out in a MooseX:: module.

Ok, I already read that document before I set off hacking on this, but I was not sure if that applies for this feature (it breaks no existing code
to my knowledge, and is just the logical extrapolation of some other
feature).

Well it applies to all new features, but smaller ones (like this) can be exceptions if reviewed and judged to be small and safe enough to include.

Then I'll create Just Another CPAN Module That Nobody Uses But Me.. :-)

Don't be so negative, if it is a good idea then others will use it.

So, the more I think about it (and discussed with another core dev) the less I think it needs to be a MooseX:: (especially since your answers to my pokes above were to keep it simple). I just ask that you fix the uniqueness issue, add some docs, a changelog entry, and your name to the contributors list in Moose.pm.

I am not sure what the policy is in terms of pulling from github forks, you will have to check with those more git savvy then I. They might want you to move it into a topic branch in the git.moose.perl.org repo though.

- Stevan












Reply via email to