Discussion:
[jifty-devel] RFC: Improved Template::Declare Wrappers
David E. Wheeler
2009-11-09 18:47:25 UTC
Permalink
Shifty Jiftys,

I've been thinking about Template::Declare wrappers since I [blogged](http://www.justatheory.com/computers/programming/perl/catalyst/template-declare-wrapper.html
) about them last week. They just feel way too bolted on. This is my
fault, as I created them without really understanding how
Template::Declare worked. But now that I do understand, I'd like to
propose a new API for wrappers. My inspiration is Mason autohandlers,
FWIW.

First, wrappers would be declared just like other templates, but with
the `wrapper` keyword, like so:

wrapper template wrap => sub {
my $self = shift;
};

As with all other templates, arguments would be passed as normal. They
would also have paths, just like other templates. However, you'd be
limited to one wrapper template per path level per dispatch class.
Thus, given the above wrapper, creating a second one like this would
result in an error:

wrapper template foo => sub {};

Putting the wrapper in a subpath, however, would not cause an error:

wrapper template => 'politics/wrapper' => sub {};

Similarly, mixing in or aliasing templates would mix in and alias
wrappers, and since they'd usually go into subpaths, the limitation of
one wrapper per path level would continue to hold.

The wrapper template would dispatch the rest of the template stack by
calling a method on the invocant, like so:

wrapper template wrap => sub {
my $self = shift;
html {
body {
$self->next;
};
};
};

`next` may or may not be a great name for the method. But the
advantage is that it would dispatch other wrapper templates found in
the paths, just like Mason autohandlers (though no inheritance). So
say that we have the wrap wrapper template above, and then

wrapper template 'politics/wrapper => sub {
my $self = shift;
h1 { 'Politics' }
div {
id is 'politics';
$self->next;
};
};

Then say that we have a couple of standard templates:

template story => sub {
my $self = shift;
for my $p (@args) { p { $p } }
};

template politics/story => sub {
my $self = shift;
div { 'Send feedback to us!' };
for my $p (@args) { p { $p } }
}

Then executing the 'story' template would yield output like this:

<html>
<body>
<p>first graph</p>
</body>
</html>

And executing the 'politics/story' template would yield something like:

<html>
<body>
<h1>Politics</h1>
<div>Send feedback to us!</div>
<div id="politics">
<p>first graph</p>
</div>
</body>
</html>

Thoughts?

Best,

David
Jesse Vincent
2009-11-12 18:11:18 UTC
Permalink
Post by David E. Wheeler
I've been thinking about Template::Declare wrappers since I [blogged](http://www.justatheory.com/computers/programming/perl/catalyst/template-declare-wrapper.html
) about them last week. They just feel way too bolted on. This is my
fault, as I created them without really understanding how
Template::Declare worked. But now that I do understand, I'd like to
propose a new API for wrappers. My inspiration is Mason autohandlers,
FWIW.
Hrm. I think I'd be a lot happier separating the "autohandler" concept from
a more generic "wrapper" concept.

In my ideal world, I'd be able to use different wrappers for different
templates at the same "level" - in apps with dispatchers, sometimes the
package and desired wrappers don't match up exactly 1 to 1.

I'd love to be able to say:


wrap template 'foo' with 'my_gorgeous_page_layout' => sub {


};

I can totally see a good argument for a package level default wrapper.
Maybe somethign like:


default wrapper 'my_ugly_wrapper';


( multiple default wrappers would be an error. )


-j
David E. Wheeler
2009-11-12 19:23:08 UTC
Permalink
Post by Jesse Vincent
Hrm. I think I'd be a lot happier separating the "autohandler" concept from
a more generic "wrapper" concept.
Well, I'm inspired by autohandlers, but these are really just wrappers. The only thing I've borrowed is resolving wrappers in subdirectories of the path and executing them all.
Post by Jesse Vincent
In my ideal world, I'd be able to use different wrappers for different
templates at the same "level" - in apps with dispatchers, sometimes the
package and desired wrappers don't match up exactly 1 to 1.
I'm not proposing wrappers at the package level, but at the path level.
Post by Jesse Vincent
wrap template 'foo' with 'my_gorgeous_page_layout' => sub {
};
Hrm. I was trying to get away from individual templates having to identify what wrappers they use. You can already do this with the current wrapper implementation (though it's still bolted on, we can make it a bit neater).
Post by Jesse Vincent
I can totally see a good argument for a package level default wrapper.
default wrapper 'my_ugly_wrapper';
( multiple default wrappers would be an error. )
Yeah, that's how TT wrappers work. And the only difference in what I'm suggesting, aside from syntax, is that you can have multiple wrappers execute as long as they're in different path levels. SO if you execute the template `/foo/bar`, then a wrapper in / would execute, as would a wrapper in /foo (if either or both exist, of course).

Best,

David
David E. Wheeler
2009-11-27 10:48:17 UTC
Permalink
No further comment on this? I'm back from Japan this weekend, and back to work on Monday, so might look at implementing something soon.

Best,

David
Post by David E. Wheeler
Post by Jesse Vincent
Hrm. I think I'd be a lot happier separating the "autohandler" concept from
a more generic "wrapper" concept.
Well, I'm inspired by autohandlers, but these are really just wrappers. The only thing I've borrowed is resolving wrappers in subdirectories of the path and executing them all.
Post by Jesse Vincent
In my ideal world, I'd be able to use different wrappers for different
templates at the same "level" - in apps with dispatchers, sometimes the
package and desired wrappers don't match up exactly 1 to 1.
I'm not proposing wrappers at the package level, but at the path level.
Post by Jesse Vincent
wrap template 'foo' with 'my_gorgeous_page_layout' => sub {
};
Hrm. I was trying to get away from individual templates having to identify what wrappers they use. You can already do this with the current wrapper implementation (though it's still bolted on, we can make it a bit neater).
Post by Jesse Vincent
I can totally see a good argument for a package level default wrapper.
default wrapper 'my_ugly_wrapper';
( multiple default wrappers would be an error. )
Yeah, that's how TT wrappers work. And the only difference in what I'm suggesting, aside from syntax, is that you can have multiple wrappers execute as long as they're in different path levels. SO if you execute the template `/foo/bar`, then a wrapper in / would execute, as would a wrapper in /foo (if either or both exist, of course).
Best,
David
_______________________________________________
jifty-devel mailing list
http://lists.jifty.org/cgi-bin/mailman/listinfo/jifty-devel
Jesse Vincent
2009-12-01 18:26:16 UTC
Permalink
I've been looking more at your proposal. Given that you want wrappers to
mix in and have names, it feels a little weird to force them to be "only
one per path level."

-j
David E. Wheeler
2009-12-01 19:48:49 UTC
Permalink
Post by Jesse Vincent
I've been looking more at your proposal. Given that you want wrappers to
mix in and have names, it feels a little weird to force them to be "only
one per path level."
So how would you resolve them if there were two on the same path level in the same dispatch class?

I don't care if they have names, frankly. So maybe the syntax could just be:

wrapper '/' => sub { };

Maybe if there's no path, it could default to the root path as declared:

wrapper sub { };

For a subpath you'd have to be explicit:

wrapper '/politics' => sub { };

Better?

Best,

David
Jesse Vincent
2009-12-01 20:16:06 UTC
Permalink
Post by David E. Wheeler
Post by Jesse Vincent
I've been looking more at your proposal. Given that you want wrappers to
mix in and have names, it feels a little weird to force them to be "only
one per path level."
So how would you resolve them if there were two on the same path level in the same dispatch class?
I don't have a good answer for that, short of "declaration order" or
some similar insanity. Your argument was pushing me toward "no names"
rather than "multiple per package and level"
Post by David E. Wheeler
wrapper '/' => sub { };
I wonder if making it imperative makes sense? wrap instead of wrapper.

wrap '/' => sub { };

When we're doing template resolution, "/" still means at the root, not "the
current package", right? That would lead me to want to use relative
pathing syntax here. '.' or './' or _something_, since '/' means something
fairly specific already.

Really, when we start specifying paths and subpaths, for these wrappers
to wrap, I start reaching for a path based dispatcher with globbing
and/or regexes, ala Path::Dispatcher or Jifty::Dispatcher, but I know
that's not the hammer you're looking for. Maybe we really do just want
one wrapper per package scope.
Post by David E. Wheeler
wrapper sub { };
wrapper '/politics' => sub { };
David E. Wheeler
2009-12-01 20:30:45 UTC
Permalink
Post by Jesse Vincent
I don't have a good answer for that, short of "declaration order" or
some similar insanity. Your argument was pushing me toward "no names"
rather than "multiple per package and level"
Me too.
Post by Jesse Vincent
Post by David E. Wheeler
wrapper '/' => sub { };
I wonder if making it imperative makes sense? wrap instead of wrapper.
wrap '/' => sub { };
Well, none of the other keywords are imperative. It's `template`, not `create_template`. Similarly for language keywords for declaring things (`sub`, `method`, `class`, etc.).
Post by Jesse Vincent
When we're doing template resolution, "/" still means at the root, not "the
current package", right?
Correct.
Post by Jesse Vincent
That would lead me to want to use relative
pathing syntax here. '.' or './' or _something_, since '/' means something
fairly specific already.
That would be allowable, yes.
Post by Jesse Vincent
Really, when we start specifying paths and subpaths, for these wrappers
to wrap, I start reaching for a path based dispatcher with globbing
and/or regexes, ala Path::Dispatcher or Jifty::Dispatcher, but I know
that's not the hammer you're looking for. Maybe we really do just want
one wrapper per package scope.
Things don't resolve that way, really. Templates already resolve to paths; so should wrappers.

Best,

David
Jesse Vincent
2009-12-07 17:54:26 UTC
Permalink
Post by David E. Wheeler
Post by Jesse Vincent
I don't have a good answer for that, short of "declaration order" or
some similar insanity. Your argument was pushing me toward "no names"
rather than "multiple per package and level"
Me too.
Post by Jesse Vincent
Post by David E. Wheeler
wrapper '/' => sub { };
I wonder if making it imperative makes sense? wrap instead of wrapper.
wrap '/' => sub { };
Well, none of the other keywords are imperative. It's `template`, not `create_template`. Similarly for language keywords for declaring things (`sub`, `method`, `class`, etc.).
I guess my thought process was that none of the other keywords actually
are imperative - they don't apply action at a distance.
Post by David E. Wheeler
Post by Jesse Vincent
When we're doing template resolution, "/" still means at the root, not "the
current package", right?
Correct.
Post by Jesse Vincent
That would lead me to want to use relative
pathing syntax here. '.' or './' or _something_, since '/' means something
fairly specific already.
That would be allowable, yes.
Post by Jesse Vincent
Really, when we start specifying paths and subpaths, for these wrappers
to wrap, I start reaching for a path based dispatcher with globbing
and/or regexes, ala Path::Dispatcher or Jifty::Dispatcher, but I know
that's not the hammer you're looking for. Maybe we really do just want
one wrapper per package scope.
Things don't resolve that way, really. Templates already resolve to paths; so should wrappers.
*grumble* Given that, I think we should prohibit wrappers from wrapping
things outside the namespace they contain. IE, absolute "/" would be either
be defined as the root of the current package OR it would be forbidden.

-j
David E. Wheeler
2009-12-07 21:34:58 UTC
Permalink
Post by Jesse Vincent
Post by David E. Wheeler
Well, none of the other keywords are imperative. It's `template`, not `create_template`. Similarly for language keywords for declaring things (`sub`, `method`, `class`, etc.).
I guess my thought process was that none of the other keywords actually
are imperative - they don't apply action at a distance.
Yeah, but it's an inconsistent interface. I get that you might want to have some sort of marker to demonstrate that it differentiates a bit, but it just seems too weird to me given the rest of T::D.
Post by Jesse Vincent
Post by David E. Wheeler
Things don't resolve that way, really. Templates already resolve to paths; so should wrappers.
*grumble* Given that, I think we should prohibit wrappers from wrapping
things outside the namespace they contain. IE, absolute "/" would be either
be defined as the root of the current package OR it would be forbidden.
Why would you want to limit it in that way? Personally, I might want to put wrappers in their own packages and import them where appropriate.

Best,

David
Shawn M Moore
2009-12-10 15:17:00 UTC
Permalink
Hey David and Jesse,

I like the concept of what you're proposing, David.

In the Jifty world, Jifty::Dispatcher typically provides most of the features
for which autohandlers are used. I don't Catalyst well enough to know its
analogue, but surely it must have one (or more).

In looking through the RT code, we use autohandlers for a number of reasons.
All of these features are solved quite well by Jifty::Dispatcher:

* abort if the user does not have permission to use the given template (such as
everything under /Admin if you do not have a specific right)
* abort if some other arbitrary condition is not met (such as misconfiguration,
as in the case of Shredder)
* perform some nonstandard action besides "execute this file as a mason
template" (such as optimizing the common case of serving a binary file)
* redirect the user if some condition is met (such as being in "install mode",
which sends the user somewhere else)
* send extra HTTP headers (as in the REST interface)

Only in one place do we actually inject additional HTML. In the global
/autohandler we add a footer to each page. (In RT this is asymmetrical only
because calling the header template is how each page specifies its title, tabs,
etc)

That our real-world use of autohandler is almost entirely controller-level
rather than view-level suggests that we should come up with a better metaphor
than autohandler for what we want in TD here. I think this is what Jesse was
Post by Jesse Vincent
Hrm. I think I'd be a lot happier separating the "autohandler" concept from
a more generic "wrapper" concept.
Now, skipping down to the latest post.. :)
Post by Jesse Vincent
Post by David E. Wheeler
So how would you resolve them if there were two on the same path level in th
e same dispatch class?
Post by Jesse Vincent
I don't have a good answer for that, short of "declaration order" or
some similar insanity. Your argument was pushing me toward "no names"
rather than "multiple per package and level"
I definitely prefer getting rid of wrapper names; don't see a real need for
them. For multiple wrappers, declaration order works. Or, for now, defer
the decision by making it an error to declare more than one. That affords us
forward-compat.
Post by Jesse Vincent
When we're doing template resolution, "/" still means at the root, not "the
current package", right? That would lead me to want to use relative
pathing syntax here. '.' or './' or _something_, since '/' means something
fairly specific already.
Ew. This makes me want to get rid of specifying paths for wrappers entirely.
Post by Jesse Vincent
Really, when we start specifying paths and subpaths, for these wrappers
to wrap, I start reaching for a path based dispatcher with globbing
and/or regexes, ala Path::Dispatcher or Jifty::Dispatcher, but I know
that's not the hammer you're looking for. Maybe we really do just want
one wrapper per package scope.
Yeah, agreed.

I'll detail my counterproposal.. proposals! in a followup mail.

Shawn
Shawn M Moore
2009-12-10 15:55:45 UTC
Permalink
Post by Shawn M Moore
I'll detail my counterproposal.. proposals! in a followup mail.
Okay so I think it's already been decided that we don't need to name
wrappers. Good.

I think we can get away with not allowing subpaths for wrappers.
Presumably you would use such a thing when you want to wrap only a
subset of the templates in the package. But then why not just make a new
package and use mix/alias?

So, if we have nameless wrappers that always apply to '/' (or rather, '.'),
we can have shiny syntax:

wrapper {
my ($self, @args) = @_;
html {
body {
inner(@args); # or maybe just "inner;" a la Moose
}
}
};


Another option is to specify wrappers in calls to "mix" and "alias". I
don't know if there's a good use case for this.

alias 'CRUD' under '/crud', wrapper {
h1 { "Yay CRUD!" };
inner;
p { "Back to your regularly scheduled slog." };
};

If we do add this, it should be a separate feature. I don't like that
it's kind of action-at-a-distancey.

Shawn
David E. Wheeler
2009-12-10 17:50:45 UTC
Permalink
Post by Shawn M Moore
Okay so I think it's already been decided that we don't need to name
wrappers. Good.
I think we can get away with not allowing subpaths for wrappers.
Presumably you would use such a thing when you want to wrap only a
subset of the templates in the package. But then why not just make a new
package and use mix/alias?
Because templates don't have that requirement.
Post by Shawn M Moore
So, if we have nameless wrappers that always apply to '/' (or rather, '.'),
wrapper {
html {
body {
}
}
};
Yes, agreed, that's nice. We could certainly start with that, and then perhaps support paths with a different keyword, something like:

under '/foo' put wrapper { ... }

That allows us to keep the clean syntax.
Post by Shawn M Moore
Another option is to specify wrappers in calls to "mix" and "alias". I
don't know if there's a good use case for this.
Cool idea, but I think that's starting to overload alias too much. I don't see a use case for it, so I don't really see a need for it. No need to add something no one has asked for, eh?

Best,

David

David E. Wheeler
2009-12-10 17:47:16 UTC
Permalink
Post by Shawn M Moore
I like the concept of what you're proposing, David.
In the Jifty world, Jifty::Dispatcher typically provides most of the features
for which autohandlers are used. I don't Catalyst well enough to know its
analogue, but surely it must have one (or more).
No, it leaves that sort of thing to the templating systems. Otherwise I likely wouldn't be doing this.
Post by Shawn M Moore
Only in one place do we actually inject additional HTML. In the global
/autohandler we add a footer to each page. (In RT this is asymmetrical only
because calling the header template is how each page specifies its title, tabs,
etc)
I use them a lot in Bricolage templates for creating the standard HTML header and footer sections for every page on a site or section. Extremely handy for that. I wish I understood them better when we wrote Bricolage itself, as it makes almost no use of autohandlers at all.
Post by Shawn M Moore
That our real-world use of autohandler is almost entirely controller-level
rather than view-level suggests that we should come up with a better metaphor
than autohandler for what we want in TD here. I think this is what Jesse was
Well, no. That RT and Bricolage don't making great use of authohandlers doesn't mean that the concept is flawed. That's not how Mason thinks about them.
Post by Shawn M Moore
Post by Jesse Vincent
Hrm. I think I'd be a lot happier separating the "autohandler" concept from
a more generic "wrapper" concept.
Now, skipping down to the latest post.. :)
I agree they should be separated. I was relying on the idea of autohandlers for their dispatch mechanism, but not for inheritance or the other cool stuff they support. My idea is really just wrappers, but allows them in subdirectories and they cascade their execution. That's all I've borrowed from autohandlers, and I wouldn't (and don't) call them autohanders in TD.
Post by Shawn M Moore
I definitely prefer getting rid of wrapper names; don't see a real need for
them. For multiple wrappers, declaration order works. Or, for now, defer
the decision by making it an error to declare more than one. That affords us
forward-compat.
Agreed.
Post by Shawn M Moore
Post by Jesse Vincent
When we're doing template resolution, "/" still means at the root, not "the
current package", right? That would lead me to want to use relative
pathing syntax here. '.' or './' or _something_, since '/' means something
fairly specific already.
Ew. This makes me want to get rid of specifying paths for wrappers entirely.
I think we can throw exceptions for those kinds of path specs. After all, one doesn't execute wrappers from within templates. They're handled by TD automatically. So it makes no sense to declare them in any directory other than where they should be.
Post by Shawn M Moore
Post by Jesse Vincent
Really, when we start specifying paths and subpaths, for these wrappers
to wrap, I start reaching for a path based dispatcher with globbing
and/or regexes, ala Path::Dispatcher or Jifty::Dispatcher, but I know
that's not the hammer you're looking for. Maybe we really do just want
one wrapper per package scope.
Yeah, agreed.
I'll detail my counterproposal.. proposals! in a followup mail.
Will do.

David
Loading...