Re: triggers and the single meta-attr object
On Sun, Mar 29, 2009 at 09:34:32PM -0400, Sartak wrote: On Sun, Mar 29, 2009 at 9:25 PM, Stevan Little stevan.lit...@iinteractive.com wrote: If we want to keep it, lets keep it, but if we don't really have a reason, lets just get rid of it. Get rid of it. We don't pass in the meta-attr for default or builder either. It's just going to bite us in the future when we try to avoid generating metaobjects. It's not like its removal will hinder anybody. It's gone. http://github.com/nothingmuch/moose/commit/c2685d2054e3f63cf38fca4fad1074b4a2e5be83 hdp.
Re: triggers and the single meta-attr object
On Sun, 29 Mar 2009, Hans Dieter Pearcey wrote: I don't know how to weigh these two concerns, though: * It has been documented and working for as long as I can remember that triggers receive the meta-attr object. That's not true. For a long time, immutablized classes have not passed the meta-attr to the trigger (except from the constructor). -dave /* http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) */
Re: triggers and the single meta-attr object
Personally, I don't think we need to pass that meta-attribute, if you really want/need it, then you can do this: trigger = sub { my $self = shift; $self-meta-find_attribute_by_name('foo')-... ... } The only tricky part of the above workaround is that you need to know the name of the attribute in order to get the meta-attribute. But this shouldn't be too hard to fix/work-around in most cases. My vote is for *not* passing the meta-attribute and document the above work around in Moose::Manual::Delta. - Stevan On Mar 29, 2009, at 12:32 AM, Hans Dieter Pearcey wrote: In December, mst committed ec2e2ee5a0f010fe09d57e0176717b6b4f5671a2, which removes the meta-attr object as the third argument to triggers, saying unsupport passing meta-attr object to triggers because (a) it's not tested (b) it's not documented (c) it makes it impossible to not close over the meta-attr objects As far as I can tell, though, it *was* documented and had been for a long time. (Also, that commit only removed this behavior from inlined accessors and constructors, so anyone not using make_immutable still got the meta- attr object passed to their triggers if the constructor invoked them.) A month later, nothingmuch merged in a big branch that reinstated the passing of the meta-attr object to triggers in inline constructors, but I couldn't find any comment about why this was done. (59f5bbde66d61d15b684be88d138fd798ba851d0) Today, http://rt.cpan.org/Public/Bug/Display.html?id=44429 summarizes the problem: writers (inline or not) invoking a trigger don't pass in the meta-attr. Moose::Manual::Attributes document that they do and Moose document that they don't. There aren't tests one way or another. We need to pick one and stick to it. I don't want another fix (in whichever direction) that someone else undoes a month later. If it's change the code to match the docs, that's done in http://github.com/hdp/moose/commit/b0871377ee1fb633784cf69cd4d096a0c3183493 If it's change the docs to match the code, that's easy; remove the mention of the meta-attribute object from Moose::Manual::Attributes. I don't know how to weigh these two concerns, though: * It has been documented and working for as long as I can remember that triggers receive the meta-attr object. * Passing the meta-attr object to triggers rules out some theoretical performance improvements. (When I say it like it that, it sounds like we should change the code to match the docs, but optimizing Moose performance is something I have no experience with.) hdp.
Re: triggers and the single meta-attr object
On Sun, Mar 29, 2009 at 11:40:43AM -0400, Stevan Little wrote: Personally, I don't think we need to pass that meta-attribute, if you really want/need it, then you can do this: trigger = sub { my $self = shift; $self-meta-find_attribute_by_name('foo')-... ... } The only tricky part of the above workaround is that you need to know the name of the attribute in order to get the meta-attribute. But this shouldn't be too hard to fix/work-around in most cases. What about passing in the attribute name instead of the meta-attribute? Then there's no trickery necessary at all. Or a coderef that wraps up getting the necessary objects: sub { Moose::Util::find_meta($class_name)-get_attribute($attr_name) } which requires closing over no objects. hdp.
Re: triggers and the single meta-attr object
On Sun, Mar 29, 2009 at 9:25 PM, Stevan Little stevan.lit...@iinteractive.com wrote: If we want to keep it, lets keep it, but if we don't really have a reason, lets just get rid of it. Get rid of it. We don't pass in the meta-attr for default or builder either. It's just going to bite us in the future when we try to avoid generating metaobjects. It's not like its removal will hinder anybody. Shawn
Re: triggers and the single meta-attr object
On Sun, Mar 29, 2009 at 09:43:45AM -0500, Dave Rolsky wrote: That's not true. For a long time, immutablized classes have not passed the meta-attr to the trigger (except from the constructor). ec2e2ee5 is mst's commit (Dec 9 2008) removing the meta-attr argument. % git blame -M -w lib/Moose/Meta/Method/Accessor.pm ec2e2ee5^ | fgrep '$attr-trigger' d617b644 (Stevan Little2006-11-02 21:02:04 + 231) return sprintf('$attr-trigger-(%s, %s, $attr);', $instance, $value); It worked for two years before mst turned it off, as far as I can tell. hdp.