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.
