WIP | Added BatchMoveJob
Needs ReviewPublic

Authored by emateli on Mar 1 2020, 12:02 PM.

Details

Reviewers
dfaure
ngraham
Summary

Adds a BatchMoveJob class which enables moving multiple files in a single operation.

Looking for comments/thoughts. Regarding implementation, I took a look at batchrenamejob so I did something similar with multiple hidden sub-jobs. Some of the methods such as slotstart/slotresult are adapted from batchrenamejob. Should I include the original copyright or is that not necessary.

Also looking for some thoughts on the test suite with respect to the undo. The last test I wanted to do is to undo the move so that I can verify all files have been moved back to their original locations by calling a single undo.

Issues that depend on this: D14631

Diff Detail

Repository
R241 KIO
Branch
batchmove
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23118
Build 23136: arc lint + arc unit
emateli created this revision.Mar 1 2020, 12:02 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 1 2020, 12:02 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
emateli requested review of this revision.Mar 1 2020, 12:02 PM
emateli retitled this revision from Added BatchMoveJob to WIP | Added BatchMoveJob.Mar 1 2020, 12:08 PM
emateli edited the summary of this revision. (Show Details)
emateli added reviewers: dfaure, ngraham.

In that it supports moving them to different directories but as well as moving them with a different name. This operation is to allow also batch renaming of items. See this patch for more details: D14631

eg: BatchMove {a.txt -> Sample.txt, b.md -> new/Readme.md}. In the context of the mentioned patch the directory wouldn't change of course, but this is also a more generic move job. Move these files from here to somewhere else in a single job (also so that a single undo reverts everything back to the original place).

The general concept seems sane to me, but let's make sure @dfaure agrees.

Looks good, just a few things.

autotests/batchmovejobtest.cpp
91

Use newer syntax &KIO::BatchMoveJob::fileMoved?

104

This reads really scary! :-)

Can you rename the var to what it is, like m_homeTmpDir?

And this can be re-enabled, no?

Alternatively, use QTemporaryDir, it cleans up automatically.

src/core/batchmovejob.cpp
104

prefer prefix increment (++ in front)

src/core/batchmovejob.h
37

Where is this ctor needed?

Both could just be removed, then you can use aggregate initialization with {}

50

typo: simultaneously

(bulk purchase of the 'o' letter?) :)

52

5.69

src/widgets/fileundomanager.h
140

5.69

emateli added inline comments.Apr 3 2020, 8:06 AM
autotests/batchmovejobtest.cpp
89

@dfaure How do I correctly register this job so I can undo it in one operation (In the undo call a few lines below)