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.
|
