[Development] QRegularExpression -- first round of API review

[Giuseppe D'Angelo]

On 15 December 2011 19:45, Oswald Buddenhagen
<oswald.buddenhagen at nokia.com> wrote:
> On Thu, Dec 15, 2011 at 04:43:49PM +0000, ext Giuseppe D'Angelo wrote:
>>    pos, matchedLength, endPos
>>
> inconsistent naming

Well, pos and matchedLength come straight from QRegExp and I kept
them. But please, any suggestion is welcome! (I'm not even a native
English speaker). (In case it isn't clear: endPos() == pos() +
matchedLength() - 1)

>>     void setPatternOptions(PatternOptions options, bool on = true);
>>
> i don't like that too much, because *un*setting options en masse sucks -
> you need to make a "block" of options to set, and a "block" of options
> to delete.
I copied the setXXX(flags, bool on = true) from
QPainter::setRenderHints, I was quite sure it's also used somewhere
else. So are you suggesting a simple setPatternOptions(options) that
simply sets (as in *assigns*) the argument as the current option set?

> that's half as bad if all options are off by default, but
> then one has to wonder whether a way to unset them actually makes sense
> in the first place.

Yes, all options are off by default. I don't see any particular
problem with turning an option on and then off again. What do you
think about this?

> fwiw, the usual elegant solution is having a value and a mask parameter.
> the mask could have two magic values meaning "un-/set all asserted in
> vlaue" to mean effectively what your bool means. the default argument
> would be the magic value for setting, so the standard syntax would set
> all flags named in the first argument.

I'm not sure I understand this point completely -- isn't that what the
method does? I.e.
  rx.setPatternOptions(CaseInsensitiveOption | DotMatchesEverythingOption)
turns on those two options, and leaves the others unchanged.

Thanks for the feedback.

Cheers,
-- 
Giuseppe D'Angelo