[htmltmpl] Re: Patches for HTML-Template-2.5.tar.gz

[Yunliang Yu <yu...@ma...>]

Hi Sam, thanks for your quick reply. Here I'll try to answer your
questions and clarify a few things.

> > 1. add extended expression support without impact on speed
>
> How did you test the speed of your changes?  Did you also check the memory
> impact?  Changing VAR objects from scalars to hashes will probably greatly
> expand the memory usage of large templates.
>
> Also, you're using string eval() to execute expressions.  Are you really
> sure that's faster than HTML::Template::Expr?  I wouldn't expect it to be,
> but I could be wrong.  Also, do your changes pass the HTML::Template::Expr
> test suite?
>
> > 		<TMPL_IF NAME="item ne 'pizza' && price != 0.99">
>
> I really don't like putting expressions into the NAME attribute.  I think
> EXPR, as used in HTML::Template::Expr, is a better choice.

I ran a test with a simple template with 2 nested loops last night. For
the first test, the template contains no extended expr, just a few
HTML::Template variables. Here are the results on my test
system (standalone readhat 7.3, timed with 'time wget ....' & averaged,
CGI, results should be similar for mod-perl):

	Process with standard HTML::Template     1.52s
	Process with my patched version          1.56s
        Process with HTML::Template::Expr        2.39s

       => patched version has 2.4% overhead, ::Expr 57% overhead

For the 2nd test, insert <TMPL_VAR EXPR="test ne 'pizza'"> in the inner
loop, btw, the data was pulled from a local mysql server and the loops
returns 432 records.

	Process with my patched version         1.88s
        Process with HTML::Template::Expr       3.07s

      => HTML::Template::Expr is 63% slower than the patched version.

I didn't test memory usages, but changing "$value" to "->{value}=$value"
adds a hash with only one element and shouldn't have too much impact.

And, I may upload an optimized version today when I get some time.

>
> > 2. clean up global_vars implementation
>
> Why did you force global_vars on for associate?  What does
> explicit_global_vars do exactly?  It's not documented.

$global_vars and $options->{global_vars} are totally different. Turning on
$global_vars doesn't turn global_vars on. The main reason that I made the
changes is to avoid global_vars and have expr support.

>
> > 3. add a new way to access outside variables without global_vars
> >
> > 		This is a normal variable: <TMPL_VAR NORMAL>.<P>
> > 		<TMPL_LOOP NAME=FROOT_LOOP>
> > 		   Use it inside the loop: <TMPL_VAR __NORMAL><P>
> > 		</TMPL_LOOP>
>
> I really don't like this.  How is an HTML designer supposed to understand
> this bizarre syntax?

Our HTML designers seem to do fine. I used to use the ::name notation (as
in C++). The designers we have know zero about perl.

>
> > 4. TMPL_VAR can now have a default value
> >
> >         <TMPL_VAR NAME="PARAMETER_NAME" VALUE="default value">
>
> Some people will like this, I imagine.  Maybe call it "DEFAULT"?  I think
> I might take this as a separate patch.  What do others think?

'VALUE=' was chosen so it looks similar to:
  <INPUT TEXT NAME="Name" VALUE="Department of Mathematics">

>
> > 5. add TMPL_DEF tag for defining new parameters
> >
> >         <TMPL_DEF NAME="NAME" VALUE="value">
>
> I don't like this.  Mixing display logic with setup of variables is a very
> dangerous thing.

In one project, every page is designed by users, the programmer can't tell
what data the page wants just by looking at the url. TMPL_DEF is the way
the user passes requests back to the programmer. This way the user can
create any url with any data, and the programmer only computes what's
needed from the backend database. TMPL_DEF was introduced since it has
not output.

>
> > 6. add __count__ to loops
> >
> >          <TMPL_VAR NAME="__count__">-th record in the loop.
>
> Well, this something people are perpetually asking for, so I suppose I
> should add it at some point.  It's just so easy to do when you need it
> that I never considered it a priority.  Can you send me this one as a
> separate patch too?

Best Regards,
-yu