[Development] QRegularExpression -- first round of API review
dangelog at gmail.com
Fri Dec 16 03:29:27 CET 2011
2011/12/16 Thiago Macieira <thiago.macieira at intel.com>:
> I did read most of your email :-) Thanks for the effort so far.
Hero :-) Thank you for reading!
> I'd like to start by saying I agree with Ossi: the test/set way of setting
> flags is "un-Qt-ish". I know it exists in a few places, but they are the vast
> minority. I'd prefer a regular pair of getter and setter on the QFlags type.
No problem. Will do.
>> I started looking at it and it seems too cluttered. Specially this early in
>> the process. It's hard to review something that is trying to be everything
>> or maybe it's just exposing too many things.
> João is also right: it seems you're trying to provide all the power of PCRE to
> the user in the first go. It's good that you're exploring everything, so we
> know which way we may go in the future, but I think we can trim down on the
> features at the first iteration of the API.
Ok, see the other message about this.
> The next thing that I find weird is the set of match functions. My first
> reaction was to ask you to call them "indexIn", but since they don't return an
> index but a match object, the name is fine. But still, do we need a match,
> partialMatch and exactMatch?
* match: I think it should stay :-)
* exactMatch: it's convenience, and it's there... because QRegExp
offered it (although there it was handy to perform partial matching).
As I wrote, an user can get the very same result by wrapping a pattern
between \A and \z (and that's more or less how it would be
implemented). It's just a matter of deciding if we want it or not. Up
to the consensus.
* lastMatch (again, being there because QRegExp offered it) this is
tricky to implement correctly and efficiently, and needs the same
discussion about iterating backwards.
* partialMatch: the "hard" version may be removed, for now, in favour
of adding, later, a more general way to perform incremental matching
over non contiguous chunks of data (and iterating, as in /g, over such
matches). The "soft" version might stay there for validators, if you
feel that its behaviour is the right one.
> Also note that the boolean in partialMatch is
> also "un-Qt-ish", so you'll need to replace it with an enum as well. If you
> do, you may as well merge all three functions.
Surely I can merge match, exactMatch and partialMatch in only one
method, with an enum to control the behaviour.
OTOH lastMatch may require a different offset (from the *end* of the
string), so I'm not sure if merging it as well.
More information about the Development