Migrate some more QRegExps to QRegularExpression
ClosedPublic

Authored by dhaumann on Dec 4 2017, 4:11 PM.

Details

Summary

Please make a careful review. Many of these code
changes are most likely not touched by our unit tests, so
introducing regressions may happen quickly.

Test Plan

make test

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dhaumann created this revision.Dec 4 2017, 4:11 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptDec 4 2017, 4:11 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
dhaumann requested review of this revision.Dec 4 2017, 4:11 PM
mwolff added inline comments.Dec 5 2017, 10:44 AM
src/utils/codecompletionmodelcontrollerinterface.cpp
61–62

both const

src/utils/katebookmarks.cpp
130–131

static?

src/utils/katecommandrangeexpressionparser.cpp
119

future: splitRef

src/utils/katesedcmd.cpp
164

here and below: capturedRef?

src/variableeditor/variablelistview.cpp
51

static?

53

future: splitRef

src/view/kateview.cpp
3804

what was that old code trying to do? did you understand it?

dhaumann marked 5 inline comments as done.Dec 5 2017, 8:49 PM

Update patch comes in a second.

src/utils/katebookmarks.cpp
130–131

I don't think this is critical. Maybe it's even better to not have it static, since otherwise it'll lurk around until application exit.

src/utils/katecommandrangeexpressionparser.cpp
119

Yes, later...

src/utils/katesedcmd.cpp
164

Can do, but some lines later, its converted to a QString anyways, so we don't gain anything :-)

src/view/kateview.cpp
3804

Now that's a good point: I don't completely.
It appends a leading and trailing space, and the searches with \b%1 once, and with %1\b again. But isn't that always true? This is about the highlight selection feature, and I couldn't spot any difference in behavior. Still maybe I missed something...

dhaumann updated this revision to Diff 23538.Dec 5 2017, 8:52 PM
dhaumann marked 3 inline comments as done.
  • Minor updates to the patch.
dhaumann marked 4 inline comments as done.Dec 5 2017, 8:53 PM

Mark some comments as done.

For the QLatin1String on the right side in some contructors: should it not be always QStringLiteral? Not that it will matter a lot.

src/view/kateview.cpp
3804

I think the old code tried to ensure we only add word-boundary if we still can find the string then. e.g. if the string that you highlight contains spaces or stuff like that at the borders, you don't add \b.

I think beside the

I think the old code tried to ensure we only add word-boundary if we still can find the string then. e.g. if the string that you highlight contains spaces or stuff like that at the borders, you don't add \b.

all other comments are just further beautifications.

Could that one thing be fixed and that commited?
In any case the new stuff will be nicer than the old.

Curent state: This patch is only almost good to go in... :-P

src/view/kateview.cpp
3804

This part is still not done. Besides that, this patch should be fine.

Restricted Application edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. · View Herald TranscriptDec 8 2018, 5:24 PM
dhaumann updated this revision to Diff 64248.Aug 21 2019, 7:42 PM
  • Rebase from 2016 to year 2019.

I don't even know whether it still compiles...

dhaumann updated this revision to Diff 64327.Aug 22 2019, 4:54 PM
dhaumann edited the test plan for this revision. (Show Details)
dhaumann removed reviewers: kfunk, mwolff.

Patch should be OK now, please review + approve.

dhaumann marked 4 inline comments as done.Aug 22 2019, 4:55 PM
cullmann accepted this revision.Aug 23 2019, 6:37 AM

Ok ;=)

This revision is now accepted and ready to land.Aug 23 2019, 6:37 AM
This revision was automatically updated to reflect the committed changes.