D9290: [filewidgets] Fix create path

2017-12-13 Thread 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 , const QString ) > +static QUrl concatPathsToUrl(const QUrl , const QString ) > { This one

D9290: [filewidgets] Fix create path

2017-12-12 Thread Anthony Fieroni
anthonyfieroni added inline comments. INLINE COMMENTS > pathhelpers_p.h:31 > +{ > +if (!path1.endsWith(QLatin1Char('/'))) { > +return path1 + QLatin1Char('/') + path2; To be absolutely sure we can add pedantic check if (!path1.endsWith('/') && !path2.startsWith('/')) ?

D9290: [filewidgets] Fix create path

2017-12-12 Thread Anthony Fieroni
anthonyfieroni updated this revision to Diff 23843. anthonyfieroni added a comment. Rename function, remove variadic template, it still not needed REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9290?vs=23828=23843 REVISION DETAIL

D9290: [filewidgets] Fix create path

2017-12-12 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Thanks! INLINE COMMENTS > helpers.h:23 > + > +#ifndef KIO_HELPERS_H > +#define KIO_HELPERS_H This file should be called *_p.h to make it clear that it's not installed (and

D9290: [filewidgets] Fix create path

2017-12-12 Thread Anthony Fieroni
anthonyfieroni updated this revision to Diff 23828. anthonyfieroni added a comment. Helper function in all over places. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9290?vs=23768=23828 REVISION DETAIL https://phabricator.kde.org/D9290 AFFECTED FILES

D9290: [filewidgets] Fix create path

2017-12-11 Thread David Faure
dfaure added a comment. Yes, QUrl::addPath would really be convenient. Meanwhile, make an addPath helper that just does if !endsWith('/') append('/') Calling cleanPath sounds much much slower to me... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9290 To:

D9290: [filewidgets] Fix create path

2017-12-11 Thread Anthony Fieroni
anthonyfieroni added a comment. Something like QString makePath(QString path) { return QDir::cleanPath(path); } QString makePath(Qstring path1, QString path2, ...) { return makePath(path1 + '/' + path2, ...); } REPOSITORY R241 KIO REVISION

D9290: [filewidgets] Fix create path

2017-12-11 Thread Anthony Fieroni
anthonyfieroni created this revision. anthonyfieroni added reviewers: Frameworks, dfaure, hein, aacid. Restricted Application added a project: Frameworks. REVISION SUMMARY Basically the problem is .path() + / + .. result in //path/to should we create some generic solution like