Add libzip plugin
ClosedPublic

Authored by rthomsen on Mar 13 2017, 6:41 PM.

Details

Summary

The plugin is set as optional but with highest priority for zip archives.

Diff Detail

Repository
R36 Ark
Lint
Lint Skipped
Unit
Unit Tests Skipped
rthomsen created this revision.Mar 13 2017, 6:41 PM
Restricted Application added subscribers: Ark, kde-utils-devel. · View Herald TranscriptMar 13 2017, 6:41 PM
plugins/CMakeLists.txt
14 ↗(On Diff #12444)

Shouldn't we add the plugin only if libzip >= 1.2 has been found?

elvisangelaccio requested changes to this revision.Mar 13 2017, 7:18 PM
elvisangelaccio added inline comments.
plugins/libzipplugin/libzipplugin.cpp
2 ↗(On Diff #12444)

2017

72 ↗(On Diff #12444)

Declare variable as close as possible to where they are needed. In this case this could just be:

zip_t *archive = zip_open(...);
118 ↗(On Diff #12444)

same here.

175 ↗(On Diff #12444)

We are not using the return value of std::bind(), are we? Is there a reason why we need the ugly Callback<void(double)>::func here?

255 ↗(On Diff #12444)

The Entry objects need to be deleted. I think here we should do what libarchive does, i.e. delete them (later) in the destructor.

432 ↗(On Diff #12444)

Remove m_abortOperation. The libzip plugin will run in another thread, we need to use QThread::currentThread()->isInterruptionRequested() instead of this boolean flag. Have a look at the libarchive plugin.

581 ↗(On Diff #12444)

nullptr?

plugins/libzipplugin/libzipplugin.h
2 ↗(On Diff #12444)

2017 :p

60–63 ↗(On Diff #12444)

Either initialize all the member variables here on in the constructor initializer list.

This revision now requires changes to proceed.Mar 13 2017, 7:18 PM

I finally installed libzip 1.2, but I get a bunch of failing tests. @rthomsen what about you?

plugins/libzipplugin/libzipplugin.cpp
175 ↗(On Diff #12444)

Ok ignore this comment. This is not a declaration of a new variable, but an assignment to the global func variable...

I finally installed libzip 1.2, but I get a bunch of failing tests.

Actually only one failure in adddialogtest:

FAIL!  : AddDialogTest::testBasicWidgets(zip) 'collapsibleCompression->isEnabled()' returned FALSE. ()
   Loc: [/home/elvis/dev/kde/src/ark/autotests/kerfuffle/adddialogtest.cpp(124)]
rthomsen updated this revision to Diff 12523.Mar 16 2017, 4:49 PM
rthomsen edited edge metadata.

Address Elvis' suggestions.

rthomsen marked 9 inline comments as done.Mar 16 2017, 4:50 PM
elvisangelaccio accepted this revision.Mar 16 2017, 6:32 PM
elvisangelaccio added inline comments.
plugins/libzipplugin/libzipplugin.cpp
344

nullptr

389

nullptr

419

nullptr

448

nullptr

655

nullptr

705

nullptr

This revision is now accepted and ready to land.Mar 16 2017, 6:32 PM
rthomsen updated this revision to Diff 12534.Mar 16 2017, 6:57 PM

Fix AddDialogTest.

rthomsen updated this revision to Diff 12535.Mar 16 2017, 7:04 PM

More fixes.

rthomsen updated this revision to Diff 12536.Mar 16 2017, 7:06 PM

Previous patch didnt apply.

This revision was automatically updated to reflect the committed changes.