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

Lint Skipped

Unit

Unit Tests Skipped

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	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 (5 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 (35 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 (23 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	★	★