D9290: [filewidgets] Fix create path

[David Faure]

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


  Thanks.

INLINE COMMENTS

> copyjob.cpp:113
>  
> -static QUrl addPathToUrl(const QUrl &url, const QString &relPath)
> +static QUrl concatPathsToUrl(const QUrl &url, const QString &relPath)
>  {

This one was called correctly IMHO (ok, I probably named it) because it adds one path (singular) to a URL :-)

(equivalent naming would be "concatUrlAndPath" but that's not great).

May I suggest reverting to the original name addPathToUrl?

> anthonyfieroni wrote in pathhelpers_p.h:31
> To be absolutely sure we can add pedantic check
> 
>   if (!path1.endsWith('/') && !path2.startsWith('/'))
> 
> ?

It seems to me that path2 cannot start with '/', by contract. It's supposed to be relative.
So I'd say we could even be foolish^H^H^H^Hbrave and make that

  Q_ASSERT(!path2.startsWith('/'));

REPOSITORY
  R241 KIO

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

To: anthonyfieroni, #frameworks, dfaure, hein, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171213/24135ab5/attachment.html>