Review Request 126027: Port Kickoff to the Kicker backend

David Edmundson david at davidedmundson.co.uk
Wed Nov 11 14:38:14 UTC 2015 

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126027/#review88252
-----------------------------------------------------------


Got halfway...


applets/kicker/plugin/computermodel.cpp (line 41)
<https://git.reviewboard.kde.org/r/126027/#comment60521>

    this is now the default



applets/kicker/plugin/computermodel.cpp (line 83)
<https://git.reviewboard.kde.org/r/126027/#comment60509>

    return QSortFilterProxyModel::lessThan()
    
    I dont't think you can rely on it following the source model's order.
    I had a look at qsortfilterproxy model code; sort() uses stable sort so it implicitly will, but the insert methods don't seem to.
    
    Also without this your explicit setSortRole will do nothing.



applets/kicker/plugin/computermodel.cpp (line 173)
<https://git.reviewboard.kde.org/r/126027/#comment60512>

    this is unused?



applets/kicker/plugin/computermodel.cpp (line 213)
<https://git.reviewboard.kde.org/r/126027/#comment60513>

    url.toString(QUrl::PreferLocalFile)



applets/kicker/plugin/computermodel.cpp (line 264)
<https://git.reviewboard.kde.org/r/126027/#comment60514>

    why do this? trigger is virtual



applets/kicker/plugin/recentusagemodel.cpp (line 60)
<https://git.reviewboard.kde.org/r/126027/#comment60516>

    sourceModel->setParent(this) in the ctor might be better?
    
    It would ensure sourceModel exists during the  QSortFitlerProxy destructor, which I assume is why you're doing this setSourceModel(0)



applets/kicker/plugin/recentusagemodel.cpp (line 112)
<https://git.reviewboard.kde.org/r/126027/#comment60519>

    this method can be just
    
    sourceModel()->data(...)
    
    if sourceModel happens to be a GroupSortProxyModel it will forward the data to the right index itself.



applets/kicker/plugin/recentusagemodel.cpp (line 253)
<https://git.reviewboard.kde.org/r/126027/#comment60520>

    why only for this KRun job, not the others?


- David Edmundson


On Nov. 11, 2015, 12:27 p.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126027/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2015, 12:27 p.m.)
> 
> 
> Review request for Plasma and Ivan Čukić.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> This is a roll-up of the various Kicker backend and ultimately Kickoff changes needed to obsolete Kickoff's old backend in favor of Kicker's. It raises Kickoff to Kicker's feature level (much more fleshed-out context menus with many additional features, pervasive favoriting, etc.) along with addressing numerous Kickoff bugs (e.g. sorting on the Computer page) and removing some cruft.
> 
> Migration from kickoffrc is done using a script and supports both favorites and ISV-customized system apps for the Computer page.
> 
> I suggest using the hein/kickeroff branch for review instead, as things are nicely split out into separate commits there.
> 
> 
> Diffs
> -----
> 
>   applets/kicker/CMakeLists.txt bdb6419 
>   applets/kicker/plugin/abstractentry.h e09b3cf 
>   applets/kicker/plugin/abstractentry.cpp 846a548 
>   applets/kicker/plugin/abstractmodel.h 142de27 
>   applets/kicker/plugin/abstractmodel.cpp 18eed03 
>   applets/kicker/plugin/actionlist.h 4aaa1d5 
>   applets/kicker/plugin/appentry.h 3d0b1bd 
>   applets/kicker/plugin/appentry.cpp 34bab3d 
>   applets/kicker/plugin/appsmodel.h 3cd0577 
>   applets/kicker/plugin/appsmodel.cpp 821ae88 
>   applets/kicker/plugin/computermodel.h PRE-CREATION 
>   applets/kicker/plugin/computermodel.cpp PRE-CREATION 
>   applets/kicker/plugin/favoritesmodel.cpp 4931a16 
>   applets/kicker/plugin/fileentry.h cd4e54a 
>   applets/kicker/plugin/fileentry.cpp 693816d 
>   applets/kicker/plugin/kickerplugin.cpp d31043a 
>   applets/kicker/plugin/recentappsmodel.h 6ee55c1 
>   applets/kicker/plugin/recentappsmodel.cpp 3d167c5 
>   applets/kicker/plugin/recentdocsmodel.h df12d1b 
>   applets/kicker/plugin/recentdocsmodel.cpp ae7306f 
>   applets/kicker/plugin/recentusagemodel.h PRE-CREATION 
>   applets/kicker/plugin/recentusagemodel.cpp PRE-CREATION 
>   applets/kicker/plugin/rootmodel.h 5acdb8e 
>   applets/kicker/plugin/rootmodel.cpp fde16fd 
>   applets/kicker/plugin/runnermatchesmodel.cpp d375fd7 
>   applets/kicker/plugin/runnermodel.h d827e6d 
>   applets/kicker/plugin/runnermodel.cpp 4eb6ccc 
>   applets/kicker/plugin/systementry.h df0712b 
>   applets/kicker/plugin/systementry.cpp ec527da 
>   applets/kicker/plugin/systemmodel.h 1b3eab7 
>   applets/kicker/plugin/systemmodel.cpp ef02892 
>   applets/kickoff/CMakeLists.txt 339316d 
>   applets/kickoff/STATUS-TODO f837793 
>   applets/kickoff/core/applicationmodel.h cc28656 
>   applets/kickoff/core/applicationmodel.cpp a292130 
>   applets/kickoff/core/favoritesmodel.h 7a5f225 
>   applets/kickoff/core/favoritesmodel.cpp be47347 
>   applets/kickoff/core/itemhandlers.h 8f05490 
>   applets/kickoff/core/itemhandlers.cpp d0cd423 
>   applets/kickoff/core/kickoffabstractmodel.h be5ce21 
>   applets/kickoff/core/kickoffabstractmodel.cpp 43afb1b 
>   applets/kickoff/core/kickoffmodel.h dd91f9b 
>   applets/kickoff/core/kickoffmodel.cpp fd5bb62 
>   applets/kickoff/core/kickoffplugin.h de839b3 
>   applets/kickoff/core/kickoffplugin.cpp ef6cbdd 
>   applets/kickoff/core/kickoffproxymodel.h f1af640 
>   applets/kickoff/core/kickoffproxymodel.cpp 8c58d2b 
>   applets/kickoff/core/krunnermodel.h 3916829 
>   applets/kickoff/core/krunnermodel.cpp f8c031b 
>   applets/kickoff/core/leavemodel.h f95a271 
>   applets/kickoff/core/leavemodel.cpp 154eb1c 
>   applets/kickoff/core/models.h 30e80be 
>   applets/kickoff/core/models.cpp a63d363 
>   applets/kickoff/core/org.kde.kickoff.recent.xml d013f62 
>   applets/kickoff/core/org.kde.kickoff.xml b0f5d09 
>   applets/kickoff/core/processrunner.h 5d836eb 
>   applets/kickoff/core/processrunner.cpp 6572386 
>   applets/kickoff/core/qmldir d454d1b 
>   applets/kickoff/core/recentapplications.h b73d526 
>   applets/kickoff/core/recentapplications.cpp 2dd6781 
>   applets/kickoff/core/recentlyusedmodel.h 2530fb4 
>   applets/kickoff/core/recentlyusedmodel.cpp afeed23 
>   applets/kickoff/core/systemmodel.h 5da49ad 
>   applets/kickoff/core/systemmodel.cpp 461a2ed 
>   applets/kickoff/core/urlitemlauncher.h 05c414d 
>   applets/kickoff/core/urlitemlauncher.cpp c350680 
>   applets/kickoff/package/contents/code/tools.js PRE-CREATION 
>   applets/kickoff/package/contents/config/config.qml fe059b8 
>   applets/kickoff/package/contents/config/main.xml 00dfdf9 
>   applets/kickoff/package/contents/ui/ActionMenu.qml PRE-CREATION 
>   applets/kickoff/package/contents/ui/ApplicationsView.qml ad5272a 
>   applets/kickoff/package/contents/ui/BaseView.qml e628614 
>   applets/kickoff/package/contents/ui/Breadcrumb.qml 016cf25 
>   applets/kickoff/package/contents/ui/ComputerView.qml PRE-CREATION 
>   applets/kickoff/package/contents/ui/ConfigGeneral.qml PRE-CREATION 
>   applets/kickoff/package/contents/ui/ContextMenu.qml e8f681e 
>   applets/kickoff/package/contents/ui/FavoritesView.qml f286592 
>   applets/kickoff/package/contents/ui/Footer.qml dcc762f 
>   applets/kickoff/package/contents/ui/FullRepresentation.qml a7559ac 
>   applets/kickoff/package/contents/ui/Kickoff.qml 33b49eb 
>   applets/kickoff/package/contents/ui/KickoffButton.qml ac1e96a 
>   applets/kickoff/package/contents/ui/KickoffItem.qml f7038cc 
>   applets/kickoff/package/contents/ui/KickoffLeaveItem.qml 3b06395 
>   applets/kickoff/package/contents/ui/LeaveView.qml 00e758e 
>   applets/kickoff/package/contents/ui/RecentlyUsedView.qml df25db2 
>   applets/kickoff/package/contents/ui/SearchView.qml f75a7ec 
>   applets/kickoff/package/contents/ui/SystemView.qml e0093f0 
>   applets/kickoff/package/contents/ui/configGeneral.qml d754c34 
>   desktoppackage/contents/updates/obsolete_kickoffrc.js PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126027/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20151111/b04c4e88/attachment-0001.html>

More information about the Plasma-devel mailing list