Michael Parker writes:
>Justin Mason wrote:
>> Michael Parker writes:
>>> Michael Parker wrote:
>>>> [EMAIL PROTECTED] wrote:
>>>>> Author: jm
>>>>> Date: Wed Oct 25 05:57:25 2006
>>>>> New Revision: 467629
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=467629
>>>>> Log:
>>>>> add public API to allow plugins to add methods to @TEMPORARY_METHODS.
>>>>> Michael, may need to redo parts of this with backporting from yr branch,
>>>>> but it's required in trunk to fix a test failure here
>>>> Hmmm. So, I made finish_tests a plugin method so that each plugin can
>>>> handle this stuff on its own. I'd rather NOT make this a supported
>>>> public plugin method, in favor of the other.
>>>>
>>>> Thoughts? Besides, getting off my ass and making the other just work.
>>> To be clear, I'd rather keep finish_tests method and do away with the
>>> register_generated_rule_method method.
>>
>> I was thinking about that -- however, for cleaning up temporary methods,
>> which almost all rule types will be doing, it seems to be unneccessary
>> duplication to have each plugin contain this code:
>>
>> our @TEMPORARY_METHODS = ();
>>
>> sub build_rule_methods_or_whatever {
>> ...
>> push @TEMPORARY_METHODS,
>> "Mail::SpamAssassin::Plugin::RuleType::".$method;
>> ...
>> }
>>
>> sub finish_tests {
>> my ($self) = @_;
>> foreach my $method (@TEMPORARY_METHODS) {
>> if (defined &{$method}) {
>> undef &{$method};
>> }
>> }
>> }
>>
>>
>> for what it's worth, I agree that finish_tests() should be a public API,
>> *too*, since some rule type plugins may need to do other stuff. But
>> since all rule types will duplicate the @TEMPORARY_METHODS code, why not
>> let the API do the legwork and keep the plugins simpler?
>>
>
>But then you have to expose internal workings, for instance:
>+sub register_generated_rule_method {
>+ my ($self, $nameofsub) = @_;
>+ push @Mail::SpamAssassin::PerMsgStatus::TEMPORARY_METHODS,
>+ $nameofsub;
>+}
>
>Who says that TEMPORARY_METHODS is where method names are stored in the
>future? In fact in the Check plugin branch that variable no long exists.
I agree ;) the API is actually used like this:
$pluginobj->register_generated_rule_method(
'Mail::SpamAssassin::Plugin::MIMEHeader::'.$evalfn);
so knowledge of @Mail::SpamAssassin::PerMsgStatus::TEMPORARY_METHODS
in the plugin implementation code isn't required.
>Each plugin should be responsible to deciding internally how they will
>store their generated methods for later cleanup, not make a guess about
>where other plugins store them. That is way too hackish for me.
totally agreed.
>About as far down this path I'd be willing to go might be a public
>method on the PerMsgStatus object, that allowed for the storage of
>generated methods, then you could call that instead.
that sounds pretty similar to the above, then?
> But I see no harm
>in letting plugins clean up their own messes, then you don't force
>plugins to store their information the same way.
--j.
