<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/121997/">https://git.reviewboard.kde.org/r/121997/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On giugno 3rd, 2015, 5:59 p.m. UTC, <b>Ragnar Thomsen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">I tested it and it works great :) thanks for doing this.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think we need to discuss how Ark remembers the extraction settings now that we have two places where they can be set. I see these possibilities:</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;">Always use the settings from the settings page. This means that if the user changes the settings from the extraction dialog, then these settings are used only for that particular extraction. We should emphasize in the settings dialog, that these are the default settings.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">When the user changes the settings in the extraction dialog, store these settings and use them in the future. This means making sure the settings page displays the updated settings.</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What do you think? I think option one makes most sense.</p></pre>
</blockquote>
</blockquote>
<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;">Option two is what is already done by Ark. We could migrate to option one, but changing the behavior of the extraction dialog would be too risky, imho. Long-time Ark users could be confused/annoyed (e.g. "Why I have to check again option foo?!?"). Let's just say that the extraction dialog allows the user to quickly access those settings, without having to open the settings dialog first.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On giugno 3rd, 2015, 5:59 p.m. UTC, <b>Ragnar Thomsen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/121997/diff/5/?file=378719#file378719line27" style="color: black; font-weight: bold; text-decoration: underline;">kerfuffle/ark.kcfg</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><label>Limit preview file size</label></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<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 were the labels deleted here?</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">They were not used, were they?
Plus, I rephrased them in the actual configuration dialog.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On giugno 3rd, 2015, 5:59 p.m. UTC, <b>Ragnar Thomsen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/121997/diff/5/?file=378726#file378726line23" style="color: black; font-weight: bold; text-decoration: underline;">part/dialogs/previewsettings.ui</a>
<span style="font-weight: normal;">
(Diff revision 5)
</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">23</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <string>Do not preview large files</string></pre></td>
</tr>
</tbody>
</table>
<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;">I propose to change the text to:
"Disable preview for files larger than:"</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">and then put the QSpinBox directly after this text (i.e. delete the QLabel).</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">Are you also implying to drop the related boolean setting?</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On giugno 3rd, 2015, 5:59 p.m. UTC, <b>Ragnar Thomsen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/121997/diff/5/?file=378726#file378726line77" style="color: black; font-weight: bold; text-decoration: underline;">part/dialogs/previewsettings.ui</a>
<span style="font-weight: normal;">
(Diff revision 5)
</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">77</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <number>1000</number></pre></td>
</tr>
</tbody>
</table>
<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;">I think 1000MB might be to low for maximum. Some users might want to set it higher. Should we up it to 10000?</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">Maybe, not sure about this...</p></pre>
<br />
<p>- Elvis</p>
<br />
<p>On giugno 3rd, 2015, 4:50 p.m. UTC, Elvis Angelaccio 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 and Raphael Kubo da Costa.</div>
<div>By Elvis Angelaccio.</div>
<p style="color: grey;"><i>Updated Giu. 3, 2015, 4:50 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=165314">165314</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 the standard Configuration Dialog (i.e. using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KConfigDialog</code>) in Ark, as one would expect from a KDE application.
This feature has been requested in more than one bug. I choosed to target bug 165314 since the others are more like "add the config dialog to implement the feature <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">foo</em>".</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The widgets showed in the config dialog are provided by the Ark <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Part</code> interface (just one widget for now). This should help to show the Ark settings in, for instance, Konqueror's config dialog. A similar approach is done in Kate and Cantor, from what I have seen.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't like very much the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">document-save</code> icon used for the Extraction page, but I couldn't find any better.</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Compile and run, then try to change some settings and check that they persist.</p></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/mainwindow.h <span style="color: grey">(f2366c8bd047e3484daa4eabeb4f4b790c437c4a)</span></li>
<li>app/mainwindow.cpp <span style="color: grey">(470b72d00d0544662f2c1f10dbc0638b99f993af)</span></li>
<li>kerfuffle/ark.kcfg <span style="color: grey">(97d2086688698e96c429def089c50ff3cdbe4c4e)</span></li>
<li>part/CMakeLists.txt <span style="color: grey">(cca6d25e1941cecde07d0cd342cc2602fbc5235e)</span></li>
<li>part/arkconfigpage.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>part/arkconfigpage.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>part/dialogs/extractionsettings.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>part/dialogs/extractionsettings.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>part/dialogs/previewsettings.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>part/dialogs/previewsettings.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>part/interface.h <span style="color: grey">(40f590284502d23a2a4ffaa333bfd5b63e6ec773)</span></li>
<li>part/part.h <span style="color: grey">(4c81da5541c1143ad4cfe9093f9c307145ac561e)</span></li>
<li>part/part.cpp <span style="color: grey">(ee7f75ff9acd8ea3c72aa5a400d713ffc6d1c7c4)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121997/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/06/03/3dfe3110-4f46-4c14-9b19-79060577a4c9__extraction-settings.png">extraction-settings.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/06/03/92e1e6a4-6b6a-4a2f-b149-809501173925__preview-settings1.png">preview-settings1.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/06/03/200d41a5-6f06-4bea-9f92-d8899b051bf4__preview-settings2.png">preview-settings2.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>