Changeset View
Standalone View
src/api/KWallet/kwallet.cpp
| Show First 20 Lines • Show All 170 Lines • ▼ Show 20 Line(s) | 147 | #if HAVE_KSECRETSSERVICE | |||
|---|---|---|---|---|---|
| 171 | template <typename V> | 171 | template <typename V> | ||
| 172 | int forEachItemThatMatches(const QString &key, V verb) | 172 | int forEachItemThatMatches(const QString &key, V verb) | ||
| 173 | { | 173 | { | ||
| 174 | int rc = -1; | 174 | int rc = -1; | ||
| 175 | KSecretsService::StringStringMap attrs; | 175 | KSecretsService::StringStringMap attrs; | ||
| 176 | attrs[KSS_ATTR_ENTRYFOLDER] = folder; | 176 | attrs[KSS_ATTR_ENTRYFOLDER] = folder; | ||
| 177 | KSecretsService::SearchCollectionItemsJob *searchItemsJob = secretsCollection->searchItems(attrs); | 177 | KSecretsService::SearchCollectionItemsJob *searchItemsJob = secretsCollection->searchItems(attrs); | ||
| 178 | if (searchItemsJob->exec()) { | 178 | if (searchItemsJob->exec()) { | ||
| 179 | QRegExp re(key, Qt::CaseSensitive, QRegExp::Wildcard); | 179 | const QRegularExpression re(QRegularExpression::anchoredPattern( | ||
| 180 | QRegularExpression::wildcardToRegularExpression(key))); | ||||
blaze: anchoredPattern() is excessive here, it creates a horror like that "\\A(?:\\A(?:[^/]*)\\z)\\z"… | |||||
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 upstream (to be honest, I don't know whether this is the intended behaviour or not). ahmadsamir: The pattern has to be anchored if we want to replicate what QRegExp::exactMatch() did, with \A… | |||||
QRegularExpression::wildcardToRegularExpression() already returns an anchored output. What do you mean? blaze: QRegularExpression::wildcardToRegularExpression() already returns an anchored output. What do… | |||||
I didn't get what you meant before. I've just tested and you're right; I didn't know that wildcardToRegularExpression() returned an anchored pattern, the docs didn't say anything about that (if they did, I must have missed it). I'll be filing a diff to fix that (and I'll have review all my other QRegExp port diffs, the same issue will be in more places than just here in kwallet...). Thanks for catching this issue. ahmadsamir: I didn't get what you meant before.
I've just tested and you're right; I didn't know that… | |||||
The output of wildcardToRegularExpression() method is different from what was before and it breaks the app. It has to be replaced with something else. blaze: The output of wildcardToRegularExpression() method is different from what was before and it… | |||||
Any suggestions? QRegularExpression::wildcardToRegularExpression() is what the upstream docs offer:
ahmadsamir: Any suggestions? QRegularExpression::wildcardToRegularExpression() is what the upstream docs… | |||||
I'm pretty sure the file path case is different from what we have here. Also
Doesn't it mean you can't just use a bare raw wildcard? So my suggestion is just to rollback to the previous solution, at least temporarily, until we don't have something better. blaze: > The transformation is targeting file path globbing, which means in particular that path… | |||||
The part about "[?]"; in the QRegularExpression documentation https://doc.qt.io/qt-5/qregularexpression.html#wildcardToRegularExpression "?" is used for wild card matching:
IIUC, the comment about "[?]" means if wildcardToRegularExpression() is used and you want to match a literal "?" character you'll have to use square brackets "[?]", different issue. ahmadsamir: The part about "[?]"; in the QRegularExpression documentation https://doc.qt.io/qt… | |||||
| 180 | const auto list = searchItemsJob->items(); | 181 | const auto list = searchItemsJob->items(); | ||
| 182 | QRegularExpressionMatch match; | ||||
| 181 | for (KSecretsService::SearchCollectionItemsJob::Item item : list) { | 183 | for (KSecretsService::SearchCollectionItemsJob::Item item : list) { | ||
| 182 | KSecretsService::ReadItemPropertyJob *readLabelJob = item->label(); | 184 | KSecretsService::ReadItemPropertyJob *readLabelJob = item->label(); | ||
| 183 | if (readLabelJob->exec()) { | 185 | if (readLabelJob->exec()) { | ||
| 184 | QString label = readLabelJob->propertyValue().toString(); | 186 | QString label = readLabelJob->propertyValue().toString(); | ||
| 185 | if (re.exactMatch(label)) { | 187 | match = re.match(label); | ||
| 188 | if (match.hasMatch()) { | ||||
| 186 | if (verb(this, label, item.data())) { | 189 | if (verb(this, label, item.data())) { | ||
| 187 | rc = 0; // one successful iteration already produced results, so success return | 190 | rc = 0; // one successful iteration already produced results, so success return | ||
| 188 | } | 191 | } | ||
| 189 | } | 192 | } | ||
| 190 | } else { | 193 | } else { | ||
| 191 | qCDebug(KWALLET_API_LOG) << "Cannot execute ReadItemPropertyJob " << readLabelJob->errorString(); | 194 | qCDebug(KWALLET_API_LOG) << "Cannot execute ReadItemPropertyJob " << readLabelJob->errorString(); | ||
| 192 | } | 195 | } | ||
| 193 | } | 196 | } | ||
| ▲ Show 20 Lines • Show All 1479 Lines • Show Last 20 Lines | |||||
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.