As was discussed in https://phabricator.kde.org/D28880 with dfaure, it
seems that all users (only two, KWalletManager and Falkon) of the current
read{Entry,Map,Passowrd}list() methods basically use "*" as a wildcard
to get a list of all the "entries" in a folder. Therefore it makes sense
to introduce new methods that do just that, this gets rid of regular
expression usage for matching a certain pattern. This fits with how .e.g.
KWalletManager is using readEntryList(), to get a list of all the entries
to fill a list view with them, then the "matching" is done with a QSFPM-like
class.
Details
- make && ctest
- Ported Falkon locally, and it seems to work
Diff Detail
- Repository
- R311 KWallet
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Todo: port kwalletmanager locally and see if it still works (which isn't saying much as I don't use kwallet that much). I think we can delay this to KF 5.71, to get more testing.
src/api/KWallet/kwallet.cpp | ||
---|---|---|
219 | Urgh. This actually calls for async API instead. But now I'm really confused. HAVE_KSECRETSSERVICE is never set anywhere (except to 0), how does one even compile this code? | |
src/api/KWallet/kwallet.h | ||
382 | Are there documented error codes somewhere? Otherwise a bool would do be more readable, no? But then if we don't have error codes, wouldn't QMap<QString, QByteArray> entriesList() be better? | |
src/runtime/kwalletd/backend/kwalletbackend.cc | ||
558 | rc = map.values(), append() makes little sense if it's empty. | |
src/runtime/kwalletd/backend/kwalletbackend.h | ||
140 | This makes no sense. The list is a value. Does this mean "The caller takes ownership of the entries"? |
Check this commit message https://phabricator.kde.org/R32:f56cdda54b7f88b119f2c7cfb2f534b4fe74478f
Sorry for the delay. We applied a workable hack in https://phabricator.kde.org/D28880 to get rid of the '[^/]' in the string that wildcardToRegularExpression returns, and it seemed to work when I tested falkon.
I guess I'll have migrate this to invent.kde and try addressing the review comments.