D28880: [KWallet] Port last usage of QRegExp to QRegularExpression

[Ahmad Samir]

ahmadsamir added a comment.


  In D28880#651415 <https://phabricator.kde.org/D28880#651415>, @dfaure wrote:
  
  > In D28880#651209 <https://phabricator.kde.org/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 <https://phabricator.kde.org/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...

REPOSITORY
  R311 KWallet

REVISION DETAIL
  https://phabricator.kde.org/D28880

To: ahmadsamir, #frameworks, dfaure, blaze
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200419/4e4f7fda/attachment-0001.html>