D7563: Add privilegeExecution field to file protocol description

feverfew added a comment. In D7563#674682 , @sitter wrote: > This really cannot land right now IMHO. Dolphin can actually deadlock itself because it uses way too much nested event looping and will be entirely unresponsive to mouse inputs when

D29743: sftp: map sftp_open error to kio error

feverfew accepted this revision. This revision is now accepted and ready to land. REPOSITORY R320 KIO Extras BRANCH release/20.04 REVISION DETAIL https://phabricator.kde.org/D29743 To: sitter, feverfew Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov,

D29634: sftp: break large writes into multiple requests

feverfew added a comment. In D29634#670419 , @meven wrote: > In D29634#670377 , @ngraham wrote: > > > Nice work. > > > > In D29634#670159 , @feverfew

D29634: sftp: break large writes into multiple requests

feverfew accepted this revision. feverfew added a comment. I imagine something similar should be done for FileJob::write? REPOSITORY R320 KIO Extras BRANCH release/20.04 REVISION DETAIL https://phabricator.kde.org/D29634 To: sitter, ngraham, meven, feverfew Cc: meven, feverfew,

D29634: sftp: break large writes into multiple requests

feverfew added inline comments. INLINE COMMENTS > meven wrote in kio_sftp.cpp:1831-1832 > Maybe we can deduce the server buffer size based on the `bytesWritten` value > : at init `serv_buffer_size =MAX_XFER_BUF_SIZE; ` and then ` if (length > > bytesWritten) { serv_buffer_size = bytesWritten

D29634: sftp: break large writes into multiple requests

feverfew added a comment. Seems like something similar should also occur in `FileJob::write`? INLINE COMMENTS > kio_sftp.cpp:1831-1832 > +while (offset < buffer.size()) { > +const auto length = qMin(MAX_XFER_BUF_SIZE, > buffer.size()); > +ssize_t

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

feverfew added a comment. In D29385#662435 , @dfaure wrote: > In D29385#662422 , @feverfew wrote: > > > Quick question, how does this affect D23384 ? Previously

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

feverfew added a comment. Quick question, how does this affect D23384 ? Previously KRun used KIO::DesktopExecParser::resultingArguments() which handled the conversion of URLs to local KIOFuse URLs if needed, but now I believe this new API doesn't?

D7563: Add privilegeExecution field to file protocol description

feverfew added a comment. In D7563#653171 , @elvisangelaccio wrote: > In D7563#650117 , @ngraham wrote: > > > [insert I-have-no-idea-what-I'm-doing dog meme here] > > > > When trying to create

D28535: [KIO-MTP] Fix null pointer dereference

This revision was automatically updated to reflect the committed changes. Closed by commit R320:94e7b64325f9: [KIO-MTP] Fix null pointer dereference (authored by feverfew). REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28535?vs=79252=79998 REVISION

D28535: [KIO-MTP] Fix null pointer dereference

feverfew added a comment. In D28535#642298 , @feverfew wrote: > In D28535#642289 , @elvisangelaccio wrote: > > > In D28535#640833 , @fvogt wrote: > >

D28520: Fix lifetime of slot in KIO-MTP

This revision was automatically updated to reflect the committed changes. Closed by commit R320:f4e0183adfd8: Fix lifetime of slot in KIO-MTP (authored by feverfew). REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28520?vs=79196=79609 REVISION DETAIL

D28535: [KIO-MTP] Fix null pointer dereference

feverfew added a comment. In D28535#642289 , @elvisangelaccio wrote: > In D28535#640833 , @fvogt wrote: > > > I assume there is a reason why `MTPDevice::getDevice()` has code for handling this very

D28535: [KIO-MTP] Fix null pointer dereference

feverfew added a comment. Ok so I've updated the code as can be seen. Mucked around with disconnecting/connecting, no issues on my side at least. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D28535 To: feverfew, akrutzler, dfaure, elvisangelaccio Cc:

D28535: [KIO-MTP] Fix null pointer dereference

feverfew updated this revision to Diff 79252. feverfew added a comment. - Don't try to release device in get method REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28535?vs=79198=79252 BRANCH fixNullPtr (branched from master) REVISION DETAIL

D28535: [KIO-MTP] Fix null pointer dereference

feverfew added a comment. In D28535#640703 , @anthonyfieroni wrote: > In D28535#640699 , @feverfew wrote: > > > So to be succinct, the only correct fix here is to change `getDevice()` to `return

D28535: [KIO-MTP] Fix null pointer dereference

feverfew added a comment. If you want to, feel free, I'm a bit tight on time. But I will say this. the whole `getDevice()` function confuses me. I'm not entirely sure why this `if` check is necessary at all. The lifetime of devices and its children (i.e storage) should be managed by the

D28535: [KIO-MTP] Fix null pointer dereference

feverfew added a comment. In D28535#640674 , @fvogt wrote: > What you're suggesting is to change `MTPDevice::getDevice` to return the old device if reopening fails - but reopening without releasing might not work. This seems to be a

D28535: [KIO-MTP] Fix null pointer dereference

feverfew created this revision. feverfew added reviewers: akrutzler, dfaure, elvisangelaccio. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. feverfew requested review of this revision. REVISION SUMMARY A null pointer can be returned from

D28520: Fix lifetime of slot in KIO-MTP

feverfew added a comment. @apol just found another one where this occurs, forgot about it but noticed it earlier as well. This doesn't affect KIOFuse as much though (as we don't listen to the processedSize() signal). I'll wait for another approval, or before 20.04 tagging, whichever

D28520: Fix lifetime of slot in KIO-MTP

feverfew updated this revision to Diff 79196. feverfew added a comment. - Fix another slot lifetime REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28520?vs=79154=79196 BRANCH slotLifetime (branched from master) REVISION DETAIL

D28520: Fix lifetime of slot in KIO-MTP

feverfew created this revision. feverfew added reviewers: akrutzler, dfaure, elvisangelaccio. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. feverfew requested review of this revision. REVISION SUMMARY This slot was incorrectly made to

D28440: Mark KIOFuse mounts as Probably slow

This revision was automatically updated to reflect the committed changes. Closed by commit R241:ec5adbccc23d: Mark KIOFuse mounts as Probably slow (authored by feverfew). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28440?vs=78907=78920 REVISION DETAIL

D28440: Mark KIOFuse mounts as Probably slow

feverfew added reviewers: Frameworks, dfaure, broulik, bruns. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28440 To: feverfew, #frameworks, dfaure, broulik, bruns Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28440: Mark KIOFuse mounts as Probably slow

feverfew created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. feverfew requested review of this revision. REVISION SUMMARY Mark KIOFuse mounts as "Probably Slow". Note that this isn't always true, there are plenty of slaves that simply

D27999: [DesktopExecParser] Open {ssh, telnet, rlogin}:// urls with ktelnetservice

feverfew added a reviewer: ngraham. feverfew accepted this revision. feverfew added a subscriber: ngraham. feverfew added a comment. Quick testing with `fish` protocol doesn't break anything KIOFuse side. Going to page in @ngraham, if it works for him with `smb` it's an all good from the

D27999: [DesktopExecParser] Open {ssh, telnet, rlogin}:// urls with ktelnetservice

feverfew added a comment. don't merge yet, looking at the code I sense something might break here but I need some time to do my own testing first. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27999 To: ahmadsamir, #frameworks, dfaure, sitter, meven, feverfew Cc:

D27999: [DesktopExecParser] Open {ssh, telnet, rlogin}:// urls with ktelnetservice

feverfew added a reviewer: feverfew. feverfew added a comment. I'll review this when I have a chance (sometime this week). REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27999 To: ahmadsamir, #frameworks, dfaure, sitter, meven, feverfew Cc: feverfew,

D27810: Correctly disable UDS_ACCESS being set

feverfew added a comment. In D27810#621427 , @sitter wrote: > I am pretty sure I picked that up somewhere, might be worth asking lxr for possible other places we set ACCESS to -1 grep through kio/kio-extras doesn't reveal this being a

D21795: [KAuth] Add support for action details in Polkit1 backend.

This revision was automatically updated to reflect the committed changes. Closed by commit R283:f53d6a29a049: [KAuth] Add support for action details in Polkit1 backend. (authored by feverfew). REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21795?vs=76739=76740

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew updated this revision to Diff 76739. feverfew added a comment. - Fix & - Use enum class - Use enum instead of string comparison REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21795?vs=76497=76739 BRANCH arcpatch-D21795_2 REVISION DETAIL

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew added a comment. I believe most of your (@sitter) comments (apart from the misaligned `&` probably were caused by me forgetting to rebase), lmk if otherwise. INLINE COMMENTS > sitter wrote in Polkit1Backend.cpp:75 > Is there a reason you use stringy connection syntax instead of

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew updated this revision to Diff 76497. feverfew added a comment. - Merge branch 'master' into arcpatch-D21795_1 - Update version REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21795?vs=76419=76497 BRANCH arcpatch-D21795_1 REVISION DETAIL

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew planned changes to this revision. feverfew added a comment. Ok, I see what's going on here. Earlier I mucked up the diff a bit and had to go back to different diff id and reapply my changes. In the process I forgot to rebase onto master. Once I do that David's copyright will be back

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew updated this revision to Diff 76419. feverfew added a comment. - Fix the diff REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21795?vs=76073=76419 BRANCH arcpatch-D21795_1 REVISION DETAIL https://phabricator.kde.org/D21795 AFFECTED FILES

D27153: port sftp to Result system to force serialization of error/finish condition

feverfew accepted this revision. This revision is now accepted and ready to land. REPOSITORY R320 KIO Extras BRANCH sftp-errors REVISION DETAIL https://phabricator.kde.org/D27153 To: sitter, dfaure, feverfew Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n,

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew added a comment. Ok I've commandeered this on request, as we're so close to getting this done. I've simply addressed sitter's comments here seeming as they're simple enough to do so without understanding the code that well. I haven't actually tested this in any capacity, as again,

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew updated this revision to Diff 76073. feverfew added a comment. - add const & REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21795?vs=76071=76073 BRANCH arcpatch-D21795 REVISION DETAIL https://phabricator.kde.org/D21795 AFFECTED FILES

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew updated this revision to Diff 76071. feverfew marked 4 inline comments as done. feverfew added a comment. - Rebase - Respond to sitter's comments REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21795?vs=63880=76071 BRANCH arcpatch-D21795

D21795: [KAuth] Add support for action details in Polkit1 backend.

feverfew commandeered this revision. feverfew added a reviewer: chinmoyr. REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D21795 To: feverfew, apol, bruns, davidedmundson, #frameworks, dfaure, cfeck, sitter, chinmoyr Cc: elvisangelaccio, bcooksley, ngraham, sitter,

D27153: port sftp to Result system to force serialization of error/finish condition

feverfew requested changes to this revision. feverfew added a comment. This revision now requires changes to proceed. Good stuff, I'll admit I kind of skimmed over the bits that were rewrites `error(...); return;` -> `Result::fail()`, I assume you've mapped correctly there. Only minor

D27291: install smb as both smb:// and cifs://

feverfew added a comment. In D27291#609358 , @dfaure wrote: > feverfew: you're probably looking at master while this is a patch for the 19.12 branch. > See https://phabricator.kde.org/D26358 which happened in master. > > I guess that

D27291: install smb as both smb:// and cifs://

feverfew added a comment. I'm a bit confused here, isn't the protocol file now a JSON file? I'm looking at the repo and it appears to be the case, so I'm not seeing how this diff is seeing this as a rename? REPOSITORY R320 KIO Extras BRANCH bug327295 REVISION DETAIL

D26191: Add support for FileJob->truncate() in smb/sftp slaves

This revision was automatically updated to reflect the committed changes. Closed by commit R320:3bb841890e5a: Add support for FileJob-truncate() in smb/sftp slaves (authored by feverfew). REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=73308=73309

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew updated this revision to Diff 73308. feverfew added a comment. - Merge branch 'master' into arcpatch-D26191 - adhere to protcol -> json switch REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=73306=73308 BRANCH arcpatch-D26191

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew updated this revision to Diff 73306. feverfew added a comment. Cleanup code according to comments REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=72584=73306 BRANCH arcpatch-D26191 REVISION DETAIL

D23384: Adding support for mounting KIOFuse URLs for applications that don't use KIO

This revision was automatically updated to reflect the committed changes. Closed by commit R241:fcf4529f62e1: Adding support for mounting KIOFuse URLs for applications that dont use KIO (authored by feverfew). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew updated this revision to Diff 72584. feverfew added a comment. Fix introduction of BIC method REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=72181=72584 BRANCH arcpatch-D26191 REVISION DETAIL https://phabricator.kde.org/D26191

D26148: Add truncation support to FileJob

This revision was automatically updated to reflect the committed changes. Closed by commit R241:578fbf45297b: Add truncation support to FileJob (authored by feverfew). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26148?vs=72538=72585 REVISION DETAIL

D26148: Add truncation support to FileJob

feverfew added a comment. In D26148#585958 , @dfaure wrote: > In D26148#585944 , @ngraham wrote: > > > Thanks @dfaure! Would you be able to also review D26191 ?

D26148: Add truncation support to FileJob

feverfew updated this revision to Diff 72538. feverfew added a comment. Fix introduction of BIC method REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26148?vs=72180=72538 BRANCH arcpatch-D26148 REVISION DETAIL https://phabricator.kde.org/D26148 AFFECTED

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew updated this revision to Diff 72181. feverfew added a comment. Add fact that protocol supports truncation REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=72155=72181 BRANCH arcpatch-D26191 REVISION DETAIL

D26148: Add truncation support to FileJob

feverfew edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26148 To: feverfew, fvogt, dfaure, sitter Cc: apol, ngraham, sitter, dfaure, kde-frameworks-devel, fvogt, LeGast00n, GB_2, michaelh, bruns

D26148: Add truncation support to FileJob

feverfew updated this revision to Diff 72180. feverfew added a comment. Add feature to determine if protocol supports truncation REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26148?vs=71990=72180 BRANCH arcpatch-D26148 REVISION DETAIL

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew marked 3 inline comments as done. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D26191 To: feverfew, sitter, dfaure, fvogt Cc: kde-frameworks-devel, kfm-devel, ngraham, pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela,

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew updated this revision to Diff 72155. feverfew added a comment. Avoid code duplication REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=72154=72155 BRANCH arcpatch-D26191 REVISION DETAIL https://phabricator.kde.org/D26191 AFFECTED

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew updated this revision to Diff 72154. feverfew added a comment. Avoid free(NULL) REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=72104=72154 BRANCH arcpatch-D26191 REVISION DETAIL https://phabricator.kde.org/D26191 AFFECTED FILES

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew added inline comments. INLINE COMMENTS > fvogt wrote in kio_sftp.cpp:1492 > AFAICT, this has to be done in every case. I wasn't too sure if `attr` was needed by libssh if the call succeeded. Looking at the source code it seems that you are probably right and can be freed in all

D26148: Add truncation support to FileJob

feverfew added a comment. In D26148#582365 , @apol wrote: > It would be interesting to explain why it's important or how it's to be used in the commit message. Done REPOSITORY R241 KIO REVISION DETAIL

D26148: Add truncation support to FileJob

feverfew edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26148 To: feverfew, fvogt, dfaure, sitter Cc: apol, ngraham, sitter, dfaure, kde-frameworks-devel, fvogt, LeGast00n, GB_2, michaelh, bruns

D26148: Add truncation support to FileJob

feverfew edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26148 To: feverfew, fvogt, dfaure, sitter Cc: apol, ngraham, sitter, dfaure, kde-frameworks-devel, fvogt, LeGast00n, GB_2, michaelh, bruns

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew added a dependency: D26148: Add truncation support to FileJob. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D26191 To: feverfew, sitter, dfaure, fvogt Cc: kde-frameworks-devel, kfm-devel, ngraham, pberestov, iasensio, fprice, LeGast00n, MrPepe,

D26148: Add truncation support to FileJob

feverfew added a dependent revision: D26191: Add support for FileJob->truncate() in smb/sftp slaves. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26148 To: feverfew, fvogt, dfaure, sitter Cc: apol, ngraham, sitter, dfaure, kde-frameworks-devel, fvogt, LeGast00n, GB_2,

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew updated this revision to Diff 72104. feverfew added a comment. - Free unneeded struct pointer REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26191?vs=72103=72104 BRANCH master REVISION DETAIL https://phabricator.kde.org/D26191 AFFECTED

D26191: Add support for FileJob->truncate() in smb/sftp slaves

feverfew created this revision. feverfew added reviewers: sitter, dfaure, fvogt. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. feverfew requested review of this revision. REVISION SUMMARY This implements the new method added in D26148

D26148: Add truncation support to FileJob

feverfew updated this revision to Diff 71990. feverfew added a comment. Add unit test for truncation REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26148?vs=71963=71990 BRANCH TruncateSupport REVISION DETAIL https://phabricator.kde.org/D26148 AFFECTED

D26148: Add truncation support to FileJob

feverfew created this revision. feverfew added reviewers: fvogt, dfaure, sitter. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. feverfew requested review of this revision. REVISION SUMMARY This patch adds support for truncation for FileJob. It also

D23384: Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 71666. feverfew added a comment. fix indentation REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=71543=71666 BRANCH arcpatch-D23384_1 REVISION DETAIL https://phabricator.kde.org/D23384 AFFECTED FILES

D23384: Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew retitled this revision from "[WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO" to "Adding support for mounting KIOFuse URLs for applications that don't use KIO". feverfew edited the summary of this revision. feverfew edited the test plan for this

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 71543. feverfew added a comment. - fix code style REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=71423=71543 BRANCH arcpatch-D23384 REVISION DETAIL https://phabricator.kde.org/D23384 AFFECTED FILES

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. In D23384#577174 , @dfaure wrote: > For SMB shares I guess what can happen is that the URL doesn't have a username in it, but it still needs a password? IIRC there was no username in "old" samba (W95

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew marked 9 inline comments as done. feverfew added a comment. The current patch should be tested with latest kio-fuse master, as the blacklisting has been moved there to keep this patch clean. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23384 To: feverfew,

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. On further looking, it seems like git grep doesn't really tell the full picture. It seems like `resultingArguments` is called before `resolveURLs` is, so I've simplified the diff as requested. INLINE COMMENTS > feverfew wrote in desktopexecparser.cpp:331 > From

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 71423. feverfew added a comment. - Merge branch 'master' into arcpatch-D23384 - Address comments - Remove unnecessary mount requests in krun - Only use KIOFuse if password is empty REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added inline comments. INLINE COMMENTS > dfaure wrote in desktopexecparser.cpp:323 > Could we have a kill switch for KIOFuse if it fails to work properly in some > release? > > E.g. some `export KIO_FUSE=0` or `export KIO_FUSE_DISABLE=1`. > Or a KConfig entry (use

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. In D23384#572363 , @dfaure wrote: > Why does KRun duplicate all of the (new) code from DesktopExecParser, when DesktopExecParser is actually a helper class for KRun? > I would expect it to have solved all this

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. Thanks for the reviews everyone! I'll update the diff and respond to the comments no later than Tuesday REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23384 To: feverfew, fvogt, davidedmundson, dfaure, ngraham Cc: alexde, broulik, sitter,

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. Issues with KIOFuse itself should be opened up as an issue, or as a reply to the MR that is being tested; this differential only concerns communication between the KIOFuse mount and KIO itself. In particular, if you could test writing with both master and

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23384 To: feverfew, fvogt, davidedmundson, dfaure, ngraham Cc: broulik, sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, michaelh, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 70866. feverfew added a comment. don't send http(s) to kioexec REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=70830=70866 BRANCH arcpatch-D23384 REVISION DETAIL https://phabricator.kde.org/D23384 AFFECTED

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. So before this patch we had the following behaviour: 1. If app is KIO-enabled, pass URLs unchanged. 2. else for each URL, if app claims to support the protocol pass the original URL, otherwise change the URL to the corresponding KIOExec path. This causes

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 70830. feverfew added a comment. ignore scheme handler REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=70823=70830 BRANCH arcpatch-D23384 REVISION DETAIL https://phabricator.kde.org/D23384 AFFECTED FILES

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. In D23384#571358 , @fvogt wrote: > In D23384#571276 , @ngraham wrote: > > > I'm afraid that even with that change, the issue is still present. I honestly don't think it

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 70823. feverfew added a comment. Convert KIO URLs to KIOFuse URL is app is not KIO-enabled. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=70799=70823 BRANCH arcpatch-D23384 REVISION DETAIL

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 70799. feverfew added a comment. Chek for userInfo instead of just password REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=70773=70799 BRANCH arcpatch-D23384 REVISION DETAIL

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 70773. feverfew added a comment. Check for password instead of authority REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=70661=70773 BRANCH arcpatch-D23384 REVISION DETAIL https://phabricator.kde.org/D23384

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. In D23384#570347 , @ngraham wrote: > In D23384#570119 , @feverfew wrote: > > > In D23384#570016 , @ngraham wrote: > > > > >

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. In D23384#570349 , @ngraham wrote: > In D23384#570125 , @dfaure wrote: > > > @ngraham AFAIK gnome has a trick where a fuse mount is created, its path is passed to the

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. In D23384#570016 , @ngraham wrote: > In D23384#569929 , @feverfew wrote: > > > This isn't reaching KIOFuse at all. I believe this is related to this bug and that you've

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 70661. feverfew added a comment. - Get rid of unnecessary copying of URLs - Switch to interface generated at compile time - Delete unused interfaces - Merge branch 'master' into arcpatch-D23384 - remove kiofuseinterface from build - Align

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew reopened this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23384 To: feverfew, fvogt, davidedmundson, dfaure, ngraham Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, michaelh, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit R241:92c2886c3714: [WIP] Adding support for mounting KIOFuse URLs for applications that dont use… (authored by feverfew).

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added a comment. In D23384#569926 , @ngraham wrote: > In D23384#569645 , @fvogt wrote: > > > **Issue #1:** > > > > That happens because the .desktop file sets

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23384 To: feverfew, fvogt, davidedmundson, dfaure, ngraham Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, michaelh, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added inline comments. INLINE COMMENTS > sitter wrote in krun.cpp:598 > So did the old code though. See old line 584. > > It's iterating by-reference, so you can simply assign to it via > > https://doc.qt.io/qt-5/qurl.html#operator-eq-1 > > to change the content of the QUrl object

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew added inline comments. INLINE COMMENTS > sitter wrote in krun.cpp:583 > Coding style says we use curly braces even for single-line if statements I > think. Yep you're right, will fix in a mo. > sitter wrote in krun.cpp:598 > What I meant is you should literally iterate using > >

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 69788. feverfew added a comment. - Align arguments REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=69774=69788 BRANCH arcpatch-D23384 REVISION DETAIL https://phabricator.kde.org/D23384 AFFECTED FILES

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew marked 8 inline comments as done. feverfew added inline comments. INLINE COMMENTS > sitter wrote in desktopexecparser.cpp:317 > There's nothing wrong with this. But wouldn't using > `QHash, QUrl>` make for easier to read code all in > all since you don't have to mess with pairs? > >

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 69774. feverfew added a comment. - Get rid of unnecessary copying of URLs - Switch to interface generated at compile time - Delete unused interfaces - Merge branch 'master' into arcpatch-D23384 - remove kiofuseinterface from build REPOSITORY

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

feverfew updated this revision to Diff 69769. feverfew added a comment. - Align function arguments - Remove hack - Get rid of iterator REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23384?vs=69201=69769 BRANCH KDEDModule (branched from master) REVISION

  1   2   >