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

[David Faure]

dfaure added a comment.


  In D14631#499600 <https://phabricator.kde.org/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 imagine a KIO::moveAs that takes two QList<QUrl> and then this information is fetched from there rather than using m_dest.
  >>  In fact, if the existing moveAs() method is ported to call the two-QLists one, that will mean less special casing in the code (which wouldn't use m_dest anymore in slotResultStating, when m_asMethod).
  > 
  > The two list version could work, but I was thinking of one `QList<QSomeStruct>` that contains src and dest names. Looks less error prone IMO.
  
  Yes, or that. Might be more tedious to fill in, I don't know. It's ok with me in any case.
  
  > I was thinking of implementing this as a new subclass of Job where it will create the new Job and add a subjob for each of the files to be moved.
  
  Like all composite jobs, yes.
  
  This is less efficient, though, in the case where the dest dir is the same for 500 files because EACH moveAs will :
  
  - stat() the dest dir
  - check for enough free space at destination
  
  whereas this could be done only if the dest dir is different from the previous one.
  
  Also, user interaction like "skip all" or "overwrite all" won't be possible, because that's within a single CopyJob. If you start 500 copyjobs, this information won't be shared.
  
  > overloading will not break binary compatibility
  
  That's correct, it doesn't.
  It just has to be non-ambiguous for existing code, but that's fine for the current proposals.
  
  > then this can be an overloaded `KIO::moveAs(QList<SomeStruct>)`
  
  If that doesn't return a CopyJob like the other moveAs method, it'll be a bit confusing in itself IMHO.
  
  > It also has to remain as a single job so that it can be undone in one go instead of undoing for each item that was renamed.
  
  Yes, that part is clear and agreed upon.
  
  >   auto items = {
  >           KioRenameItem{QUrl("~/a.doc"), QUrl("~/Documents/a.doc")},
  >           KioRenameItem{QUrl("~/dir/file"), QUrl("~/Documents/file-2019")},
  >   };
  
  QUrl doesn't support ~ and needs fromLocalFile() when built from local paths, but I get the idea :-)
  
  Maybe the struct could be named KIO::MoveItem ?
  Generally we call it renaming when it's in the same directory and moving for the general case (same or different directory).
  
  > The option of adapting move to accept a list of dest also involves modifying CopyJobPrivate's dest which will lead to a larger refactoring needed than the proposal below, right?
  
  CopyJobPrivate's m_dest already changes over time (when copying dirs it has to recurse into subdirs at dest). Only m_globalDest is currently fixed, and would have to change over time now.
  And when it changes, we need to go back to state DEST_NOT_STATED and the "Stat the dest" code, to stat the new dest (to check that it exists and is a dir, check for free space).
  
  IMHO it's not a question of which is the bigger amount of work, but which one will work better. Support for "Overwrite all" requires that this is done within CopyJob.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190721/23fd09f5/attachment-0001.html>