D26205: KWallet: Port QRegExp to QRegularExpression

blaze added a comment. I was debugging the code locally and here's what I found: simple wildcard like `*` is being replaced with cool stuff like `\\A(?:[^/]*)\\z`. As you can see, this pattern excludes forward slashes, which may be a good thing for a file path, but it is exactly the source

D26205: KWallet: Port QRegExp to QRegularExpression

blaze added a comment. In D26205#595428 , @ahmadsamir wrote: > I still fail to see what's broken, I tested with the system kwallet (5.65 on tumbleweed) and I don't see any difference between it and a build from a git checkout; i.e. falkon

D26205: KWallet: Port QRegExp to QRegularExpression

ahmadsamir added a comment. In D26205#595111 , @blaze wrote: > > otherwise, we could just revert this patch > > The rest of the code is OK. The part that works funny is just the wildcard method, and since it's relatively new, I hope there

D26205: KWallet: Port QRegExp to QRegularExpression

ahmadsamir added a comment. INLINE COMMENTS > blaze wrote in kwallet.cpp:180 > > The transformation is targeting file path globbing, which means in > > particular that path separators receive special treatment. > > I'm pretty sure the file path case is different from what we have here. > >

D26205: KWallet: Port QRegExp to QRegularExpression

aacid added a comment. In D26205#595080 , @ahmadsamir wrote: > In D26205#595034 , @blaze wrote: > > >

D26205: KWallet: Port QRegExp to QRegularExpression

blaze added a comment. > otherwise, we could just revert this patch The rest of the code is OK. The part that works funny is just the wildcard method, and since it's relatively new, I hope there could be a good workaround until the situation stabilizes REPOSITORY R311 KWallet

D26205: KWallet: Port QRegExp to QRegularExpression

ahmadsamir added a comment. In D26205#595034 , @blaze wrote: > https://github.com/KDE/falkon/blob/master/src/plugins/KDEFrameworksIntegration/kwalletpasswordbackend.cpp#L187 > > QMap entries; > if (m_wallet->readEntryList("*", entries)

D26205: KWallet: Port QRegExp to QRegularExpression

blaze added a comment. https://github.com/KDE/falkon/blob/master/src/plugins/KDEFrameworksIntegration/kwalletpasswordbackend.cpp#L187 QMap entries; if (m_wallet->readEntryList("*", entries) != 0) { qWarning() << "KWalletPasswordBackend::initialize Cannot read entries!";

D26205: KWallet: Port QRegExp to QRegularExpression

blaze added a comment. In D26205#594867 , @ahmadsamir wrote: > A test case of what is broken would be appreciated, to try and fix/debug the issue. INLINE COMMENTS > ahmadsamir wrote in kwallet.cpp:180 > Any suggestions?

D26205: KWallet: Port QRegExp to QRegularExpression

ahmadsamir added inline comments. INLINE COMMENTS > blaze wrote in kwallet.cpp:179 > QRegularExpression::wildcardToRegularExpression() already returns an anchored > output. What do you mean? I didn't get what you meant before. I've just tested and you're right; I didn't know that

D26205: KWallet: Port QRegExp to QRegularExpression

blaze added inline comments. INLINE COMMENTS > ahmadsamir wrote in kwallet.cpp:179 > The pattern has to be anchored if we want to replicate what > QRegExp::exactMatch() did, with \A and \z or ^ and $. > > If you have a test case where it anchoredPattern() anchors twice, file a bug > report

D26205: KWallet: Port QRegExp to QRegularExpression

ahmadsamir added a comment. A test case of what is broken would be appreciated, to try and fix/debug the issue. REPOSITORY R311 KWallet REVISION DETAIL https://phabricator.kde.org/D26205 To: ahmadsamir, #frameworks, aacid, apol Cc: blaze, kde-frameworks-devel, LeGast00n, GB_2,

D26205: KWallet: Port QRegExp to QRegularExpression

ahmadsamir added inline comments. INLINE COMMENTS > blaze wrote in kwallet.cpp:179 > anchoredPattern() is excessive here, it creates a horror like that > "\\A(?:\\A(?:[^/]*)\\z)\\z" with the stuff being anchored twice. > It has to be removed with no doubt at all. The pattern has to be anchored

D26205: KWallet: Port QRegExp to QRegularExpression

blaze added a comment. This changeset breaks the app, making it impossible to get the list of entries from outside. Tested against Falkon browser with KDE integration plugin. See my inline comments here INLINE COMMENTS > kwallet.cpp:179 > if (searchItemsJob->exec()) { > -

D26205: KWallet: Port QRegExp to QRegularExpression

This revision was automatically updated to reflect the committed changes. Closed by commit R311:bb4eed3bfda0: KWallet: Port QRegExp to QRegularExpression (authored by ahmadsamir). REPOSITORY R311 KWallet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26205?vs=72132=72146 REVISION

D26205: KWallet: Port QRegExp to QRegularExpression

apol accepted this revision. This revision is now accepted and ready to land. REPOSITORY R311 KWallet BRANCH l-qregularexpression (branched from master) REVISION DETAIL https://phabricator.kde.org/D26205 To: ahmadsamir, #frameworks, aacid, apol Cc: kde-frameworks-devel, LeGast00n, GB_2,

D26205: KWallet: Port QRegExp to QRegularExpression

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, aacid. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY Port QRegExp::exactMatch() with QRegularExpression::anchoredPattern() Port QRegExp::Wildcard with