mwolff added inline comments.
INLINE COMMENTS
> foldermodel.cpp:1671
> + return;
> +
> + m_screenMapper = screenMapper;
missing disconnect on the old mapper for the invalidate filter
> foldermodel.cpp:1704
> + setScreen(containment->screen());
> + connect(containment,
> &Plasma::Containment::screenChanged, this, &FolderModel::setScreen);
> + }
when the interface changes, don't you need to disconnect the old containment
here?
> screenmapper.cpp:58
> +
> +void ScreenMapper::addScreen(int screenId)
> +{
conceptually, what is the difference between addScreen and registerScreen? I
can see the code of course, but the names are super similar, and the code as
well. Can/should they share code? Most notably `m_firstScreen` isn't updated in
`addScreen`?
> screenmapper.cpp:90
> +{
> + const auto it = std::min_element(m_availableScreens.constBegin(),
> m_availableScreens.constEnd());
> + return *it;
`return m_firstScreen`, no? faster, and your code would crash if there is no
screen available
> screenmapper.cpp:97
> + if (m_corona != corona) {
> + m_corona = corona;
> + if (m_corona) {
missing disconnect on old m_corona, if valid
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
