D9103: Add BatchRenameJob to KIO

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> chinmoyr wrote in batchrenamejob.cpp:75
> Here it is searching for exactly one sequence. If more than one sequence are present like $$file$$name$$ then it is considered invalid.

But why? Why doesn't this lead to 12file$$name$$?

It's a weird input in the first place, I don't mind if the output is weird, because the benefit of doing it that way is:

- easier documentation (no need to talk about the case of multiple disconnected sequences)
- faster implementation (look for first placeholder, look if repeated, done)

OK I see that you solved the latter point in this commit though, it's at least fast now. Then I don't mind as much. You can leave it as is, I just wanted to finish explaining my point ;)

> chinmoyr wrote in batchrenamejob.cpp:109
> I just copied whatever there was in Dolphin. Where should I move it to?

The public header, where people will see it ;)

I see you did that now, thanks.

> chinmoyr wrote in batchrenamejob.cpp:196
> In dolphin the overwrite dialog pops up. Otherwise the fileRenamed is still emitted and the job continues. Maybe I should make that its emission conditional?

Ah right KIO::moveAs itself pops up the overwrite dialog, but then can it end with KIO::ERR_FILE_ALREADY_EXIST ? At that point the user should overwrite or skip or cancel. Are you really sure this error code can happen in this slot?

> batchrenamejob.cpp:76
> +        if (first != -1) {
> +            while (first < newName.size() && newName.at(first++) == placeHolder);
> +        }

The trailing ';' looks like a mistake, even if it's not.

`{}` solves this, or moving `++first` to the body of the while (more readable IMHO).

BTW if it moves forward it shouldn't be called `first`, it makes the indexOf below very confusing. Maybe just call it `pos`.

> batchrenamejob.h:72
> + * the file to "Test 0012.jpg".
> + *
> + * @param src The list of items to rename.

... and then this should mention the case where no placeholder is present, and 12 gets appended (before the extension, IIUC).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9103

To: chinmoyr, #frameworks, dfaure
Cc: apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171202/ee9c5402/attachment.html>