NOTE: This needs to be backed by a related KIO (from KDE Frameworks) code change.
Details
Details
- Reviewers
asensi gengisdave - Group Reviewers
Krusader - Maniphest Tasks
- T2401: Preferring krarc as a primarily chosen protocol for various archive formats
- Commits
- R167:d1459aebf744: Pick the right (krarc) archive protocol from available protocols
Opening various archive formats always (over many krusader restarts) work through krarc:/ protocol.
Diff Detail
Diff Detail
- Repository
- R167 Krusader
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
I've updated the diff. It needs KF5 code change covered in KF 5.23 version to work. It creates a cache of all archive mimetypes of krarc protocol and then use it to prefer krarc over other archive protocols.
I now also think we don't need to duplicate this protocol preference in Konfigurator (proposed in D1611) as there is not much performance impact in current change and existing users might not even get these defaults anyway. But I'd like hear another opinion on this :-).
Note that requirement of KF 5.23 is not covered in this diff. Is there somebody who knows how to specify a variable in cmake to make this new KF method call conditional? Otherwise I'll try to figure it out myself :-).
Comment Actions
Thanks, Martin! Although using the last KF5 is out of my reach, the changes seem to be good.
I now also think we don't need to duplicate this protocol preference in Konfigurator (proposed in D1611) as
there is not much performance impact in current change and existing users might not even get
these defaults anyway. But I'd like hear another opinion on this :-).
Not duplicating the protocol preference looks like a good idea there :-)
Comment Actions
Thanks for reviews and opinions :-). I did one more diff update accordingly. Changes:
- removed the automatic application/zip handling by krarc from the "defaults"
- added option based on KF 5.23 presence in CMakeLists.txt
Please take a look at the changes as I'm not sure wheter I did everything right. Thanks!
Comment Actions
Thanks, Martin, I've tried your changes in Kubuntu 16.04 and Krusader has been compiled successfully even not having KF 5.23. Several operations with archives (opening archives, deleting files, adding files to archives) have worked correctly, I've tried to rename a file inside an archive and it hasn't work, as it happened before the patch.
Some notes:
- Using Kubuntu 16.04: after having uninstalled Krusader, having deleted the build folder and created it again, I executed cmake [...] and it complained about this:
CMake Warning at /usr/share/ECM/find-modules/FindKF5.cmake:74 (find_package): Could not find a configuration file for package "KF5KIO" that is compatible with requested version "5.23".
The following configuration files were considered but not accepted:
/usr/lib/x86_64-linux-gnu/cmake/KF5KIO/KF5KIOConfig.cmake, version: 5.18.0
Call Stack (most recent call first): CMakeLists.txt:43 (find_package)
- Could NOT find KF5KIO , checked the following files: /usr/lib/x86_64-linux-gnu/cmake/KF5KIO/KF5KIOConfig.cmake (version 5.18.0)
- Could NOT find KF5 (missing: KIO) (Required is at least version "5.23")
Maybe some variables could be renamed, and about other details: someone who is reading this, with more experience with cmake, can help?
- Maybe it would be good if we add the words "at least" at the end of:
- if KF5 version is 5.23
Thanks, Martin!
Comment Actions
Toni, thanks a lot for your review and guidance!
Based on Your input I've done some changes to the diff:
- added QUIET keyword to find_package cmake call to silent the warning
- adjusted the cmake comment to contain "at least" to better explain what the code is trying to do
One more change:
- changed KIO call according KF 5.23 API (turned out that previously I've called *my* proposed method KIO instead of the one written and commited by David Faure)
Of course I'll still be glad if somebody with better cmake knowledge will review the cmake change.
Comment Actions
Thank you for your improvements, Martin!
Using Kubuntu 16.04: after having uninstalled Krusader, having deleted the build folder and created it again, I've tried Martin's changes and Krusader has been compiled successfully (without cmake warnings) even not having KF 5.23. Several operations with archives (opening archives, deleting files, adding files to archives) have worked correctly, I've tried to rename a file inside an archive and it hasn't work, as it happened before the patch.
I agree with what Martin said. Greetings!