<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124018/">https://git.reviewboard.kde.org/r/124018/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch looks good. I have only few comments from my side. Wait Raphael for the final word.</p></pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124018/diff/1/?file=378870#file378870line92" style="color: black; font-weight: bold; text-decoration: underline;">kerfuffle/archive_kerfuffle.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">typedef QHash<int, QVariant> ArchiveEntry;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">92</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">struct</span> <span class="n">fileRootNodePair</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Why not making it a class? Having a struct in one of the most important signature (i.e. <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">copyFiles</code>) doesn't look too good to me.
Plus, why not leaving <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QVariant</code> for the type of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">file</code> and <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">rootNode</code>? You could add getters to retrieve their <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QString</code> conversion when needed.</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124018/diff/1/?file=378870#file378870line156" style="color: black; font-weight: bold; text-decoration: underline;">kerfuffle/archive_kerfuffle.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">121</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ExtractJob</span><span class="o">*</span> <span class="nf">copyFiles</span><span class="p">(</span><span class="k">const</span> <span class="n">QList</span><span class="o"><</span><span class="n">QVariant</span><span class="o">></span> <span class="o">&</span> <span class="n">files</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span> <span class="o">&</span> <span class="n">destinationDir</span><span class="p">,</span> <span class="n">ExtractionOptions</span> <span class="n">options</span> <span class="o">=</span> <span class="n">ExtractionOptions</span><span class="p">());</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">156</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ExtractJob</span><span class="o">*</span> <span class="nf">copyFiles</span><span class="p">(</span><span class="k">const</span> <span class="n">QList</span><span class="o"><</span><span class="n">QVariant</span><span class="o">></span> <span class="o">&</span> <span class="n">files</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span> <span class="o">&</span> <span class="n">destinationDir</span><span class="p">,</span> <span class="n">ExtractionOptions</span> <span class="n">options</span> <span class="o">=</span> <span class="n">ExtractionOptions</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This function is not used anymore, why not removing it?</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124018/diff/1/?file=378881#file378881line66" style="color: black; font-weight: bold; text-decoration: underline;">part/part.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">66</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#include <QPair></span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Unnecessary <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">include</code>.</p></pre>
</div>
</div>
<br />
<p>- Elvis Angelaccio</p>
<br />
<p>On June 5th, 2015, 4:23 p.m. UTC, Ragnar Thomsen wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Utils, Elvis Angelaccio and Raphael Kubo da Costa.</div>
<div>By Ragnar Thomsen.</div>
<p style="color: grey;"><i>Updated June 5, 2015, 4:23 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=187152">187152</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ark
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch implements multiple drag'n'drop selection. This is used when dragging multiple files from Ark to e.g. Dolphin for extraction. It's based partially on work done by Raphael in the multiple-dnd-selection branch. However, in the multiple-dnd-selection branch, each cluster of files with a common parent node had to be run in an individual ExtractJob. This patch does the following:</p>
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Create a struct fileRootNodePair, which contains two QStrings: The filename including relative path and a RootNode.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Change the first argument of ExtractJob::ExtractJob, which specifies which files are to be extracted, from a QVariantList to QList<fileRootNodePair>.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">ReadOnlyArchiveInterface::copyFiles and the corresponding methods in inheriting classes (CliInterface, LibArchiveInterface, LibSingleFileInterface) are changed to take QList<fileRootNodePair> instead of QVariantList as argument.</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This allows us to pass an individual RootNode for each file, enabling the extraction to be done in a single ExtractJob, which is more appropriate since the UI can then display the total number of entries being extracted and a single progress bar.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The CLI plugins, e.g. zip and rar, still extract with full path, due to not supporting RootNodes. However, this is a separate issue and will be fixed in another commit.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Selecting and dragging a subfolder from an archive results in the folder being extracted without path to destination and all its subfolders/files being extracted with relative path (i.e. path below selected subfolder).</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Selecting and dragging one or more files results in the file being extracted without path.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">A single job tracker is displayed with the total number of files+folders and the progress bar works as expected.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Selecting and dragging files/folders from zip/rar archives results in the selected entries being extracted with full path.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Batching-extracting using ark -b <archive> works as expected.</li>
</ol></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>app/batchextract.cpp <span style="color: grey">(3ea90bd)</span></li>
<li>kerfuffle/archive_kerfuffle.h <span style="color: grey">(886da3e)</span></li>
<li>kerfuffle/archive_kerfuffle.cpp <span style="color: grey">(d5b290b)</span></li>
<li>kerfuffle/archiveinterface.h <span style="color: grey">(968237d)</span></li>
<li>kerfuffle/cliinterface.h <span style="color: grey">(e93d443)</span></li>
<li>kerfuffle/cliinterface.cpp <span style="color: grey">(2ce6a06)</span></li>
<li>kerfuffle/jobs.h <span style="color: grey">(2911cf5)</span></li>
<li>kerfuffle/jobs.cpp <span style="color: grey">(ff365c3)</span></li>
<li>part/archivemodel.h <span style="color: grey">(13e062d)</span></li>
<li>part/archivemodel.cpp <span style="color: grey">(7999e4b)</span></li>
<li>part/archiveview.cpp <span style="color: grey">(2acefbd)</span></li>
<li>part/part.h <span style="color: grey">(dadde04)</span></li>
<li>part/part.cpp <span style="color: grey">(084ef9f)</span></li>
<li>plugins/libarchive/libarchivehandler.h <span style="color: grey">(088c0fe)</span></li>
<li>plugins/libarchive/libarchivehandler.cpp <span style="color: grey">(598cbec)</span></li>
<li>plugins/libsinglefileplugin/singlefileplugin.h <span style="color: grey">(6140bfe)</span></li>
<li>plugins/libsinglefileplugin/singlefileplugin.cpp <span style="color: grey">(941022d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124018/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>