Differential D1172

Fix broken overwrite query when creating new archives
ClosedPublic
Actions

Authored by elvisangelaccio on Mar 18 2016, 6:30 PM.

Details

Reviewers
rthomsen
Commits
R36:010affc22051: Let Ark overwrite existing archives
Summary

BUG: https://bugs.kde.org/show_bug.cgi?id=352687

We currently ask confirmation to overwrite an existing archive in the
CreateDialog. This is wrong because the Ark Part could be also used elsewhere
(and we don't know if they query the user).

The Part itself now asks the user whether to overwrite, and removes the
target file if the user confirms the operation.

Diff Detail

Repository
R36 Ark
Branch
fix-overwrite
Lint
No Linters Available
Unit
No Unit Test Coverage
elvisangelaccio updated this revision to Diff 2837.Mar 18 2016, 6:30 PM
elvisangelaccio retitled this revision from to Fix broken overwrite query when creating new archives.
elvisangelaccio added a reviewer: rthomsen.
elvisangelaccio updated this object.
Restricted Application added a project: Ark. · View Herald TranscriptMar 18 2016, 6:30 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald Transcript
rthomsen edited edge metadata.Mar 18 2016, 11:14 PM

Code looks good. I guess it would be best to not delete the old archive until the user actually creates a new archive by adding some files. Is this possible with the current architecture?

part/part.cpp
605 ↗(On Diff #2837)

I think "Could not overwrite..." would be a better choice here...

elvisangelaccio added a comment.Mar 19 2016, 11:17 AM
In D1172#22309, @rthomsen wrote:

Code looks good. I guess it would be best to not delete the old archive until the user actually creates a new archive by adding some files. Is this possible with the current architecture?

I think it would be possible, we would just need a boolean to carry around until an AddJob starts. However there is another problem, if we don't delete the old archive, its content will show up in the ArchiveView. I'm not sure how to prevent this...

elvisangelaccio updated this revision to Diff 2846.Mar 19 2016, 11:21 AM
elvisangelaccio edited edge metadata.

Use Ragnar's suggestion

elvisangelaccio marked an inline comment as done.Mar 19 2016, 11:22 AM
rthomsen accepted this revision.Mar 19 2016, 7:29 PM
rthomsen edited edge metadata.

Ship it! :)

This revision is now accepted and ready to land.Mar 19 2016, 7:29 PM
Closed by commit R36:010affc22051: Let Ark overwrite existing archives (authored by elvisangelaccio). · Explain WhyMar 20 2016, 10:17 AM
This revision was automatically updated to reflect the committed changes.
elvisangelaccio mentioned this in R36:010affc22051: Let Ark overwrite existing archives.

Revision Contents
Changeset List

PathPackages
Mkerfuffle/createdialog.cpp (1 line)
Mpart/part.h (6 lines)
Mpart/part.cpp (28 lines)

Diff 2846

View Options

kerfuffle/createdialog.cpp

Loading...
View Options

part/part.h

Loading...
View Options

part/part.cpp

Loading...