D9325: Use QUrl in the ScreenMapper API

[Milian Wolff]

mwolff accepted this revision.
mwolff added a comment.
This revision is now accepted and ready to land.


  two questions, otherwise lgtm

INLINE COMMENTS

> foldermodel.cpp:291
>  
> -    const auto oldUrl = m_url;
> +    const auto oldUrl = this->resolvedUrl();
>  

could you call this at the top? then the `this->` call shouldn't be required, I think.

> foldermodel.cpp:327
>          m_screenMapper->removeScreen(m_screen, oldUrl);
> -        m_screenMapper->addScreen(m_screen, url);
> +        m_screenMapper->addScreen(m_screen, this->resolvedUrl());
>      }

is this different from the local `resolvedUrl` variable? If so, why? confusing ;-) Add a comment then

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

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