----- Original Message ---- From: Randy W. Sims > > 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"? It is a line or multiple lines. The lexer cannot reliably lex partial lines. > 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. Already changed to an object in the latest version: http://search.cpan.org/dist/TAPx-Parser/ > > 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. Right. > > 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. Hmm, I didn't like the name "results" either. Yeah, I think next() sounds good. Any vetoes? > 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. I do like this suggestion, but it implies that I need to think about how to handle the versioning. By having TAPx::Parser::Grammar contain this information, I could have all of the grammar information encapsulated in one place and my intention was to enventually have something like: TAPx::Parser::Grammar::V1 TAPx::Parser::Grammar::V2 TAPx::Parser::Grammar::V2_1 And so on, and then fetch the appropriate grammar using a passed in version number. Still, I wasn't certain how well this would play with how I had the result objects set up. How would I properly version them? Suggestions welcome. > You would possibly have to include a way of ordering the rules so that > rules are checked in the correct order. Currently in TAP, the order of rule checking is irrelevant as there's no ambiguity in how an individual line is parsed. There *are* efficiency gains to be made (check for tests before bailouts, for example), but that's it. > 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. Agreed. I'm not too happy with them either, but the source of tprove (http://search.cpan.org/src/OVID/TAPx-Parser-0.11/examples/tprove), you can see how those come in handy. The problem is that I really don't like the resulting if/elsif chains since those are so prone to bugs. I've thought about allowing callbacks to be set for each type like this: my %callbacks = ( test => \&_test_callback, comment => \&_comment_callback, ... ); my $parser = TAPx::Parser->new({ stream => $stream, version => $version, callbacks => \%callbacks }); $parser->run; # or for fine-grained control: while ( my $result = $parser->next ) { $result->do_callback; # extra stuff } Thoughts?
Cheers, Ovid
