Differential D1111

When packing multiple selected files - use containing folder name
ClosedPublic
Actions

Authored by tctara on Mar 10 2016, 8:22 PM.

Details

Reviewers
elvisangelaccio
rthomsen
Commits
R36:80080c9e8eb2: Use containing-folder name when packing multiple selected files
Summary

Fix bug 279861
https://.kde.org/show_bug.cgi?=279861

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.
tctara updated this revision to Diff 2678.Mar 10 2016, 8:22 PM
tctara retitled this revision from to When packing multiple selected files - use containing folder name.
tctara updated this object.
tctara edited the test plan for this revision. (Show Details)
tctara added reviewers: elvisangelaccio, rthomsen.
tctara set the repository for this revision to R36 Ark.
tctara added a project: Ark.
tctara added a subscriber: Ark.
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptMar 10 2016, 8:22 PM
rthomsen accepted this revision.Mar 10 2016, 9:08 PM
rthomsen edited edge metadata.

Tested and works as intented. The change makes sense. Nice work! :)

kerfuffle/addtoarchive.cpp
166

Coding style: Please remove the spaces within the QLatin1Char().

This revision is now accepted and ready to land.Mar 10 2016, 9:08 PM
tctara updated this revision to Diff 2681.Mar 10 2016, 9:12 PM
tctara updated this object.
tctara edited edge metadata.
elvisangelaccio requested changes to this revision.Mar 10 2016, 9:24 PM
elvisangelaccio edited edge metadata.

The logic for the bugfix looks ok.
But I would prefer to move this new code to a dedicated function ;)

kerfuffle/addtoarchive.cpp
162

How about introducing a new (public) function?
You would then do something like const QString base = detectBaseName(m_input);

This would allow us to easily unit test it, in the future.

This revision now requires changes to proceed.Mar 10 2016, 9:24 PM
tctara updated this revision to Diff 2685.Mar 10 2016, 9:56 PM
tctara edited edge metadata.
tctara marked an inline comment as done.
elvisangelaccio accepted this revision.Mar 10 2016, 10:00 PM
elvisangelaccio edited edge metadata.

Thanks! Nice work :)

kerfuffle/addtoarchive.h
63

The function should be const.
The stringlist should be passed by const reference.

This revision is now accepted and ready to land.Mar 10 2016, 10:00 PM
tctara updated this revision to Diff 2686.Mar 10 2016, 10:10 PM
tctara edited edge metadata.
tctara marked an inline comment as done.

Revision Contents
Changeset List

PathPackages
Mkerfuffle/addtoarchive.h (1 line)
Mkerfuffle/addtoarchive.cpp (23 lines)

Diff 2687

View Options

kerfuffle/addtoarchive.h

Loading...
View Options

kerfuffle/addtoarchive.cpp

Loading...