D27088: [applets/SystemTray] Implement sorting in the model
Konrad Materka
noreply at phabricator.kde.org
Wed Feb 19 17:34:34 GMT 2020
kmaterka added inline comments.
INLINE COMMENTS
> davidedmundson wrote in sortedsystemtraymodel.cpp:47
> I think calling QSortFilterProxyModel::lessThan(left, right); would do the same
>
> then you don't need compareDisplayAlphabetically
>
> your implementation looks fine though, so do whichever
The question of being implicit or explicit. I like your idea more, will change that.
> davidedmundson wrote in sortedsystemtraymodel.h:35
> We tend not to write virtual at the start now we have the clearer override keyword
Another embarrassing lack of C++ knowledge... Especially this "new" (hehe, C++11) feature :)
> davidedmundson wrote in systemtraymodel.cpp:115
> The applet is still alive after removeApplet is called. "this" is still alive
>
> dataItem is not.
>
> If an applet is added, removed and then (potentially during applet teardown) it changes its title, this would crash.
>
> I don't know if that's a realistic scenario or not, but I would still maybe disconnect all signals from applet -> this on removeApplet?
During lifetime of the SystemTray, dataItems are never removed, they are just marked as not "renderable". Item is still alive, it is needed in configuration page (you can enable applet again in the configuration).
Anyway, it is a good idea to disconnect signals, for the sake of consisteny. In addition, dataItems are deleted eventually which may (?) lead to a crash on shutdown.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D27088
To: kmaterka, #plasma_workspaces, #plasma, davidedmundson, ngraham, broulik
Cc: mart, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200219/e66f8c11/attachment-0001.html>
More information about the Plasma-devel
mailing list