On Jul 19, 2007, at 12:19 AM, Eric Wilhelm wrote:
# from Father Chrysostomos
# on Wednesday 18 July 2007 10:26 pm:
for everyoneʼs perusal.
I'm seeing this a lot.
+ $self->_extract_forms() unless $self->{_extracted_forms};
Given that it is a void-context and is a noop if _extracted_forms is
false, why not just put
return unless $self->{_extracted_forms}
I hope you mean 'if', as opposed to 'unless'. :-)
inside the _extract_forms() method?
In the current_form and forms methods, I'm trying to follow the style
of the existing code (see the links method, for instance). I'll
change it if Andy says to.
But I do agree that it is redundant to repeat this line elsewhere.
And in a couple of cases (form_name and form_with_fields) it was
completely unnecessary. Thank you for catching this.
In the remaining cases, I can change $self->{form} to $self-
>current_form and $self->{forms} to $self->forms.
Also, I would guess that separate init() and options() methods
would be
preferable in writing plugins rather than having init() handle options
and self but options() gets just options. Similarly, the init() call
should get $self as the first parameter such as:
my $plugin_init = "WWW::Mechanize::Plugin::$plugin" . '::init';
$self->$plugin_init()
And similarly for $self->$plugin_options(@opts).
That is, unless the plugin is an object, which maybe it should be.
Which is what I intended. I think your idea of keeping init and
options separate, such that init never gets options is good and makes
the interface cleaner. How about this:
($plugins->{$plugin} =
"WWW::Mechanize::Plugin::$plugin"->init($self))
->options(@opts);
(Now the thought has just occurred to me that, if a plugin has '::'
in its name, we might want to allow a hyphen instead).
What
do the plugin class-methods do for state storage? They just throw
keys
into the $mech object?
They use the plugin object.
It seems that you can have multiple plugins, but they overwrite
callbacks as they're installed?
No, they get added to a list of callbacks for a particular hook (see
the push statement in add_handler). A callback can call
WWW::Mechanize::next_handler to signal Mech try the next one. Yes,
this does need documentation.
Attached are two patches. Mech-plugins-2-from-scratch.diff is a diff
between my working copy and Mech 1.30. Mech-plugins.diff is a diff
between the working copy and the last patch. Look at whichever of the
two is easier for you.
Father Chrysostomos
Mech-plugins-2-from-scratch.diff
Description: Binary data
Mech-plugins-2.diff
Description: Binary data
WWW-Mechanize-Plugin-JavaScript-0.0.2.tar.gz
Description: GNU Zip compressed data
| 
