D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 marked 2 inline comments as done. brute4s99 added a comment. I'll not leave it like this from next time, that's for sure. :p REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: pino,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

dfaure added a comment. Indeed. I assumed it wasn't in your actual commit log (bad idea, for this exact reason). Phabricator often shows outdated descriptions compared to the commit log (unless people use `arc diff --verbatim`) so I stopped complaining about the description Clearly I

D22105: WIP : Fix SFTP Plugin of KIO for Windows

This revision was automatically updated to reflect the committed changes. Closed by commit R320:d6d724b9d4b9: WIP : Fix SFTP Plugin of KIO for Windows (authored by brute4s99). REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=62254=62255 REVISION

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added a comment. should've removed WIP before landing  REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added a comment. yeah I can push! I'm a GSoCer lol REPOSITORY R320 KIO Extras BRANCH arcpatch-D22105 REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks for your persistence :-) Do you have commit access? Otherwise can I have your name and email, for the git author information? REPOSITORY R320 KIO Extras BRANCH

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 marked an inline comment as done. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron, fbampaloukas,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 62254. brute4s99 marked an inline comment as done. brute4s99 added a comment. updated! REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=62237=62254 BRANCH arcpatch-D22105 REVISION DETAIL

D22105: WIP : Fix SFTP Plugin of KIO for Windows

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kio_sftp.cpp:2028 > } > -else if (QT_STAT( QFile::encodeName(sPart), ) == 0) { // > should a very small ".part" be deleted? > +else

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 edited the summary of this revision. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron, fbampaloukas,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 62237. brute4s99 marked 3 inline comments as done. brute4s99 added a comment. updated REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=62187=62237 BRANCH arcpatch-D22105 REVISION DETAIL

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > dfaure wrote in kio_sftp.cpp:2257 > This is marked as Done, but it's not Done. In fact it matches my own > suggestion above, so I agree with Albert ;-) so sorry, it seems I updated a previous version of the diff. REPOSITORY R320 KIO Extras

D22105: WIP : Fix SFTP Plugin of KIO for Windows

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Almost there :-) INLINE COMMENTS > kio_sftp.cpp:406 > +#ifdef Q_OS_WIN > +// TODO Check if this works for other OSes too. > +fileType = QT_STAT_LNK; Yes,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 62187. brute4s99 added a comment. updated REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=62159=62187 BRANCH arcpatch-D22105 REVISION DETAIL https://phabricator.kde.org/D22105 AFFECTED FILES

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 marked 6 inline comments as done. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron, fbampaloukas,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kio_sftp.cpp:266 > > -qCDebug(KIO_SFTP_LOG) << "name=" << name << " instruction=" << > instruction << " prompts=" << n; > +qDebug() <<

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > pino wrote in kio_sftp.cpp:2050-2051 > what is this commented code for? it uses buff.st_atime . Since I'm removing use of buff, I'm not sure how to handle this. For now I've commented out setting the file access time instruction for now.

D22105: WIP : Fix SFTP Plugin of KIO for Windows

pino added inline comments. INLINE COMMENTS > kio_sftp.cpp:2047 > +QString error_msg = receivedFile.errorString(); > +qDebug() << QStringLiteral("Couldn't update modified > time : ") << error_msg; > +} no need for QStringLiteral here,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added a comment. Please revert the changes to qCDebug REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 62159. brute4s99 marked 4 inline comments as done. brute4s99 added a comment. udpated REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=61972=62159 BRANCH arcpatch-D22105 REVISION DETAIL

D22105: WIP : Fix SFTP Plugin of KIO for Windows

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kio_sftp.cpp:1946 > // check if destination is ok ... > QT_STATBUF buff; > +QFileInfo copyFile(sCopyFile); Please remove this variable, since

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > dfaure wrote in kio_sftp.cpp:2037 > Why ReadWrite, if we know it doesn't exist? > > Does setFileTime() even need open() first? I wouldn't have thought so. `setFileTime()` needs the file to be open. Source :

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 61972. brute4s99 marked 7 inline comments as done. brute4s99 added a comment. updated wrt new comments. @dfaure please take another look!  REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=61419=61972

D22105: WIP : Fix SFTP Plugin of KIO for Windows

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kio_sftp.cpp:1939 > QT_STATBUF buff; > const bool bDestExists = (QT_STAT(QFile::encodeName(sCopyFile), ) > != -1); > +QFileInfo

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth accepted this revision. vonreth added a comment. This revision is now accepted and ready to land. looks good REPOSITORY R320 KIO Extras BRANCH arcpatch-D22105 REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc:

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added a comment. I think you need to ```set(CMAKE_CXX_STANDARD 17)``` to make this compile with mingw REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: andriusr, kde-frameworks-devel,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > kio_sftp.cpp:29 > #include > -#include > this builds without `utime.h` on my system (Arch Linux with latest Plasma, Qt and other packages), . Please inform if someone else has issues without this header. REPOSITORY R320 KIO Extras

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 61419. brute4s99 added a comment. Updated with `QT_` pre-procs for S_IFDIR, S_IFLNK and others. REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=61225=61419 BRANCH arcpatch-D22105 REVISION DETAIL

D22105: WIP : Fix SFTP Plugin of KIO for Windows

albertvaka added inline comments. INLINE COMMENTS > brute4s99 wrote in kio_sftp.cpp:2257 > yes! thanks Hannah!  You can keep QT_STAT_LNK on every platform and remove the ifdef. That's the point of the abstraction Qt provides. For consistency, I would also change the other (S_IFDIR, etc.) to

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > vonreth wrote in kio_sftp.cpp:2257 > Does > https://code.woboq.org/qt5/qtbase/mkspecs/common/posix/qplatformdefs.h.html#111 > work? yes! thanks Hannah!  REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To:

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 marked 9 inline comments as done. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, feverfew, meven,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 61225. brute4s99 added a comment. added handling for WIndows. I'll try it on linux and revert on this patch! REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=61035=61225 BRANCH arcpatch-D22105 REVISION

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added inline comments. INLINE COMMENTS > brute4s99 wrote in kio_sftp.cpp:2257 > `S_IFLNK` is not defined on WIndows Does https://code.woboq.org/qt5/qtbase/mkspecs/common/posix/qplatformdefs.h.html#111 work? REPOSITORY R320 KIO Extras REVISION DETAIL

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > albertvaka wrote in kio_sftp.cpp:2257 > I think what they mean is that if you remove the ifdef it should just work. `S_IFLNK` is not defined on WIndows REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To:

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > albertvaka wrote in kio_sftp.cpp:2257 > I think what they mean is that if you remove the ifdef it should just work. ah, gotcha! REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

albertvaka added inline comments. INLINE COMMENTS > andriusr wrote in kio_sftp.cpp:2257 > I'm not sure what is intended to achieve here, the remote file _can_ be a > symlink, and IIRC when we had sftp on dolphin in KDE4 those symlinks were > shown as such. I think what they mean is that if

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added a comment. In D22105#489954 , @vonreth wrote: > Any reason why you skip the symlinks? > I mean displaying thm should be fine, dolphin probably can also follow them. > Creating them on windows is a bit more problematic.

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added a comment. Any reason why you skip the symlinks? I mean displaying thm should be fine, dolphin probably can also follow them. Creating them on windows is a bit more problematic. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To:

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 marked an inline comment as done. brute4s99 added inline comments. INLINE COMMENTS > vonreth wrote in kio_sftp.cpp:402 > What about the QFileInfo? For my use case, I could not find any possibility of symlinks, (traversing Android filesystem) so I left it blank in here for any future

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 added inline comments. INLINE COMMENTS > vonreth wrote in CMakeLists.txt:12 > What requires this bump? ah, I thought QFileDevice needs it. Reverting this. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 updated this revision to Diff 61035. brute4s99 marked 6 inline comments as done. REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22105?vs=60701=61035 BRANCH arcpatch-D22105 REVISION DETAIL https://phabricator.kde.org/D22105 AFFECTED FILES

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added reviewers: sitter, dfaure. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, feverfew, meven,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

andriusr added inline comments. INLINE COMMENTS > kio_sftp.cpp:2257 > +#ifdef Q_OS_WIN > +// TODO ADD CODE FOR HANDLING FILETYPE - SYMLINK IN WIN > +#else I'm not sure what is intended to achieve here, the remote file _can_ be a symlink, and IIRC when we had sftp on dolphin in

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth requested changes to this revision. vonreth added a comment. This revision now requires changes to proceed. We also should add a maintainer for review INLINE COMMENTS > CMakeLists.txt:12 > > -set(QT_MIN_VERSION "5.8.0") > +set(QT_MIN_VERSION "5.10.0") > set(KF5_MIN_VERSION

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 edited the summary of this revision. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman Cc: kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

brute4s99 created this revision. brute4s99 added reviewers: albertvaka, vonreth, sredman. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. brute4s99 requested review of this revision. REVISION SUMMARY CAUTION : Still WIP. The main