D9325: Use QUrl in the ScreenMapper API

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> foldermodel.cpp:1063
> +    auto mappableUrl = [this, dropTargetFolderUrl](const QUrl &url) -> QUrl {
>          QString mappedUrl = url.toString();
>          if (dropTargetFolderUrl != m_dirModel->dirLister()->url()) {

move down into the first conditional, return original url when nothing changed

> foldermodel.cpp:1068
>              if (mappedUrl.startsWith(local)) {
>                  mappedUrl.replace(0, local.size(), internal);
>              }

return `QUrl::fromUserInput(mappedUrl, {}, QUrl::AssumeLocalFile)`

> foldermodel.cpp:1071
>          }
> -        return mappedUrl;
> +        return QUrl::fromUserInput(mappedUrl, {}, QUrl::AssumeLocalFile);
>      };

return url;

> foldermodel.cpp:1509
>                  // Associated with this folderview if the view is on the first available screen
> -                if (m_screen == m_screenMapper->firstAvailableScreen(url())) {
> -                    m_screenMapper->addMapping(name, m_screen, ScreenMapper::DelayedSignal);
> +                if (m_screen == m_screenMapper->firstAvailableScreen(QUrl::fromUserInput(m_url,
> +                                                                     {}, QUrl::AssumeLocalFile))) {

not your change: why is m_url not an url :-/

also: introduce the helper function you have in the tests here, too - maybe even move it into a static function in the ScreenMapper and then use it everywhere inplace of the QUrl::fromUserInput three-arg function call

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

To: amantia, #plasma, mwolff, dakon, broulik
Cc: 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/20171218/3a9e52bf/attachment.html>