D25257: refactor(search): De-couple baloo URL parsing logic from UI

Stefan BrĂ¼ns noreply at phabricator.kde.org
Mon Nov 11 21:54:45 GMT 2019


bruns added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in dolphinquerymodel.cpp:55-57
> Please move the documentation in the header file and add doxygen tags (`/** ... */`).
> 
> I'd also "invert" the definition: is this class says this is a search them, then the UI (e.g.  DolphinFacetsWidget) will be able to handle it.

The intention is to first factor the model out, i.e. no behavior changes.

Afterwards, the model can be changed to e.g. use Baloo::AdvancedQueryParser, which splits out *all* terms with a prefix (modified, type, rating, ...). The Widget can then pick out the ones it handles specifically and keep the remainder verbatim.

> elvisangelaccio wrote in dolphinquerymodel.cpp:60-63
> Why not a simpler `QStringList` ?

`QLatin1String x[] = {...}` is definitely simpler then `QStringList`.

> iasensio wrote in dolphinquerymodel.h:45
> I'm not really sure about this. Wouldn't it be useful to set the terms here and access them from the outside to avoid duplication, for instance, in `DolphinFacetsWidget`itself?

But the DolphinFacetsWidget still has to handle each term prefix individually, it can gain no knowledge by using this.

REPOSITORY
  R318 Dolphin

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

To: iasensio, #dolphin, elvisangelaccio, bruns
Cc: bruns, kfm-devel, pberestov, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20191111/9056ffc6/attachment.htm>


More information about the kfm-devel mailing list