D26285: [KuitFormatterPrivate] Start porting QRegExp to QRegularExpression

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  I wonder if others agree that "a ? b() : c()" is bad form for void b() and void c(), compared to an if(), or if it's just me.

INLINE COMMENTS

> kuitmarkup.cpp:1582
> +                // unknown Unicode point, leave it as is
> +                ok ? plain.append(c) : plain.append(match.captured(0));
>              } else if (s->xmlEntities.contains(ent)) { // known entity

I don't really like ternary operators that don't return a value, this is really an if().

(I guess the QChar/QString type difference is the reason why append(ok?c:...) wouldn't work).

> ahmadsamir wrote in kuitmarkup.cpp:1572
> I wouldn't know about C, having never written any code with it.
> 
> > This is in no way more efficient, construction isn't very different from assignment.
> 
> That is the key I was missing (which means I should do some research, before "assuming" stuff).

My statement doesn't apply to all classes obviously, the ctor/dtor could be very expensive and then my statement is incorrect.

But QString is really just a wrapper around a QStringPrivate "d", and both assignment or constructor (from another QString) just do some refcounting and grab the "d" of the other QString, so it's the same.

> ahmadsamir wrote in kuitmarkup.cpp:1597
> I concede that changing var names makes reading the diff harder, and I should, if needed, have done it in a separate patch.
> 
> My, weird, logic is that "original" is the original string that is being modified, v.s. text which is a temporary place to store the parts of original after processing them to prevent the match looping on one singular place in the string. Simply original didn't translate to constant in my head. (And "plain" didn't make sense, it's not plain when it may have "&blah4;" appended to it).
> 
> Anyway, I'd rather change them back than argue. :)

I thought it was the resolved version, without entities (if they were successfully resolved).

REPOSITORY
  R249 KI18n

BRANCH
  l-finalizeVisualText (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26285

To: ahmadsamir, #frameworks, ilic, dfaure, mlaurent, aacid
Cc: kde-frameworks-devel, ltoscano, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200101/b71278c5/attachment-0001.html>