[Catalyst-dev] RFC: Catalyst::View::Mason::Templated

[Sebastian Willert]

On Tue, 2007-09-18 at 08:46 -0500, Dave Rolsky wrote:
> On Tue, 18 Sep 2007, Sebastian Willert wrote:
> 
> > I found myself in a Catalyst coding spree lately (or rather in a
> > packaging spree for quite old code) so I present my second request for
> > comments tonight. I use HTML::Mason extensively for most of my projects
> > and was not really satisfied with the amount of features accessible
> > through Catalyst::View::Mason so I whipped up my own some time ago.
> 
> I really don't think we need two Catalyst views for Mason. If the 
> Templated base class is "the new thing", that's fine, but there's no need 
> for two views.

In my opinion there is a large need to keep Catalyst::View::Mason
backwards-compatible and I think it is important to stick as closely to
the constraints set by Catalyst::View::Templated if you are to build a
view that is based on it. I guess these two requirements are mutually
exclusive or at least would introduce large amounts of backcompat code
into Catalyst::View::Mason that would be really hard to maintain. I also
don't think that it is a good idea to fiddle around with the inheritance
chain and method resolution of a long established module, especially if
the changes are as radical as needed to use Catalyst::View::Templated.
Nearly no method would be named or called the same which means a world
of pain for custom views (or the module that tries to accomodate for
that) that override some of them.

> > MASON CONFIGURATION
> >  This allows you to to pass additional settings to the
> >  HTML::Mason::Interp constructor or to set the options as below:
> >
> >  "DATA_DIR"
> >    Defaults to "File::Spec->tmpdir"
> >
> >  "ALLOW_GLOBALS"
> >    Defaults to "qw/$c $name $base/"
> >
> >    If you add additional allowed globals those will be prepended to the
> >    list of default globals.
> >
> >  Most of the parameters listet in HTML::Mason::Params are also present
> >  (preverably in uppercase), so you can e.g. use your custom request
> >  class.
> 
> Why uppercase? This is just going to make things a little confusing for 
> people already familiar with the parameters Mason accepts.

Because uppercase is the default case for C::V::Templated. I find it
equally confusing to have both uppercase and lowercase config variables
mixed together and having to remember which variable has to be cased in
which way. So I choose to stick to the C::V::Templated convention but
also allow lowercase variables. Maybe just rephrasing the last paragraph
to "Both lowercase and uppercase variants are acceptable, so you are
free to stick with either the C::V::Templated or the HTML::Mason
convention" would be better, I don't know.

> It might make sense to use upper case for parameters that the view is 
> consuming instead of passing on to Mason, however.
> 
> >       = { INCLUDE_PATH  =>
> >         [ '/usr/local/foo', '/usr/local/foo/widgets',
> >           '/usr/local/bar', '/usr/local/bar/widgets' ],
> >         };
> >
> >    gets transformed to:
> >
> >       MyApp->config->{View::Mason}
> >       = { INCLUDE_PATH  =>
> >         [ [ MAIN => '/usr/local/foo' ],
> >           [ FOO_WIDGETS => '/usr/local/foo/widgets' ],
> >           [ BAR => '/usr/local/bar' ],
> >           [ BAR_WIDGETS => '/usr/local/bar/widgets' ] ],
> >         };
> 
> Is this really necessary. Mason already supports a comp_path parameter 
> that does this, as you obviously know. What is gained by renaming this?
> 
> The ability to give a list of unnamed roots is worthwhile, but maybe you 
> could submit a patch for Mason to do this?

I am once again gaining compatibility with C::V::Templated. INCLUDE_PATH
is introduced and specified in this module and I wanted to stick to it
as closely as possible.

As for writing a patch for Mason itself: I think having named roots is
desirable and well supported within Mason. Mason is also unlikely to
have an external entity to rely on the INCLUDE_PATH to be presented as a
plain array. This does not hold true for Catalyst so I tried to find a
sensible way to unify them and took great care to build sensible root
names from unnamed paths. But maybe it would also be a good idea to
provide a comp_root configuration variable and map the paths given there
back to INCLUDE_PATH as a second option.

To wrap thinks up: I really think Catalyst::View::Templated takes an
desirable approach in trying to unify view configuration and APIs, even
if there are lots of hops to jump through to map between the Mason way
and the C::V::T way. I tried to allow the Mason variant wherever
possible but discouraged it in the documentation because sticking to the
Mason API is contrary to the main motivation behind
Catalyst::View::Templated.

Cheers,
  Sebastian