Hi, I'm the guy that originally wrote this for GSOC, so I figured I'd jump in. I'd be happy to help with getting the PEG module merge-ready.
On Wed, Jan 26, 2011 at 7:40 PM, Noah Lavine <noah.b.lav...@gmail.com> wrote: > Hello again, > > I've attached my coverage results. The html file expects the css file > to be in the same directory. If you look at the html file, you'll see > that almost all of peg.scm is hit by the tests. > > As far as I can tell, the two functions that are not tested are > keyword-flatten (line 512) and peg-string-compile (line 713). I looked > at these, but I don't understand what either of them does well enough > to test them yet. keyword-flatten is described in api-peg.texi. It's basically a special case of context-flatten which collapses S-expressions according to the symbol they start with. From the documentation: @deffn {Scheme Procedure} keyword-flatten terms lst A less general form of @code{context-flatten}. Takes a list of terminal atoms @code{terms} and flattens @var{lst} until all elements are either atoms, or lists which have an atom from @code{terms} as their first element. @lisp (keyword-flatten '(a b) '(c a b (a c) (b c) (c (b a) (c a)))) @result{} (c a b (a c) (b c) c (b a) c a) @end lisp peg-string-compile is a function that will compile PEGs expressed as strings into lambda expressions. It does this by first parsing the string using the PEG-parsing-PEG, then turning the output into an S-expression representation of a PEG, compressing it, and passing the compressed S-expression to peg-sexp-compile. It's used if you e.g. call peg-match with a string instead of an S-expression as the first argument. > > Other than that, some error code is not hit, and the ends of some cond > clauses. These should be tested more, but I need to understand the > code more to know what will test them. There are also a few lines that > I find suspicious, in particular lines 39, 134-136, 146, 157, 165, > 506-508, and 649. (Ludovic - sorry I haven't isolated test cases. I'm > just pointing these out now to show that possibly the test suite tests > more than the coverage makes it appear. In the future I might be able > to isolate the issues.) > > Lines 14-15 look to me like a function that was used for debugging and > now serves no purpose. > > Let me give a few more overall thoughts on peg.scm, after working with > it for a few more days. It looks like good code, but the documentation > isn't great. It took me several read-throughs to figure out what some > of it did, and I'm still not sure about those two functions that don't > have tests. (Although they are a small part of the overall module.) > I'm not sure what this means about its fitness to merge. If you give me some more details on the parts that are weakest I'll try and beef up the documentation. > > Noah > > On Sun, Jan 23, 2011 at 8:29 PM, Noah Lavine <noah.b.lav...@gmail.com> wrote: >> Hello all, >> >>> It should have produced $top_builddir/guile.info, which can be used as >>> input to LCOV to generate an HTML code coverage report >>> (http://ltp.sourceforge.net/coverage/lcov.php). >> >> Oh, that worked. The current tests check 92.6% of the lines in >> peg.scm, and 90.7% of the functions. I looked through lcov's HTML >> guide, and it looks like what the tests miss is almost all >> error-handling code. However, I must say that the HTML output looked a >> bit suspicious - for instance, there were places where the first line >> of a function was marked as hit, but the second line was not. >> >> On another note, I looked at the PEG documentation, and it was quite good. >> >> When I merged the 'mlucy' branch into Guile mainline, the merge went >> almost cleanly - the only issues were a page of links in Guile's >> documentation, which was a two-line issue, and ice-9/psyntax-pp.scm, >> which I fixed by choosing the mainline's version and had no problems. >> >> Given this, what are the issues blocking PEG being merged (if there >> are any)? I'd like to work on them. >> >> Noah >> >