Detect compression method
ClosedPublic
Actions

Authored by rthomsen on Oct 8 2016, 5:27 PM.

Details

Reviewers

Commits

R36:50a9f590bd2d: Detect compression method

Summary

A new property was added to Archive to store detected compression method(s). Currently, it's only displayed in PropertiesDialog, but the plan is to also use it to select a sane default compression method when adding files to an existing archive. Code was added to all plugins for detecting the compression method(s) when opening an archive and a signal added to ReadOnlyArchiveInterface to set the property.

Many archive types support multiple compression methods in same archive, so the property is a QStringList.

Test Plan

Open various archive types and check that the correct compression method is shown in PropertiesDialog.

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 7219.Oct 8 2016, 5:27 PM

rthomsen retitled this revision from to Detect compression method.

rthomsen updated this object.

rthomsen edited the test plan for this revision. (Show Details)

rthomsen added a reviewer: elvisangelaccio.

rthomsen set the repository for this revision to R36 Ark.

rthomsen added a project: Ark.

Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptOct 8 2016, 5:27 PM

elvisangelaccio requested changes to this revision.Oct 9 2016, 9:17 AM

elvisangelaccio edited edge metadata.

elvisangelaccio added inline comments.

kerfuffle/archive_kerfuffle.h
225	Missing reference in the argument
kerfuffle/archiveinterface.h
170	Can we call this `compressionMethodFound`? Same with the slots, e.g. onCompressionMethodFound
plugins/cli7zplugin/cliplugin.cpp
270–274	This check is duplicated in two plugins. Can we move it in the Archive's slot that sets the new property?
plugins/clirarplugin/cliplugin.cpp
220	This creates a temporary `std::initializer` list that is then converted to QStringList. A more efficient way is using `QStringList {QStringLiteral("foo")}`
222	same as above.
plugins/cliunarchiverplugin/cliplugin.cpp
245	`QStringList {QStringLiteral("foo")}`
247	`QStringList {QStringLiteral("foo")}`
plugins/libarchive/libarchiveplugin.cpp
63	`QStringList {compMethod}`

This revision now requires changes to proceed.Oct 9 2016, 9:17 AM

Implement Elvis' suggestions.

rthomsen marked 7 inline comments as done.Oct 9 2016, 4:00 PM

rthomsen marked an inline comment as done.

elvisangelaccio accepted this revision.Oct 9 2016, 4:16 PM

elvisangelaccio edited edge metadata.

elvisangelaccio added inline comments.

plugins/libarchive/libarchiveplugin.cpp
542 ↗	(On Diff #7229)	These `else` are not really necessary since they come after a return, but i guess this is a subjective style preference. Up to you :p

This revision is now accepted and ready to land.Oct 9 2016, 4:16 PM

Closed by commit R36:50a9f590bd2d: Detect compression method (authored by rthomsen). · Explain WhyOct 9 2016, 4:39 PM

This revision was automatically updated to reflect the committed changes.

rthomsen mentioned this in R36:50a9f590bd2d: Detect compression method.

Revision Contents
Changeset List

			Path	Packages
M			kerfuffle/archive_kerfuffle.h (4 lines)
M			kerfuffle/archive_kerfuffle.cpp (11 lines)
M			kerfuffle/archiveinterface.h (1 line)
M			kerfuffle/propertiesdialog.cpp (1 line)
M			kerfuffle/propertiesdialog.ui (64 lines)
M			plugins/cli7zplugin/cliplugin.h (1 line)
M			plugins/cli7zplugin/cliplugin.cpp (30 lines)
M			plugins/clirarplugin/cliplugin.cpp (5 lines)
M			plugins/cliunarchiverplugin/cliplugin.cpp (6 lines)
M			plugins/clizipplugin/cliplugin.h (2 lines)
M			plugins/clizipplugin/cliplugin.cpp (18 lines)
M			plugins/libarchive/libarchiveplugin.h (1 line)
M			plugins/libarchive/libarchiveplugin.cpp (28 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	7219			Oct 8 2016, 5:25 PM	★	★
Diff 2	7229			Oct 9 2016, 11:50 AM	★	★
Diff 3	7234	a3921c6	rARK50a9f590bd2d0a32ac3fe3e4d129637864c977de	Oct 9 2016, 4:38 PM	★	★