This updates the ContentList plugin with a slightly more
declarative syntax. It also makes changes to accomodate using
it in different applications.
Details
Details
Tested with master of Peruse.
Diff Detail
Diff Detail
- Repository
- R157 Peruse
- Branch
- contentlist_generalization
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
Nice work on the new API, very much better than the sort of slap-dash version in the original. Unfortunately, as the inline comments suggest, needs a bit of work ;)
| src/contentlist/BalooContentLister.cpp | ||
|---|---|---|
| 132 | This line crashes for me, when attempting to read the __location property (also, a quick check and it looks to be crashing on the very first result): #0 0x000000000130c310 in ?? ()
#1 0x00007ffff4bdd023 in QObject::property(char const*) const () from /usr/lib64/libQt5Core.so.5
#2 0x00007fffd23d4365 in BalooContentLister::queryResult (this=0xb0e030, query=0x231eec0, file=...) at /home/leinir/projects/comicreader/peruse/src/contentlist/BalooContentLister.cpp:129
#3 0x00007fffd23d5f60 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0, 1>, QtPrivate::List<Baloo::QueryRunnable*, QString const&>, void, void (BalooContentLister::*)(Baloo::QueryRunnable*, QString)>::call (f=
(void (BalooContentLister::*)(BalooContentLister * const, Baloo::QueryRunnable *, QString)) 0x7fffd23d430e <BalooContentLister::queryResult(Baloo::QueryRunnable*, QString)>, o=0xb0e030, arg=0x7fffbc0069c0)
at /usr/include/qt5/QtCore/qobjectdefs_impl.h:136
#4 0x00007fffd23d5dde in QtPrivate::FunctionPointer<void (BalooContentLister::*)(Baloo::QueryRunnable*, QString)>::call<QtPrivate::List<Baloo::QueryRunnable*, QString const&>, void> (f=
(void (BalooContentLister::*)(BalooContentLister * const, Baloo::QueryRunnable *, QString)) 0x7fffd23d430e <BalooContentLister::queryResult(Baloo::QueryRunnable*, QString)>, o=0xb0e030, arg=0x7fffbc0069c0)
at /usr/include/qt5/QtCore/qobjectdefs_impl.h:169
#5 0x00007fffd23d5a4f in QtPrivate::QSlotObject<void (BalooContentLister::*)(Baloo::QueryRunnable*, QString), QtPrivate::List<Baloo::QueryRunnable*, QString const&>, void>::impl (which=1, this_=0x2382370, r=0xb0e030, a=0x7fffbc0069c0,
ret=0x0) at /usr/include/qt5/QtCore/qobjectdefs_impl.h:398
#6 0x00007ffff4bdf7a2 in QObject::event(QEvent*) () from /usr/lib64/libQt5Core.so.5
#7 0x00007ffff5927e6c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/libQt5Widgets.so.5
#8 0x00007ffff592f164 in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/libQt5Widgets.so.5
#9 0x00007ffff4bb0c98 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib64/libQt5Core.so.5
#10 0x00007ffff4bb3675 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib64/libQt5Core.so.5
#11 0x00007ffff4c08ee3 in postEventSourceDispatch(_GSource*, int (*)(void*), void*) () from /usr/lib64/libQt5Core.so.5
#12 0x00007ffff0e80f97 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#13 0x00007ffff0e811d0 in ?? () from /usr/lib64/libglib-2.0.so.0
#14 0x00007ffff0e8125c in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#15 0x00007ffff4c0855f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/libQt5Core.so.5
#16 0x00007fffe9138f61 in ?? () from /usr/lib64/libQt5XcbQpa.so.5
#17 0x00007ffff4baf4aa in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/libQt5Core.so.5
#18 0x00007ffff4bb7fe4 in QCoreApplication::exec() () from /usr/lib64/libQt5Core.so.5
#19 0x0000000000403758 in main (argc=1, argv=0x7fffffffdbb8) at /home/leinir/projects/comicreader/peruse/src/app/main.cpp:146 | |
| src/contentlist/CMakeLists.txt | ||
| 19 | Are you sure? | |
| src/contentlist/ContentList.cpp | ||
| 140–142 | This breaks the consumption end for Peruse, so even if files are found for me (with baloo turned off), it fails to add the files properly, as it doesn't read filePath. It likely is a bug in the original use anyway, but this still breaks it, so it needs changing there as well :) | |
| 215 | This was deliberate, please put back the original logic. Not too keen on returning in the middle of functions unless it's unavoidable within reason. | |
| 248 | surely this would be be if(d->autosearch) { automatically start search } ;) | |
Comment Actions
- Change ContentList::data back to single return point
- Fix FileSystem searcher when locations is not set
- Properly perform mime-type filtering in Baloo searcher
- Use the right role from ContentList for retrieving the file path
- Simplify if statement in ContentList::componentComplete
| src/contentlist/BalooContentLister.cpp | ||
|---|---|---|
| 132 | It seems Baloo passes an invalid runnable. :( Fixed now by using a lambda as discussed on IRC. :) | |
| src/contentlist/CMakeLists.txt | ||
| 19 | Qt5::Gui is needed for QImageReader::supportedFormats(), which is used in ContentQuery.cpp line 151. I could use a hardcoded list for images as well, but I was hoping to have a bit more flexible approach for mapping "query type" to mimetype. I also originally assumed Qt5Qml was already linking to Qt5Gui but that seems to be false. :) | |
| src/contentlist/ContentList.cpp | ||
| 140–142 | Woops, you are right. Fixed the use in BookListModel. Are there any other places where ContentList is used directly? As far as I could tell, everything else uses BookListModel. | |
| 215 | Ah, I generally prefer early return instead. But fair enough, changed it back. | |
| 248 | Hah, good point. I think I originally had some more code following this if, but now it no longer makes sense indeed. :) | |
Comment Actions
Right! It looks like this does the trick! Push away :D
| src/contentlist/BalooContentLister.cpp | ||
|---|---|---|
| 132 | Silly Baloo... Oh well, this is nice and clean anyway, so that works! :) Yay for lamdas ;) | |
| src/contentlist/CMakeLists.txt | ||
| 19 | Right, i guess that's ok, yeah - certainly better than a random bit of hardcoding and such :) | |
| src/contentlist/ContentList.cpp | ||
| 140–142 | Thank you! Nope, i deliberately kept it centralised, so there's only the only place that connects to the ContentList :) | |