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