Re: [boost] [Review] xpressive

From: Darren Cook (darren_at_[hidden])
Date: 2005-09-15 23:48:12

This is a mini-review, based just on reading the documentation, not on
trying to compile anything. My comments on documentation, with a few
questions are attached below.

Design looks well thought out; library is potentially useful.
Documentation was good. I only read the user documentation, not the
reference docs.
Are you knowledgeable about the problem domain? I've used boost.regex
and spirit, as well as using regexes a lot in scripting languages.

Do you think the library should be accepted as a Boost library? Yes, but
conditional on having benchmarks showing a worthwhile speed improvement
over boost.regex. (Or alternatively over spirit.) Without that there is
no strong reason to have it in Boost along with both Spirit and boost.regex.

Darren

-------------------
URLs are relative to
http://boost-sandbox.sourceforge.net/libs/xpressive/doc/html/xpressive/

* user_s_guide.html
 As I read I assumed "sregex" meant static (compile-time) regex. I then
thought compile() must be very clever and wondered why bother with the
alternative ">>" syntax.
 So I think you need to make it clearer on this page that sregex means
std::string regex, and that compile() is for a run-time regex, and the
">>" syntax is for a compile-time regex.

* creating_a_regex_object.html
 1. Either the meaning of Perl's /s modifier needs to be defined
clearly, or the difference between "_" and "~_n" needs to be shown with
an example (incidentally none of your examples at examples.html match
strings with carriage-returns).

 2. I see I can use icase("Abc") but is there a way to say the whole
regex should be case-insensitive? I.e. the equivalent of:
 "/match something/i"

* grammars_and_nested_matches.html
In the example that starts:
  sregex parentheses;
  parens = '('

should "parens" actually be "parentheses" ?
If not, I don't understand what is happening, and this example needs
more explanation.

 2. In Filtering Nested Results, I wasn't clear what the purpose was. Is
it to show all the name matches before all the id matches? If so,
choosing a less regular example string would help, e.g. with more names
than ids, names following names some of the time, etc.

 3. "See the definition of output_nested_results in the Examples section."
  I think that function should be moved to
grammars_and_nested_matches.html; it seemed out of place in examples.html.

* Other
 1. I'd like to see some fuller examples, that show the I/O part as
well. E.g. a full program that takes a list of email addresses on stdin,
one per line, and spits out a list of the illegal ones.

 2. Benchmarking. I wanted to see the relative speed of compile-time vs.
run-time vs. boost::regex (and ideally vs. PCRE or a scripting language)
on some realistic application.