Re: [cgiapp] CGI::App 3.2 proposed release available

[Cees Hek <suppressed>]

Quoting Mark Stosberg <suppressed>:
> Here's a link to a updated CGI::App distribution that I'm proposing
> become CGI::App 3.2: 

Hi Mark,

thanks for keeping this stuff going.

> http://mark.stosberg.com/perl/CGI-Application-3.2_mls2.tar.gz
> 
> Here's the Changelog for it:
> 
> - header_add() has been added to allow setting extra headers, particularly
> cookies,
>     after header_props has already been called (Cees Hek, Mark Stosberg)

I still think I like having the header_set and header_add functions instead of
just the header_add.  The way you have implemented header_add seems like it
would be too easy to make a mistake in the code (ie remembering to put the [] in
when dealing with cookie headers).  I guess the reason I went with the set and
add style is because of the way mod_perl and apache handle headers, which is in
the Apache::Table class.  They use add and set to modify the headers.  I was
more aiming for the use of header_props to be depricated in later releases,
because I think it is not the best named method in the module, and I believe it
has caused confusion for people starting out with CGI::Application (it did with
me anyway).

However, I can live with what you have provided, because what I really want is a
solution that will solve the underlying problem with the current header_props
implementation.  And I think most of the solutions that have been brought up do
that...

By the way, I do like the documentation entries that you provided.  They are
very clear and concise.

> ######
> There is still one test that is emitting a warning from an eval
> statement. It doesn't seem critical, but maybe someone else could find
> that.  

See the patch below for a fix.

>  From working with the code, I also have a number of other suggestions
> which I think will improve the distribution and make it easier to
> maintain:
> 
> - The two 'warn' statements that happen when the module is being used
>   correctly should only appear when a debugging flag is turned on.

Agreed
 
> - The Changes file should be reversed to follow the convention of having
>   new release information at the top. This makes it easier to find the
>   new information that people care most about.

Agreed
 
> - Jump on the "Module::Build" bandwagon to promote this more pleasant
>   and portable alternative to MakeMaker. For the simple needs of
>   CGI::Application, we could use the 'traditional' compatibility option. 
>   This would mean that users wouldn't even need Module::Build to install
>   it because an old-school Makefile.PL would be auto-generated.

I am not well versed in this area, so I will leave it up to others to discuss
the benefits of this change.

Cheers,

Cees



Here is the patch.  the mailing list doesn't seem to like attachments, so I have
had to inline it.  It is just two very small changes in the new
_header_props_update method.  One just fixes the wording and cleans up test for
the header_type warning and the other fixes the 'uninitialized value' warning
that was being spouted by CGI.pm because it was being provided and undef value.

--- Application.pm.orig 2003-11-24 21:31:17.000000000 -0500
+++ Application.pm      2003-11-24 23:43:57.000000000 -0500
@@ -329,7 +329,7 @@

        # If data is provided, set it!
        if (scalar(@data)) {
-               warn("header_props called while header_type set to 'none',
headers will NOT be sent!") unless $self->header_type ne 'none';
+               warn("header_props or header_add called while header_type set to
'none', headers will NOT be sent!") if $self->header_type eq 'none';
                # Is it a hash, or hash-ref?
                if (ref($data[0]) eq 'HASH') {
                        # Make a copy
@@ -346,6 +346,7 @@
         if ($in{add}) {
             for my $key_set_to_aref (grep { ref $props->{$_} eq 'ARRAY'} keys
%$props) {
                 my $existing_val = $self->{__HEADER_PROPS}->{$key_set_to_aref};
+                next unless defined $existing_val;
                 my @existing_val_array = (ref $existing_val eq 'ARRAY') ?
@$existing_val : ($existing_val);
                 $props->{$key_set_to_aref}  = [ @existing_val_array, @{
$props->{$key_set_to_aref} } ];
             }

---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
              http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed