<table><tr><td style="">dfaure added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D14631">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D14631#499600" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D14631#499600</a>, <a href="https://phabricator.kde.org/p/emateli/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@emateli</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>More or less but they don't have to be in the same directory. Think of it as a sequence of <tt style="background: #ebebeb; font-size: 13px;">KIO::moveAs</tt> operations. Any N files can be moved anywhere.</p></div>
</blockquote>
<p>Right.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>I could imagine a KIO::moveAs that takes two QList<QUrl> and then this information is fetched from there rather than using m_dest.<br />
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).</p></blockquote>
<p>The two list version could work, but I was thinking of one <tt style="background: #ebebeb; font-size: 13px;">QList<QSomeStruct></tt> that contains src and dest names. Looks less error prone IMO.</p></blockquote>
<p>Yes, or that. Might be more tedious to fill in, I don't know. It's ok with me in any case.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.</p></blockquote>
<p>Like all composite jobs, yes.</p>
<p>This is less efficient, though, in the case where the dest dir is the same for 500 files because EACH moveAs will :</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">stat() the dest dir</li>
<li class="remarkup-list-item">check for enough free space at destination</li>
</ul>
<p>whereas this could be done only if the dest dir is different from the previous one.</p>
<p>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.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>overloading will not break binary compatibility</p></blockquote>
<p>That's correct, it doesn't.<br />
It just has to be non-ambiguous for existing code, but that's fine for the current proposals.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>then this can be an overloaded <tt style="background: #ebebeb; font-size: 13px;">KIO::moveAs(QList<SomeStruct>)</tt></p></blockquote>
<p>If that doesn't return a CopyJob like the other moveAs method, it'll be a bit confusing in itself IMHO.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.</p></blockquote>
<p>Yes, that part is clear and agreed upon.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">auto items = {
KioRenameItem{QUrl("~/a.doc"), QUrl("~/Documents/a.doc")},
KioRenameItem{QUrl("~/dir/file"), QUrl("~/Documents/file-2019")},
};</pre></div></blockquote>
<p>QUrl doesn't support ~ and needs fromLocalFile() when built from local paths, but I get the idea :-)</p>
<p>Maybe the struct could be named KIO::MoveItem ?<br />
Generally we call it renaming when it's in the same directory and moving for the general case (same or different directory).</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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?</p></blockquote>
<p>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.<br />
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).</p>
<p>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.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D14631">https://phabricator.kde.org/D14631</a></div></div><br /><div><strong>To: </strong>emateli, Frameworks, dfaure, mlaurent<br /><strong>Cc: </strong>anthonyfieroni, chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, bruns<br /></div>