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

[Michael Graham]

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>