D9325: Use QUrl in the ScreenMapper API

[Milian Wolff]

mwolff added a comment.


  I'd personally prefer to use an API with less conversions if possible. The fact that the test code becomes (marginally) more complex doesn't really count in my opinion.

INLINE COMMENTS

> foldermodel.cpp:291
>  
> -    const auto oldUrl = m_url;
> +    const auto oldUrl = QUrl::fromUserInput(m_url, {}, QUrl::AssumeLocalFile);
>  

couldn't you use `this->resolvedUrl()` here?

> foldermodel.cpp:327
>          m_screenMapper->removeScreen(m_screen, oldUrl);
> -        m_screenMapper->addScreen(m_screen, url);
> +        m_screenMapper->addScreen(m_screen, QUrl::fromUserInput(m_url, {}, QUrl::AssumeLocalFile));
>      }

shouldn't/couldn't this use resolvedUrl?

> foldermodel.cpp:633
>      if (m_usedByContainment) {
> -        m_screenMapper->addScreen(screen, url());
> +        m_screenMapper->addScreen(screen, QUrl::fromUserInput(m_url, {}, QUrl::AssumeLocalFile));
>      }

`resolvedUrl()`?

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

To: amantia, #plasma, mwolff, dakon, broulik
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/20171219/63404900/attachment.html>