Show IOSlaves that return local files when in local file mode
Needs ReviewPublic

Authored by ngraham on Dec 10 2019, 5:04 PM.

Details

Reviewers
None
Group Reviewers
Frameworks
Dolphin
Summary

kdialog --get{open,save}filename returns only local paths, but does not show IOSlaves
that return local paths. It should. :)

TODO: show removable disks too, which are always local. Couldn't figure out how to do
that though.

BUG: 415019
FIXED-IN: 19.12.1

Test Plan

Run kdialog --getsavefilename and see the "recently used" IOSlaves in the places panel

Diff Detail

Repository
R229 KDialog
Branch
show-local-ioslaves (branched from release/19.12)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19662
Build 19680: arc lint + arc unit
ngraham requested review of this revision.Dec 10 2019, 5:04 PM
ngraham created this revision.
ngraham updated this revision to Diff 71212.Dec 10 2019, 5:05 PM

Formalize TODO

meven added a subscriber: meven.Dec 10 2019, 5:25 PM
meven added inline comments.
src/kdialog.cpp
865

The trash recentlyused tags and desktop ioslaves can't be save filename' folder since they are not writable.
So I believe this should be undone here.

ngraham added inline comments.Dec 10 2019, 5:27 PM
src/kdialog.cpp
865

kdialog can be used to show open dialogs as well as save dialogs, where writability is not an issue. Also, the default dialog will show these ioslaves and let you try to write into them, so I believe if we want to do that, they should be filtered out there in the KIO places model, rather than here.

Isn't that what KProtocolInfo::protocolClass() == ":local" is for?

fvogt added a subscriber: fvogt.Dec 10 2019, 5:58 PM

IMO this should be done in KIO, so that all users benefit.

Isn't that what KProtocolInfo::protocolClass() == ":local" is for?

Almost, but not quite - it just means that the resource is on the same system and therefore there's no hostname in the URL.
Protocols like man, tar, zip, etc. are all :local as well, but can't be translated to file:///....
I'd hope there is some other way to query for that property.

IMO this should be done in KIO, so that all users benefit.

How would you do it in KIO?

Isn't that what KProtocolInfo::protocolClass() == ":local" is for?

Almost, but not quite - it just means that the resource is on the same system and therefore there's no hostname in the URL.
Protocols like man, tar, zip, etc. are all :local as well, but can't be translated to file:///....
I'd hope there is some other way to query for that property.

Yeah for now I would just folter those out.

fvogt added a comment.Dec 10 2019, 9:20 PM

IMO this should be done in KIO, so that all users benefit.

How would you do it in KIO?

Add code to KFileDialog to allow specific protocols/slaves if the file scheme is supported.

meven added a comment.Jan 14 2020, 1:56 PM

IMO this should be done in KIO, so that all users benefit.

How would you do it in KIO?

Add code to KFileDialog to allow specific protocols/slaves if the file scheme is supported.

I would guess something in the slave interface sort of like KDE-KIO-Protocols in json/.protocol files expect it would be about supported output scheme instead of input/handled.
Something like KDE-KIO-Output-Protocols
Then there would be an equivalent to KProtocolInfoFactory::findProtocol to get those, like KProtocolInfoFactory::findOutputProtocol
Basically ioslaves desktop file recentlyused trash and tags would have "file" set in there. And also recentdocuments timeline for those who use those.

What do you think ?

fvogt added a comment.Jan 14 2020, 2:07 PM

IMO this should be done in KIO, so that all users benefit.

How would you do it in KIO?

Add code to KFileDialog to allow specific protocols/slaves if the file scheme is supported.

I would guess something in the slave interface sort of like KDE-KIO-Protocols in json/.protocol files expect it would be about supported output scheme instead of input/handled.
Something like KDE-KIO-Output-Protocols
Then there would be an equivalent to KProtocolInfoFactory::findProtocol to get those, like KProtocolInfoFactory::findOutputProtocol
Basically ioslaves desktop file recentlyused trash and tags would have "file" set in there.

IMO an overgeneralization: I can't come up with any use for that new key other than file.
There's the mostLocalUrl() method already, so maybe it should just be indicated that mostLocalUrl() returns file://?

And also recentdocuments timeline for those who use those.

AFAIK those can also have non-local URLs inside, so it's not possible to guarantee that only locally-reachable files are shown that way.

What do you think ?

It seems like there's no easy way to fully implement this :-/

Can this please be moved to Gitlab, thanks. Update 415019 when done.