Re^10: Removing AUTOLOAD from CGI.pm (suggestions)

[LanX]

> doesn't show the full delta

True!

I'm not fully comfortable with github yet, but I hope that's the code in question.

Allow me some suggestions:

start up time

foreach my $tag ( _all_html_tags() ) {
    eval "sub $tag {
        return _tag_func(\$tag,\@_);
    }";
...
# again for end and start tags

This loop calls eval about 300 times, you can avoid this with installing this simple closure into your symbol table.

foreach my $tag ( _all_html_tags() ) {
    *$tag = sub { return _tag_func($tag,@_); }
...

As you can see in ... (closure vs eval) tag install time will improve with a factor >5-20 and you get rid of silly escaping.

NB: I've tested on a netbook this should pretty much reflect old HW.

lost features

_all_html_tags() seems to be static, hence you are also deprecating the possibility to import new tags on demand like the <wibble> you mentioned.

deprecation warning for -compile

actually you are not deprecating -compile but making it the default now. So any legacy code using -compile should be fine.

Not sure if you need to fill the logs then.

killing AUTOLOAD

I have the impression your main problem with AUTOLOAD was the maintenance of the ugly code-template to be evaled.

If you switch to installing closures this problem disappears. ³

killing CGI::Pretty

IMHO CGI::Pretty should be able to easily survive now by monkeypatching _tag_func($tag,@_) instead of _make_tag_func() (AFAIS).

Again no eval needed then.

I understand that you are not interested, but maybe others like Your Mother want to contribute.

best way for evaled code templates

I agree that the currently evaled template in CGI::Pretty is a horrible mess because of all the escaped sigils.

If you want/need to continue evaling code you might want to have another look at my sprintf approach in ... (closure vs eval).²

Placeholders are simply marked with %1$s (1 = first arg) in a single quoted string (no escaping for $ or @ sigils needed, only % needs to be duplicated to %% )¹

Hope this helps!

:)

¹) Otherwise regexing the placeholders is very stable with something like

  DB<106> $tmpl = '$#abc#=15' 
 => "\$#abc#=15"

  DB<107> $placeholder{abc}='str'
 => "str"

  DB<108> $tmpl =~ s/#(\w+)#/$placeholder{$1}/g
 => 1

  DB<109> $tmpl
 => "\$str=15"

  DB<110> eval $tmpl
 => 15

  DB<111> $str
 => 15

The need to escape #(\w+)# should be miniscule. (YMMV, but you are free to switch syntax, like {{(\w+)}} :)

²) in short

    eval sprintf <<'__CODE__', $tag;
        sub %1$s {
            "<%1$s>",@_,"</%1$s>";
        }
__CODE__

³) maybe one day you'll decide it's easier to maintain AUTOLOAD for the -any and -autoload edge cases than to keep discussing with angry users? :)

[leej]

Excellent suggestions thanks. I'll have a look at these sometime in the next week or so.