* Simon Wilcox <essuu at ourshack.com> [2003/12/17 10:25]:
> OK. Here it is. Be gentle.

[-- snip --]

> Comments welcome.

Some comments:

Generating a linked breadcrumb is likely going to be one of the most
used pieces of this.  I'd maybe include a specific method to do so,
called something like generate_breadcrumb_trail, that takes a uri and a
string to join nodes on.  At the very least include an example macro in
the documentation that does the same thing.

The constructor takes a path; perhaps this should be considered relative
to INCLUDE_PATH, unless it is absolute.  You can grab the content of a
file that lives somewhere in INCLUDE_PATH by (inappropriately) using the
Template::Context objects' insert method:

    sub new {
        # as before

        if (!file_name_is_absolute($datafile) && ref($context)) {
            my $content = $context->insert($datafile);
            $self->{ _xp } = XML::XPath->new(xml => $content);
        }
        else {
            $self->{ _xp } = XML::XPath->new(filename => $datafile);
        }

        # continue as before

The filename portion (index.html, in _findme) could be trivially parameterized.

I think that's it.  Overall, it looks good.

(darren)

-- 
All extreme positions are wrong.
    -- Erann Gat

Attachment: pgp00000.pgp
Description: PGP signature

Reply via email to