D8493: Make Folder View screen aware

Andras Mantia noreply at phabricator.kde.org
Wed Nov 1 13:36:59 UTC 2017


amantia added inline comments.

INLINE COMMENTS

> mwolff wrote in foldermodel.cpp:1316
> adding some comment here would help people reading the code in the future. I.e. something like "exclude items that are shown on a different screen"
> 
> also, I personally think this code is not pretty at all. When no mapping is found, the first model that encounters the item in this `filterAcceptsRow` will take up the mapping. But... Isn't that too random? Shouldn't the mapping be done elsewhere? Why can there be items encountered here that have no mapping? At the very least, can you add a comment here to explain why "first comes, first served" is the right approach?

Added comments and made it less random (folder views on the first available screen will own the new items). There is a TODO though, as we seem to not react correctly for the case when a folder view is removed from being a desktop containment. Will handle it in a followup diff.

> mwolff wrote in foldermodel.cpp:1691
> shorten this to:
> 
>   if (containment && containment->corona())
>       m_screenMapper->registerCorona(containment->corona());
> 
> ? Or is asking for the corona too expensive?

I don't like to call the same method twice, and in corona() call usually is is an if and qobject_cast, but can be also a recursive call.

> mwolff wrote in screenmapper.cpp:96
> shouldn't you reset m_firstScreen and potentially the other maps, too?

Actually that clear() there was wrong. Available screens are either added via registerScreen or when the screen setup changes, via signals.

> mwolff wrote in screenmapper.cpp:121
> count -> size
> 
> newMap.reserve(size / 2);

count() and size() is the same, no?

REPOSITORY
  R119 Plasma Desktop

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

To: amantia, #plasma, ervin, mlaurent, dvratil, hein, aacid, davidedmundson, apol
Cc: mwolff, davidedmundson, broulik, mart, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171101/dda269ff/attachment.html>


More information about the Plasma-devel mailing list