On Sat, Sep 08, 2001 at 07:21:23PM -0400, Matt Cashner wrote:
> As a first step towards subclassable, well, everything, I've patched
> POE::Preprocessor to allow someone to import the macros and constants
> defined by another module.
> 
> Syntax:
>     use POE::Preprocessor (isa => 'POE::SomeModule');
> 
> you can also override the constants and macros coming from SomeModule
> simply by defining your own.
> 
> This includes a documentation update. a test patch will follow when i
> fully grok the POE test suite :) POE passes all tests with this patch. 
> 
> patch follows sig.

Applied, but I have minor problems with the code.  In fact they're so
minor that I didn't bother tweaking them.  I figured I would tweak
them the next time I worked in POE::Preprocessor.

> ---- PATCH STARTS ----
> --- Preprocessor.pm   2001/07/25 15:28:24     1.23
> +++ Preprocessor.pm   2001/09/08 23:19:14
> @@ -23,10 +23,13 @@
>  #sub DEBUG_INVOKE () { 1 }
>  #sub DEBUG_DEFINE () { 1 }
>  
> +#sub WARN_DEFINE () { 1 }
> +

Nnnnrgh!  Things don't line up!  Man, am I petty or what?

>  BEGIN {
>    defined &DEBUG        or eval 'sub DEBUG        () { 0 }'; # preprocessor
>    defined &DEBUG_INVOKE or eval 'sub DEBUG_INVOKE () { 0 }'; # macro invocs
>    defined &DEBUG_DEFINE or eval 'sub DEBUG_DEFINE () { 0 }'; # macro defines
> +  defined &WARN_DEFINE  or eval 'sub WARN_DEFINE () { 0 }';  # macro/const 
>redefinition warning

Nnnnrgh!  Ditto!

>  };
>  
>  # text_trie_trie is virtually identical to code in Ilya Zakharevich's
> @@ -126,20 +129,53 @@
>  my (%constants, %macros, %const_regexp, %macro);
>  
>  sub import {

[...]

> +
> +    my @isas;
> +        

Maybe a comment before the argument parsing code here, or at least a
"this is where the inheritance is" sort of note.  Still no big deal.

> +    if($args{isa}) {

[...]

> +    }
> +
>      $conditional_stacks{$package_name} = [ ];
>      $excluding_code{$package_name} = 0;
>  
>      my $set_const = sub {
>        my ($name, $value) = @_;
>  

I would put the WARN_DEFINE first here, but I admit I don't have a
concrete reason for it.  Actually I do, but I'm not entirely sure it's
a helpful thing.  I'm assuming that conditional short-circuiting goes
left to right.  Under that assumption, if WARN_DEFINE is false, the
entire block goes away at compile time when it comes first.  Otherwise
the variable condition comes first, which may not be caught at compile
time.

> -      if (exists $constants{$package_name}->{$name}) {
> +      if (exists $constants{$package_name}->{$name} && WARN_DEFINE) {
>          warn "const $name redefined at $file_name line $line_number\n"
>            unless $constants{$package_name}->{$name} eq $value;
>        }
> @@ -336,7 +372,7 @@
>                $macro{$package_name}->[MAC_CODE] =~ s/^\s*//;
>                $macro{$package_name}->[MAC_CODE] =~ s/\s*$//;
>  

Ditto on the WARN_DEFINE order here.

> -              if (exists $macros{$package_name}->{$macro_name}) {
> +              if (exists $macros{$package_name}->{$macro_name} && WARN_DEFINE) {
>                  warn( "macro $macro_name redefined at ",
>                        "$file_name line $line_number\n"
>                      )

[doc patches]

The docs look fine to me.  No, wait, you didn't credit your work in
the copyright section.

-- Rocco Caputo / [EMAIL PROTECTED] / poe.perl.org / poe.sourceforge.net

Reply via email to