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.
  >> 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 
  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 

  R241 KIO


To: emateli, #frameworks, dfaure, mlaurent
Cc: anthonyfieroni, chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, 
kde-frameworks-devel, LeGast00n, sbergeron, michaelh, bruns

Reply via email to