Add support for editing comments
ClosedPublic

Authored by rthomsen on Apr 25 2016, 7:03 PM.

Details

Summary

Support was added for editing comments in supported archive types (currently only RAR). A new bool was added to plugin json files ("SupportsWriteComment") to indicate support. A new action (Edit Comment) was added and is found in Archive menu. The editing of comment is done in the same QPlainTextEdit used to display comment before. When user modifies comment, a KMessageWidget pops up with a "Save" button. Actual saving of comment to archive is achieved by a new job type (CommentJob).

Bug 357594

Test Plan
  • All unit tests pass.
  • Open RAR archive with comment -> QPlaintTextEdit is visible and editable.
  • Open Zip archive with comment -> QPlaintTextEdit is visible and readonly.

Diff Detail

Repository
R36 Ark
Lint
Lint Skipped
Unit
Unit Tests Skipped
rthomsen updated this revision to Diff 3518.Apr 25 2016, 7:03 PM
rthomsen retitled this revision from to Add support for editing comments.
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 TranscriptApr 25 2016, 7:03 PM
rthomsen updated this revision to Diff 3574.Apr 30 2016, 1:31 PM
  • Rebase against current master.
  • Remove finished call in CliInterface::addComment().
rthomsen updated this revision to Diff 3575.Apr 30 2016, 2:02 PM

Fix QTemporaryFile for comment.

rthomsen updated this object.Apr 30 2016, 2:28 PM
elvisangelaccio requested changes to this revision.Apr 30 2016, 2:38 PM
elvisangelaccio edited edge metadata.

One more thing, we should try to call the new action "Add Comment" (instead of "Edit Comment") if the archive has no comment yet.
It should be possible by checking this in updateActions()

kerfuffle/archive_kerfuffle.cpp
179 ↗(On Diff #3575)

Q_ASSERT(isReadOnly()); here

kerfuffle/cliinterface.cpp
701 ↗(On Diff #3575)

Copy-pasted comment, drop it :p

1182 ↗(On Diff #3575)

Missing emit finished(false) here. The process is not running, so must be done manually.

kerfuffle/cliinterface.h
464 ↗(On Diff #3575)

Please initialize this to null in the constructor, and delete it either in the destructor on in processFinished(). If you don't delete it the temp file won't be removed.

part/part.cpp
456 ↗(On Diff #3575)

Now that we have this function, can you also call it from Part::slotLoadingFinished() (~line 744-745)?

This revision now requires changes to proceed.Apr 30 2016, 2:38 PM
rthomsen updated this revision to Diff 3577.Apr 30 2016, 4:58 PM
rthomsen edited edge metadata.

Incorporate Elvis' suggestions.

rthomsen marked 5 inline comments as done.Apr 30 2016, 4:59 PM
rthomsen updated this revision to Diff 3578.Apr 30 2016, 5:04 PM
rthomsen edited edge metadata.

Hide comment box if user saves an empty comment.

elvisangelaccio accepted this revision.Apr 30 2016, 5:15 PM
elvisangelaccio edited edge metadata.

Good to go!

This revision is now accepted and ready to land.Apr 30 2016, 5:15 PM
This revision was automatically updated to reflect the committed changes.