[cgiapp] [patch] CAP::MessageStack + CAP::FormState

[Alex]

Hi All!

a) hours
I'll try to do spend the hours this weekend.

b) CAP::TT
CAP::TT should IMHO set tmpl_class, as it was introduced exactly to be able
to determine, what templating mechanism is used. As it uses
Template::Toolkit behind the scenes, it should set it to that. Maybe someone
who is more familiar with it should think about what is better, Template or
TT. I would prefer TT, as it seems to be the eponym to CAP::TT.

c) AnyTemplate
I'm not shure if I didn't get it wrong, but doesn't CAP::AnyTemplate must
know, what templating engine (is that really the term for it?) is used when
calling the load_tmpl hook? I'm pretty shure, this must be deterministic. So
depending on the templating engine used, CAP::AnyTemplate could set up
tmpl_class as any other templating plugin does. That what I think.

d) die_on_bad_params option defaults to true
That's true. But as this behavior is deterministic, we can safely assume
that if die_on_bad_params is not present, or not 0, it is 1, can't we?

Regards, Alex

PS: The CAP::Authentication patch is done! *yay*
 
-----Ursprüngliche Nachricht-----
Von: cgiapp-bounces at lists.openlib.org
[mailto:cgiapp-bounces at lists.openlib.org] Im Auftrag von Michael Graham
Gesendet: Montag, 19. Oktober 2009 15:51
An: cgiapp at lists.openlib.org
Betreff: Re: [cgiapp] [patch] CAP::MessageStack + CAP::FormState


I'm one of the bad authors in question.  I've sat on many bugs for several
years.  I don't have any good excuses.  I apologize, and I'm making an
effort to be better from now on.  I released a new version of AnyTemplate
last night, and closed some ancient tickets.  I hope to get to the rest of
the showstoppers in the next two weeks.

I'm also putting up my modules on github to try to enable easier patching
and co-maintenance.

I would like to address Alex's issues with FormState, however the problem is
actually quite tricky, and I need help.

The basic idea is to do some magic with the template params if (and only
if) the user is using HTML::Template.

We should be able to detect the use of HTML::Template with
$self->tmpl_class.

BUT, both CAP::TT and CAP::AnyTemplate call the load_tmpl hook, and neither
of them sets $self->tmpl_class().  There may be other templating plugins out
there that do the same.

So using this approach, FormState will *always* think that the template
class is HTML::Template.

Another strategy might be to detect the presense of the 'die_on_bad_params'
option.

However, this option defaults to true!  So the *absense* of the option
indicates that die_on_bad_params behaviour should be enabled.

Neither CAP::TT nor CAP::AnyTemplate are going to set that option, so we're
back to where we started.  FormState will *always* think that the template
class is HTML::Template *and* that die_on_bad_params is set.

So we have a bit of a conundrum here.

Should CAP::TT and CAP::AnyTemplate set $self->tmpl_class()?

Should FormState and MessageStack (and other modules that add values to
tmpl_params) hack around this by having their own die_on_bad_params config
options?

Basically, both options involve patches to a lot of modules.

For reference, here's the solution I tried to implement based on Alex's
patch:

  http://github.com/mgraham/CAP-FormState/tree/v0.12_2

There's a failing CAP::TT test to illustrate the problem.

And I'd also like to point out a problem with the original patch:

> if( (!exists $ht_params->{die_on_bad_params} or 
> $ht_params->{die_on_bad_params} = 1)
> 	   and $self->html_tmpl_class() eq 'HTML::Template' and scalar( 
> @$messages ) > 0 ) {

Spot the bug!  This is why it's usually not as simple as "just applying the
patch" to fix the problem.  You have to write test cases, including test
cases for situations you don't normally use yourself (e.g. other templating
systems).  

Have a look at the new FormState tests on github to get an idea of what is
involved to actually implement a change request of this scope.

I'm not trying to make excuses for my Massive Module Maintenance Delinquincy
(TM), but it does take several hours of work to implement a change request
of this size.  If you do those hours of work for me, then believe it or not,
I really am actually less likely to sit on your patch for months.  :)


Michael



--
Michael Graham <magog at the-wire.com>

#####  CGI::Application community mailing list  ################
##                                                            ##
##  To unsubscribe, or change your message delivery options,  ##
##  visit:  http://lists.openlib.org/mailman/listinfo/cgiapp    ##
##                                                            ##
##  Web archive:   http://lists.openlib.org/pipermail/cgiapp/   ##
##  Wiki:          http://cgiapp.erlbaum.net/                 ##
##                                                            ##
################################################################