Support for setting compression method
ClosedPublic

Authored by rthomsen on Sep 28 2016, 11:47 AM.

Details

Summary

Support for setting compression method was implemented. The setting is
available in the Compression collapsible group.

Diff Detail

Repository
R36 Ark
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rthomsen updated this revision to Diff 6971.Sep 28 2016, 11:47 AM
rthomsen retitled this revision from to Support for setting compression method.
rthomsen updated this object.
rthomsen edited the test plan for this revision. (Show Details)
rthomsen added a reviewer: elvisangelaccio.
Restricted Application added a project: Ark. · View Herald TranscriptSep 28 2016, 11:47 AM
Restricted Application added a subscriber: kde-utils-devel. · View Herald Transcript
elvisangelaccio requested changes to this revision.Oct 4 2016, 4:58 PM
elvisangelaccio edited edge metadata.

What about createdialogtest? Does it make sense to check whether the new widget is enabled/disabled?

autotests/kerfuffle/addtoarchivetest.cpp
70

This file has only empty lines as changes.

kerfuffle/cliinterface.cpp
1047–1074

What about making this method virtual? This way each plugin could implement it and provide whatever switch they prefer.

This revision now requires changes to proceed.Oct 4 2016, 4:58 PM

Another thing that I forgot: do we really want to allow the "copy" method (=no compression)? What would be the point?

Another thing that I forgot: do we really want to allow the "copy" method (=no compression)? What would be the point?

The Copy method of cli7z is the same as store method in clizip. It can be used to combine several files into one i.e. like an uncompressed tar archive, so I don't think we should remove it.

rthomsen updated this revision to Diff 7235.Oct 9 2016, 6:14 PM
rthomsen edited edge metadata.
  • Implement Elvis' suggesions
  • Use detected comp.method when adding files to existing archive
rthomsen marked 2 inline comments as done.Oct 9 2016, 6:16 PM

What about createdialogtest? Does it make sense to check whether the new widget is enabled/disabled?

We can test this and also check that the QComboBox contains the correct entries. However, I would rather do this in a separate diff since this one is already rather big.

elvisangelaccio accepted this revision.Oct 10 2016, 4:09 PM
elvisangelaccio edited edge metadata.

Ship it :)

This revision is now accepted and ready to land.Oct 10 2016, 4:09 PM
This revision was automatically updated to reflect the committed changes.