Ovid wrote:
Hi all,

[...]
A few comments before I start work:

A stream is merely a coderef which returns "chunks" of TAP.  When it returns 
undef, the stream is considered finished.  It will be trivial for me to change this, if 
needed.

What is a "chunk"? Is it a "line"? A partial or multiple lines? Can it contain a partial or more than one complete "expression"?

Still not sure about the coderef. An object could possibly be more flexible, but I can't think of an actual use-case atm, and coderefs are possibly easier on the user.

Second, "Bail out!" doesn't stop the parser.  A careful reading of the TAP docs 
made it clear that this the the TAP generator's responsibility, not the parsers.  I can 
change this behavior, if necessary.

That sounds right. I'm assuming that a stream is one planned test "file" and no more, so that everything in the stream relates to that one plan.

Third, the docs are fairly complete, but not well organized.  Specifically, I 
need to have token methods moved to the main TAPx::Parser page so folks don't 
have to jump from POD to POD to understand how to use this thing.

Fourth, the interface has changed:

    use TAPx::Parser;
my $parser = TAPx::Parser->new( { tap => $string_o_tap } );
    # or
    my $parser = TAPx::Parser->new( { stream => $stream_o_tap } );
while ( my $result = $parser->results ) {
        print $result->as_string;
}

The method name results() doesn't convey any meaning, nor does it make it obvious that it is an iterator. I liked next() better.

The $result object is the token I spoke of earlier.  Each type (qw<comment plan 
test bailout unknown>) is documented as an appropriate subclass of 
TAPx::Parser::Results.

Fifth, I deliberately broke the grammar out into its own class to make it 
easier, in the future, to load different version grammars.  This exposed a 
problem which Schwern pointed out at one point:  I pushed too much work into 
the lexer (Ovid shakes his tiny, tiny fist at Schwern).

I notice that TAPx::Parser::Grammar contains information about each of the Results types. Could it be moved into the Results types? Maybe let the Results types be sort of like plugins that register a rule. When the rule matches you invoke the constructor for the matching Results class and return it.

You would possibly have to include a way of ordering the rules so that rules are checked in the correct order.

Then again, that may be adding more complexity than it removes.

All the is_${type} methods feel wrong (from a purely abstract design-type perspective), but I guess I don't know enough about how the Parser is going to be used and extended to suggest alternatives.

This is still alpha, so feedback appreciated.

Okay!

Randy.

Reply via email to