D14631: Adds a new RenameDialog to KIO with more options for batch renaming

dfaure added a comment. Only if you can find a way to change BatchRenameJob in a binary and behaviour compatible way. And then it will be a dual-headed thing with two modes of operations, awful. All this sounds to me like much more trouble than writing a different job. REPOSITORY R241

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

luco added a comment. @emateli Thank you, I do understand that `BatchMoveJob` could be more versatile for a various kind of operations, but couldn't it be easier to just edit the `BatchRenameJob`'s hardcoded logic and make it more parametric? If I understand correctly you're referring to

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. You're completely right in the sense that for a batch rename files will be in the same folder. However, rename really is just a move operation. The current BatchRenameJob has a hardcoded logic about placeholders and extensions which make it unusable outside of its

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

luco added a comment. @emateli Thank you for your response, I saw that patch and I sincerely don't understand the reason why you're writing a completely different job (`BatchMoveJob`) when the required operation is exactly a `BatchRenameJob`. I'm asking this because I've never used a

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. I honestly think the GUI is mostly ready, at least what I wanted to achieve with it: Something that is advanced enough but not over the top with options as there's already tools for that. Will check at some point to finish the items left from the review here. What

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

luco added a comment. @emateli Are you still working on this? I'd love to see this feature implemented, I wanted to start by myself then on the Matrix Dolphin channel they suggested seeing this patch and it's awesome! Could you use some help? REPOSITORY R241 KIO REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. Well, this patch is more or less complete but it doesn't make sense to be deployed without batch operations from KIO as it would spawn a job for each file to be renamed. Like I stated in my last comment, I tried to give adding batch renaming a go but it felt

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a reviewer: meven. ngraham added a comment. @emateli are you going to be able to finish this? Should someone take over? It would be a shame for such great work to get stalled so close to the finish line. REPOSITORY R241 KIO REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a reviewer: Dolphin. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent, meven, #dolphin Cc: meven, anthonyfieroni, chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, LeGast00n,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. I spent some time with modifying CopyJob but didn't make much progress and it's too deep into KIO internals - a lot one has to familiarize himself with. Maybe it's best someone who already did work on KIO to add the batch stuff. Once that is done and merged we can

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

meven added a comment. This could be an alternative to D17595 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: meven, anthonyfieroni, chinmoyr, mlaurent, asensi, rkflx, dfaure,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. @emateli Ping :) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: anthonyfieroni, chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

dfaure added a comment. In D14631#499600 , @emateli wrote: > More or less but they don't have to be in the same directory. Think of it as a sequence of `KIO::moveAs` operations. Any N files can be moved anywhere. Right. >> I could

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. In D14631#499038 , @dfaure wrote: > KIO::move (implemented by CopyJob) can move N files to a single destination directory, but they get the same name at that dest. > KIO::moveAs (implemented by CopyJob too) can

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

dfaure added a comment. KIO::move (implemented by CopyJob) can move N files to a single destination directory, but they get the same name at that dest. KIO::moveAs (implemented by CopyJob too) can move/rename a single file to a specific filename at destination. What you're suggesting

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. Before continuing work I'd like to hear some opinions on this. 1. From @dfaure or other people at frameworks: How do we handle batch rejame jobs. Should I create a new patch to submit a basic batch move job that simply moves files around without any additional

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. Ping. :) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: anthonyfieroni, chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. No problem, thanks for checking in again! REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: anthonyfieroni, chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

anthonyfieroni added inline comments. INLINE COMMENTS > batchrenametypes.cpp:98-106 > +QStringList firstCapturedGroups() > +{ > +return capturedGroups; > +} > + > +void clearCapturedGroups() > +{ Make a class instead, free functions with global variable in, is no-go. It's totally miss of

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. I'll need to rewrite the BatchRenameJob part which does the batch processing without breaking binary compatibility. Been unusually busy lately between job and uni but I'll start working on this sooner than later. REPOSITORY R241 KIO REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. Ping! REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. Any news on this @emateli? I'd really love to see it in Dolphin! REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli planned changes to this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. > "By consulting https://lxr.kde.org/search?_filestring=&_string=BatchRenameJob we can see that Dolphin is the only application to make use of this BatchRenameJob API."

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. It might not be the worst thing in the world if you made this a general-purpose rename dialog, so that it handles the simple common case of renaming one items as well as the complicated case where multiple items are renamed. If you do that, I'll abandon D17595

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a subscriber: chinmoyr. emateli added a comment. Pinging @chinmoyr as the original author of the BatchRenameJob class. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: chinmoyr, mlaurent, asensi, rkflx,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 47694. emateli added a comment. - convert to namespace from class - do not show first captured group - remove help button - Use d-ptr pattern - remove show/hide methods and autoshow dialog - update doc - KFileItemList -> QList; KFileItem usage

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli marked 3 inline comments as done. emateli added a comment. There's still some work to do with the actual BatchRenameClass now and the d-pointer pattern. I'll update this in a few days. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. @dfaure, how's this looking now? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 45438. emateli marked 9 inline comments as done. emateli added a comment. - create on stack - upd define - remove unused import - use qstringliteral - remove unused export - match file name - remove import prefixes - remove q_unused on used

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > batchrenamedialog.cpp:38 > +#include > +#include > +#include Remove all the framework prefixes. E.g. this should be . This way, it fails to find the

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

mlaurent accepted this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. In D14631#355791 , @mlaurent wrote: > For me seems good +1 You still have a "Request changes" status for this patch. If you're happy with it now, can you change that to "Accepted? Thanks! REPOSITORY R241 KIO

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

mlaurent added a comment. For me seems good +1 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure, mlaurent Cc: mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 44938. emateli added a comment. - Initialize pointers to nullptr - Code style fix --- Thoughts on proceeding with the proposed changes to batchrenamejob? Also, the filenameutils namespace feels like duplicated work. Any existing solution

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

mlaurent added inline comments. INLINE COMMENTS > emateli wrote in batchrenamedialog.h:69 > You mean to initialize them to nullptr on the header file or using a > constructor initialization list? Also see what problem? in header file. REPOSITORY R241 KIO REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. Also, thoughts on my previous comments regarding `BatchRenameJob`? INLINE COMMENTS > mlaurent wrote in batchrenamedialog.h:69 > it's a good idea to initialize to nullptr each pointer here. > So we can see problem easily. You mean to initialize them to nullptr on the

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 44737. emateli marked 4 inline comments as done. emateli added a comment. - coding style fix REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14631?vs=44218=44737 BRANCH batchrename2 REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

mlaurent requested changes to this revision. mlaurent added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > batchrenamedialog.cpp:40 > +#include > +#include > +#include remove QtWidget > batchrenamedialog.cpp:41 > +#include > +#include > + remove QtGui

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli marked 2 inline comments as done. emateli added a comment. Something that might need a bit of input is `BatchRenameJob`. - This was added a while back so that Dolphin doesn't do a batch rename as a series of single file renames. - This job takes care of replacing the `#`

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 44218. emateli marked 3 inline comments as done. emateli added a comment. - Remove QtCore/ REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14631?vs=44217=44218 BRANCH batchrename2 REVISION DETAIL

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 44217. emateli added a comment. - fix indent - remove setlayout call - Add missing parent parameters - Remove unnecessary QtCore/ prefix REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14631?vs=44189=44217 BRANCH

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

mlaurent requested changes to this revision. mlaurent added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > CMakeLists.txt:65 > + rename/batchrenamedialog.cpp > +rename/batchrenamedialogmodel_p.cpp > +rename/batchrenametypes_p.cpp fix indent >

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. It makes sense to me to have that in this same patch. Pretty awesome stuff here. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure Cc: asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 44189. emateli added a comment. - use _p notation. Add BatchRenameDialog to installable headers --- Besides any current review, the last step for this patch would be to actually use the BatchRenameJob already in KIO. Regarding that: Should I put

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

aacid added a comment. In D14631#331827 , @emateli wrote: > @aacid Would be great if you'd point me towards making them installable. Only the dialog itself should be exported. install(FILES ${KIOWidgets_HEADERS}

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli added a comment. In D14631#303886 , @aacid wrote: > As far as i can see none of these headers get installed so should they all be renamed to _p.h ? > > Also if the headers don't get installed how do you use the new classes?

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli updated this revision to Diff 42324. emateli marked 15 inline comments as done. emateli added a comment. - Code style fixes - use .insert instead of [] - Indent Q_OBJECT - use isEmpty instead of len==0 - use aggregate initialization - protected -> private - added parent

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. Any progress on this, @emateli? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks, dfaure Cc: asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > batchrenametypestest.h:20 > + > +#ifndef KIO_RENAMETYPESTEST_H > +#define KIO_RENAMETYPESTEST_H Doesn't match the header filename, and anyway, feel free to

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

aacid added a comment. As far as i can see none of these headers get installed so should they all be renamed to _p.h ? Also if the headers don't get installed how do you use the new classes? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli,

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

ngraham added a comment. Awesome! Does Dolphin need a patch to use this? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

emateli retitled this revision from "Migrate D10698 from Dolphin to KIO" to "Adds a new RenameDialog to KIO with more options for batch renaming". emateli edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14631 To: emateli, #frameworks