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