Annoying feature of QRegularExpression :(

*.txt doesn't seem like a good example for wallet search.. any idea how this code is typically used?

In D28880#650963, @dfaure wrote:

Annoying feature of QRegularExpression :(

They're being "technically" correct; I have to say using "*" to match foo/bar.txt is wrong, it should be */* if we're talking about terminal/shell-like globbing.

*.txt doesn't seem like a good example for wallet search.. any idea how this code is typically used?

I used falkon:
https://cgit.kde.org/falkon.git/tree/src/plugins/KDEFrameworksIntegration/kwalletpasswordbackend.cpp#n221

I created a dud account on some forum and added a user name / password, let falkon store/save it; then restarted falkon.

Of course you have to run kwalletd from the kwallet build dir (in my case I just did 'sudo make install'; I don't use kwallet so it didn't matter if I tested).

And what's the value of key? I wonder where/how the use of globbing/regexps comes in...

In D28880#650974, @dfaure wrote:

And what's the value of key? I wonder where/how the use of globbing/regexps comes in...

Backend::readEntryList() docs say it supports wildcards, so "key" could be "foo*" to match:
foo.com/<user name>
foo.org/<user name>
foo-someother.com/<user name>

in falkon's case they use "*" to get all the entries to list them in a "saved logins" kind of config dialog.

In D28880#650985, @dfaure wrote:

This keeps happening so I wonder if we need a QRegularExpression::wildcardToRegularExpression flag...

What do you mean by flag?

A Qt change request for something like QRegularExpression::wildcardToRegularExpression(str, QRegularExpression::AllowSlashes)

In D28880#651001, @dfaure wrote:

A Qt change request for something like QRegularExpression::wildcardToRegularExpression(str, QRegularExpression::AllowSlashes)

Ah, I see; could be useful, yes.

(Personally I think wildcards should stay where they belong, i.e. on the command line. Just use a regex, it's much more predictable/controllable /me hides).

Fair point. For searches made by users, that's arguable, but indeed here if it's a search made by C++ code, then we could very well have a readEntryList(QRegularExpression) overload for that use case [or a method with another name, especially if DBus is involved].

We still need this patch for compatibility, but in KF6 we could clean this up and only have regexp searches (or plain string and regexp, if plain string makes sense).

What do you think?

There are only two places in KDE code where readEntryList() is used, falkon and kwalletmanager; in both cases readEntryList() was used with "*" which means "read all entries", which is logical since in both cases the list is used to fill a "password manager" of some kind.

I am thinking of having 3 new methods (right now there is read{Entry, Map, Password}List, very intuitively replace "read" with "get") that take a third parameter:
enum KWallet::MatchType {KWallet::PlainText, KWallet::Regex}

this way in regex mode, the QString &key parameter is used to construct a QRegularExpression, abstracting the type of the regex class (nitpicking really, since given QRegExp has been around for what 10/15 years? QRegularExpression may live even longer with sturdy PCRE support o/...).

New method names solve some hurdles (e.g. overloading a function that takes only one QString param, and it actually invoked by a dbus call).

In the meantime: https://marc.info/?t=146788789300003&r=1&w=2 , can't say I understood all the issues there but it's definitely a grim read :)

In D28880#651209, @ahmadsamir wrote:

There are only two places in KDE code where readEntryList() is used, falkon and kwalletmanager; in both cases readEntryList() was used with "*" which means "read all entries", which is logical since in both cases the list is used to fill a "password manager" of some kind.

Does this mean that the most useful API would be entryList() which would return all entries.

I am thinking of having 3 new methods (right now there is read{Entry, Map, Password}List, very intuitively replace "read" with "get") that take a third parameter:
enum KWallet::MatchType {KWallet::PlainText, KWallet::Regex}

this way in regex mode, the QString &key parameter is used to construct a QRegularExpression, abstracting the type of the regex class (nitpicking really, since given QRegExp has been around for what 10/15 years? QRegularExpression may live even longer with sturdy PCRE support o/...).

Well actually the regexp syntax changed (a bit) so better be explicit.

New method names solve some hurdles (e.g. overloading a function that takes only one QString param, and it actually invoked by a dbus call).

Yes.

In the meantime: https://marc.info/?t=146788789300003&r=1&w=2 , can't say I understood all the issues there but it's definitely a grim read :)

Yes the idea of using QtKeychain as the application API came up again more recently, see T12219
This would make a lot of sense (abstracting the actual backend).
Still, unless someone makes the bigger effort of "finishing KSecretService" it makes sense to fix the KWallet backend...

In D28880#651415, @dfaure wrote:

In D28880#651209, @ahmadsamir wrote:

There are only two places in KDE code where readEntryList() is used, falkon and kwalletmanager; in both cases readEntryList() was used with "*" which means "read all entries", which is logical since in both cases the list is used to fill a "password manager" of some kind.

Does this mean that the most useful API would be entryList() which would return all entries.

Yes. And then the "matching"/filtering part is done with a QSFPM-like model, which is what the two, and only, current users of readEntryList() do... I was hesitant thinking that there may be many entries and that filtering right here in kwallet might make more sense, but how many passwords will an average user have? 10-20 I would say. This is the same as say firefox (didn't look at the internals there), you get a list of all "saved logins" and filter from a line edit. That option is looking more promising (and less work :D).

I am thinking of having 3 new methods (right now there is read{Entry, Map, Password}List, very intuitively replace "read" with "get") that take a third parameter:
enum KWallet::MatchType {KWallet::PlainText, KWallet::Regex}

this way in regex mode, the QString &key parameter is used to construct a QRegularExpression, abstracting the type of the regex class (nitpicking really, since given QRegExp has been around for what 10/15 years? QRegularExpression may live even longer with sturdy PCRE support o/...).

Well actually the regexp syntax changed (a bit) so better be explicit.

If we go this route, we can be explicit in the docs, but always take a "QString &pattern" param (sort of like KTextEdit find/replace classes, it's always a QString pattern param and a KFind::RegularExpression options flag).

New method names solve some hurdles (e.g. overloading a function that takes only one QString param, and it actually invoked by a dbus call).

Yes.

In the meantime: https://marc.info/?t=146788789300003&r=1&w=2 , can't say I understood all the issues there but it's definitely a grim read :)

Yes the idea of using QtKeychain as the application API came up again more recently, see T12219
This would make a lot of sense (abstracting the actual backend).
Still, unless someone makes the bigger effort of "finishing KSecretService" it makes sense to fix the KWallet backend...

Yeah...

@blaze, (I couldn't find your user name at invent.kde.org); FYI:
https://invent.kde.org/network/falkon/-/merge_requests/7