Port QRegExp to QRegularExpression
ClosedPublic

Authored by ahmadsamir on Dec 19 2019, 9:45 AM.

Details

Summary

Require Qt 5.12 for QRegularExpression::achoredPattern()

Test Plan

The code builds and unit tests pass.

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahmadsamir created this revision.Dec 19 2019, 9:45 AM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptDec 19 2019, 9:45 AM
ahmadsamir requested review of this revision.Dec 19 2019, 9:45 AM
ahmadsamir planned changes to this revision.Dec 19 2019, 4:57 PM

I messed up exactMatch stuff, will have to rethink it.

ahmadsamir updated this revision to Diff 71869.Dec 20 2019, 8:19 AM
ahmadsamir edited the summary of this revision. (Show Details)

Use QRegularExpression::anchoredPattern() to port QRegExp::exactMatch()

mlaurent requested changes to this revision.Dec 20 2019, 12:44 PM
mlaurent added a subscriber: mlaurent.
mlaurent added inline comments.
CMakeLists.txt
3 ↗(On Diff #71869)

We can't change qt version as it's a kf5 frameworks

This revision now requires changes to proceed.Dec 20 2019, 12:44 PM
ahmadsamir removed a subscriber: mlaurent.

Revert min required Qt version

Add TODO for when we can depend on Qt 5.12

ahmadsamir updated this revision to Diff 71943.Dec 21 2019, 1:16 PM

Frameworks now require Qt 5.12

meven added a comment.Dec 21 2019, 1:31 PM

Frameworks now require Qt 5.12

Indeed since 65da5bf151fa934bbdcac82543e3cd9f5cc4ab43 in baloo

meven accepted this revision.Dec 21 2019, 1:33 PM

Seems fine to me

bruns added inline comments.Dec 21 2019, 2:38 PM
src/kioslaves/timeline/timelinetools.cpp
119

Pleae avoid assigning in an if (...) expression, just move it to the previous line.

ahmadsamir updated this revision to Diff 71954.Dec 21 2019, 2:58 PM

Don't assign inside if condition

ahmadsamir marked an inline comment as done.Dec 21 2019, 2:59 PM

I guess I was confused because I saw KItemModels has the min. Qt version 5.12.0

ahmadsamir updated this revision to Diff 71992.Dec 22 2019, 2:23 PM

Link to KF6 task

dfaure requested changes to this revision.Jan 5 2020, 10:24 AM
dfaure added inline comments.
src/kioslaves/timeline/timelinetools.cpp
126

By moving this outside the else if above, we no longer go to the "else" branch on line 129 if the matching fails.

@bruns I think this is actually a good use case for assigning inside an if () expression, as we already did in a number of other port-to-QRegularExpression commits.

The alternative is helper functions or lambdas.

This revision now requires changes to proceed.Jan 5 2020, 10:24 AM
ahmadsamir updated this revision to Diff 73367.EditedJan 13 2020, 6:50 AM
  • Rebase
  • Move second match inside if condition, otherwise the control flow never passes through the next else if branch
dfaure accepted this revision.Jan 13 2020, 8:30 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 13 2020, 8:53 AM
This revision was automatically updated to reflect the committed changes.