[Development] QRegularExpression -- first round of API review
dangelog at gmail.com
Fri Dec 16 01:07:26 CET 2011
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.
More information about the Development