TAP::Parser should be *easier* to subclass [PATCH]

Andy Armstrong andy at hexten.net
Sun Jun 8 19:32:49 UTC 2008


Hi Steve,

On 8 Jun 2008, at 20:14, Steve Purkis wrote:
> It ended up being more work than I'd thought (surprise :))...  See  
> below for a summary, look forward to feedback.

I like the look of that. Haven't had a chance to merge the patch yet  
but I shall sometime in the next couple of days. I'll give more  
feedback when / if it arrises :)

> Eric: spot on.
>
> The parser now does this:
> +sub make_source      { shift->source_class->new(@_); }
> +sub make_perl_source { shift->perl_source_class->new(@_); }
> +sub make_grammar     { shift->grammar_class->new(@_); }
> +sub make_iterator    { shift->iterator_factory_class->new(@_); }
> +sub make_result      { shift->result_factory_class->new(@_); }

There'll be a clash with trunk here - I've been doing some work that
probably overlaps. I'm sure it can be merged though.

I'm currently working on TAP::Filter which allows filters that can
modify the TAP stream to be placed in front of the parser. The use case
is that, at work, we want to be able to fault tests that have no
description. With TAP::Filter you can synthesise an additional failed
test for each real test that has no description.

That exposed the following issue: between creating a parser and using it
you might want to replace the source, the iterator or the grammar. To
replace the source you'd want to do something like:

     $parser->_source( Some::Filter->new( $parser->_source ) );

Similarly to replace the grammar you'd want to do

     $parser->_grammar( Some::Filter->new( $parser->_grammar ) );

The problem is that if you create the source /and/ grammar in the
constructor it's too late to replace the source (on which the grammar
depends) after the constructor returns. If you delay creation of the
grammar until the first call to next() you never get the chance to add a
filter into the grammar chain - because the grammar doesn't come into
existence until too late.

My solution was to make $parser->_grammar lazily create a grammar on
demand - so if you do

     my $parser = TAP::Parser->new( ... ) # source created
     my $result = $parser->next; # next calls $self->_grammar which
                                 # causes a grammar to be created

but if you do

     my $parser = TAP::Parser->new( ... ) # source created
     $parser->_grammar( Some::Filter->new( $parser->_grammar ) );
     my $result = $parser->next;

the first call to _grammar creates a grammar before entering next so you
can get the grammar and hook one of your own in front of it.

This works OK too

     my $parser = TAP::Parser->new( ... ) # source created
     $parser->_source( Some::Filter->new( $parser->_source ) );
     my $result = $parser->next;

because it's OK to replace _source before the grammar is created and, in
this case, that doesn't happen until next needs it.

I'd be interested in comments on that approach. It seems to me to do the
right thing - the only caveat is that if you change the grammar before
changing the source

     my $parser = TAP::Parser->new( ... ) # source created
     $parser->_grammar( Some::Filter->new( $parser->_grammar ) );
     $parser->_source( Some::Filter->new( $parser->_source ) );
     my $result = $parser->next;

the change to the source is ineffectual because the grammar has already
been created using the original source. I can probably make it error in
that case.

So as I say, some overlap, but I'm sure I can merge your work with mine.

Thanks Steve :)

-- 
Andy Armstrong, Hexten



More information about the tapx-dev mailing list