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

Alex capfan at gmx.de
Sun Oct 11 07:19:04 EDT 2009


Hi all!

Are CGI::Application::Plugin::MessageStack and FormState dead? Will the lack
of maintenance force users to abandon it and use other modules? Will this
force ppl to code the same functionality over and over again - in
contradiction of the idea of a flexible framework?

First of all, this doesn't concern CGI::Application (CA) itself, but two of
the available plugins. CA and most of the modules work just fine and I'm
really glad that it is that way.

Here are the bugged Plugins:
-	CGI::Application::Plugin::FormState
-	CGI::Application::Plugin::MessageStack

These two have the same problem. They produce errors in conjunction with
HTML::Template with the option die_on_bad_params set to 1. They produce this
error, because they try to set a template param that isn't defined in every
template. The solution is to set the template param only for those
templates, where the template param is defined. Using HTML::Template, this
can be found out using the query() method. The problem occurs only if using
HT, so this query() check is done only if the template class is recognized.
Performance issues for any other templating system such as Template or
HTML::Template::Compiled are eliminated this way. There is more: the
approach from the patch can be extended to use some config value of the
plugin to determine if the check should be done or not - as an alternative
to setting die_on_bad_params => 0. Or, the option die_on_bad_params could be
checked for its value. I don't really care. The patch fixes a behavior that
jams users from using HTML::Template with die_on_bad_params => 1 by default.
In my opinion, this is according to the sense of a plugin. If you plug it
in, your application should continue to work and not crash with errors.
Further settings should done by detailed configuration.

I submitted a patch for both of these modules, addressing exactly the issue
mentioned. Nothing has been done so far. This time, no year has passed
between opening a ticket and getting the patch applied. But I would be
really pissed of it would take that long to copy&paste the 15 lines of code
(in case it gets done.), that ensure default compatibility with the default
templating engine of CGI::Application. I really can't imagine someone, who
would not.

It's not that I wouldn't acknowledge the effort other have put into setting
up these modules to cpan. It's not that in my real life, other things would
be more important, too. I have a job and friends and family, too. But there
has to be a way to solve this problem. Someone has to maintain the modules,
or ppl will use other modules that are maintained, or use forks - which
isn't the very spirit of cpan.

Do it yourself isn't an option either. There is two reasons:
a)	I DID it myself. I tracked the errors. I wrote complete test cases -
each one can be used for a tutorial on the module, or an example script. I
thought a lot on solutions and finally created patches and submitted then (1
via rt, 2 via eMail). *All that has to be done is copy&paste*. I even
checked for side effects as backwards-compatibility and stuff (something I
learned from Mark, thx btw).
b)	As mentioned, the original author(s) of the modules put a lot of
work in settings things up. I consider it unfair to take over others work
for maintaining purposes. They did a big part of the work, and they should
get all the credits. I simply would feel bad to take over a module to commit
a patch (e.g. of one line of code). I can't imagine, that this would be the
module's author desired way of how his module is maintained. Correct me if
I'm wrong.
c)	b) is seems to be part of something like the cpan philosophy, cf.
http://www.cpan.org/modules/04pause.html "Taking over"

This sort of bugs me. 
Imagine: Opening the cpan module site, getting a version number of 0.x and a
"last modified min 3 years ago" results in the assumption that CA* is dead.
None seems to maintain it, so probably no one is using it. I really hope I
will be told otherwise. I'm aware of the fact, that version numbers and date
of modifications can be all hollow words. If things are working fine,
numbers don't matter. But they do not work fine and that is the problem.

Regarding the topics recently discussed over the mailing list: why do you
care about a logo for CA, if it's tail (the plugin part) it isn't
maintained? Sounds like window-dressing.

Are CGI::Application::Plugin::MessageStack and FormState dead? Is there
anything we can do about that?

--------------------------------------------------------------------------
To avoid a destructive mail only, here is the constructive part.

a)	FormState
Here is the code, that could be applied as patch for the modules to ensure
compatibility with the default template engine of CA. Replace with
_add_form_state_id_to_tmpl.

# add the storage id to the template params
sub _add_form_state_id_to_tmpl {
my ($self, $ht_params, $tmpl_params, $tmpl_file) = @_;

my $storage_hash = $self->form_state->id;
my $storage_name = $self->form_state->name;

# -- check for HTML::Template and die_on_bad_params => 1
# TODO: omit check if die_on_bad_params => 0.
if( $self->html_tmpl_class() eq 'HTML::Template' ) {

my $t = undef;
# -- copied from CGI::Application::load_tmpl()
if( ref $tmpl_file eq 'SCALAR' ) {
$t = HTML::Template->new( scalarref => $tmpl_file, 
%{$ht_params} );
} elsif ( ref $tmpl_file eq 'GLOB' ) {
$t = HTML::Template->new( filehandle => $tmpl_file, 
%{$ht_params} );
} else {
$t = HTML::Template->new( filename => $tmpl_file, 
%{$ht_params});
}
return unless $t->query(name => $storage_name);
}

$tmpl_params->{$storage_name} = $storage_hash;
}

--------------------------------------------------------------------------
Here is the full diff:
--- new\FormState.pm 
+++ old\FormState.pm 
@@ -41,7 +41,7 @@
 
 =cut
 
-our $VERSION = '0.12-mod';
+our $VERSION = '0.12';
 
 =head1 SYNOPSIS
 
@@ -554,25 +554,7 @@
     my $storage_hash = $self->form_state->id;
     my $storage_name = $self->form_state->name;
 
-    # -- check for HTML::Template and die_on_bad_params => 1
-    # TODO: omit check if die_on_bad_params => 0.
-    if( $self->html_tmpl_class() eq 'HTML::Template' ) {
-    
-        my $t = undef;
-        # -- copied from CGI::Application::load_tmpl()
-        if( ref $tmpl_file eq 'SCALAR' ) {
-            $t = HTML::Template->new( scalarref => $tmpl_file,
%{$ht_params} );
-        } elsif ( ref $tmpl_file eq 'GLOB' ) {
-            $t = HTML::Template->new( filehandle => $tmpl_file,
%{$ht_params} );
-        } else {
-            $t = HTML::Template->new( filename => $tmpl_file,
%{$ht_params});
-        }
-        return unless $t->query(name => $storage_name);
-
-    }
-
     $tmpl_params->{$storage_name} = $storage_hash;
-
 }
 
 =head1 MOTIVATION
@@ -631,4 +613,4 @@
 
 =cut
 
-1;
+1;
\ No newline at end of file



--------------------------------------------------------------------------
b)	MessageStack

Replace _pass_in_messages:

sub _pass_in_messages {
    my ( $self, $ht_params, $tmpl_params, $tmpl_file ) = @_;

	my $loop_name = $config{'-loop_param_name'} || 'CAP_Messages';


    # -- check for HTML::Template and die_on_bad_params => 1
    # TODO: omit check if die_on_bad_params => 0.
    if( $self->html_tmpl_class() eq 'HTML::Template' ) {
    
        my $t = undef;
        # -- copied from CGI::Application::load_tmpl()
        if( ref $tmpl_file eq 'SCALAR' ) {
            $t = HTML::Template->new( scalarref => $tmpl_file, %{$ht_params}
);
        } elsif ( ref $tmpl_file eq 'GLOB' ) {
            $t = HTML::Template->new( filehandle => $tmpl_file,
%{$ht_params} );
        } else {
            $t = HTML::Template->new( filename => $tmpl_file,
%{$ht_params});
        }
        return unless $t->query(name => $loop_name);

    }


    # get the proper messages and update $tmpl_params
    my $session = _check_for_session( $self );
    my $current_runmode = $self->get_current_runmode();
    my $message_stack = $session->param( '__CAP_MessageStack_Stack' );
    my $messages = _filter_messages( $message_stack, { -scope =>
$current_runmode }, 1 );

    $tmpl_params->{ $loop_name } = $messages if scalar( @$messages );
    $self->clear_messages( -scope => $current_runmode ) if (
$config{'-automatic_clearing'} );
}


--------------------------------------------------------------------------
Full diff:
---
C:\Apache\cgi-bin\test\multipage_form\lib\CGI\Application\Plugin\MessageStac
k.pm 
+++ C:\Dokumente und Einstellungen\root\Desktop\MessageStack.pm 
@@ -34,7 +34,7 @@
     goto &Exporter::import;
 }
 
-$VERSION = '0.34-mod';
+$VERSION = '0.34';
 
 =head1 SYNOPSIS
 
@@ -477,34 +477,14 @@
 }
 
 sub _pass_in_messages {
-    my ( $self, $ht_params, $tmpl_params, $tmpl_file ) = @_;
-
-	my $loop_name = $config{'-loop_param_name'} || 'CAP_Messages';
-
-
-    # -- check for HTML::Template and die_on_bad_params => 1
-    # TODO: omit check if die_on_bad_params => 0.
-    if( $self->html_tmpl_class() eq 'HTML::Template' ) {
-    
-        my $t = undef;
-        # -- copied from CGI::Application::load_tmpl()
-        if( ref $tmpl_file eq 'SCALAR' ) {
-            $t = HTML::Template->new( scalarref => $tmpl_file,
%{$ht_params} );
-        } elsif ( ref $tmpl_file eq 'GLOB' ) {
-            $t = HTML::Template->new( filehandle => $tmpl_file,
%{$ht_params} );
-        } else {
-            $t = HTML::Template->new( filename => $tmpl_file,
%{$ht_params});
-        }
-        return unless $t->query(name => $loop_name);
-
-    }
-
+    my ( $self, undef, $tmpl_params, undef ) = @_;
 
     # get the proper messages and update $tmpl_params
     my $session = _check_for_session( $self );
     my $current_runmode = $self->get_current_runmode();
     my $message_stack = $session->param( '__CAP_MessageStack_Stack' );
     my $messages = _filter_messages( $message_stack, { -scope =>
$current_runmode }, 1 );
+    my $loop_name = $config{'-loop_param_name'} || 'CAP_Messages';
 
     $tmpl_params->{ $loop_name } = $messages if scalar( @$messages );
     $self->clear_messages( -scope => $current_runmode ) if (
$config{'-automatic_clearing'} );
@@ -565,4 +545,4 @@
 
 1; # End of CGI::Application::Plugin::MessageStack
 
-__END__
+__END__
\ No newline at end of file


Regards, Alex



More information about the cgiapp mailing list