Details
- Reviewers
ngraham mwolff mlaurent dfaure - Group Reviewers
Frameworks Dolphin - Commits
- R241:3e8eb3746b33: Created 'remote' section
From any kde app try to open/save a file.
It should show a new file dialog with network urls under 'remote' section
Diff Detail
- Repository
- R241 KIO
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| src/filewidgets/kfileplacesitem.cpp | ||
|---|---|---|
| 166 | != QLatin1String(":local") | |
lgtm overall. but I wonder about the naming choice. "Shared" is confusing, to me personally at least. Why not call it "Remote" or "Network"? The reasoning is that "shared" for me is only LAN/WLAN shared stuff, while "remote://" allows arbitrary remote links. I use it to connect via (S)FTP to machines on the internet e.g.
I first thought of using "Network" but this is already used as label for "remote://" url. "Remote" can be a option since it is not used yet. But "Shared" came from MacOS images posted in another review, looks like is is used by "Finder".
Well, but if we use Network for remote:// already, then the group should also have this label, no? I don't see an issue with this, really. On the contrary - maybe we could in the future remove the remote:// link and let the category header react to a click, such that it will show you e.g. all remote devices. That would remove the duplication then.
But thinking 5s more about it, I personally would say that using "Remote" for the category would be OK for now.
+1 on Remote.
We really need to remove the duplicated Places test that's the header for the whole widget, though.
| autotests/kfileplacesmodeltest.cpp | ||
|---|---|---|
| 804 | typo: correct | |
| 812 | typo: URL | |
| 819 | typo: URL | |
| src/filewidgets/kfileplacesitem_p.h | ||
| 47–52 | If the actual numbers don't matter (as shown by this commit), why do we even number these explicitly? I would just remove all the =0, =1, =2... values. | |