Re: Mech JS support

[Father Chrysostomos]

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.

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.

Also, I would guess that separate init() and options() methods would be

        ($plugins->{$plugin} =
            "WWW::Mechanize::Plugin::$plugin"->init($self))
             ->options(@opts);

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: (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

into the $mech object?

do the plugin class-methods do for state storage? They just throw keys

They use the plugin object.

It seems that you can have multiple plugins, but they overwrite
callbacks as they're installed?

Father Chrysostomos

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.

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