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