D8493: Make Folder View screen aware

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  some more nits, sorry for that ;-)

INLINE COMMENTS

> screenmapper.cpp:69
> +    auto adjustFirstScreen = [this, &screenId](const QString &path) {
> +        int firstScreen = m_firstScreenForPath.value(path, -1);
> +        if (firstScreen == screenId) {

this could now be inlined below since it's only being used in one of the branches

> screenmapper.cpp:84
> +        // needs to be updated.
> +        const auto newFirstScreen = std::min_element(m_availableScreens.constBegin(), m_availableScreens.constEnd());
> +        auto it = m_firstScreenForPath.begin();

this could be moved outside the branch and reused above, once the lambda is inlined

> screenmapper.cpp:85
> +        const auto newFirstScreen = std::min_element(m_availableScreens.constBegin(), m_availableScreens.constEnd());
> +        auto it = m_firstScreenForPath.begin();
> +        while (it != m_firstScreenForPath.end()) {

this should imo be a for loop to make it clear that it is iterating over all items (more ideomatic). Also, couldn't you use range-based for here even?

> screenmapper.cpp:91
> +                // we have now the path for the screen that was removed, so adjust it
> +                pathIt = m_screensPerPath.find(it.key());
> +                if (pathIt != m_screensPerPath.end()) {

this is actually shared with above, so that could be in a lambda

REPOSITORY
  R119 Plasma Desktop

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

To: amantia, #plasma, ervin, mlaurent, dvratil, hein, aacid, davidedmundson, apol, mwolff
Cc: anthonyfieroni, ngraham, 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/20171116/274377be/attachment.html>