<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/128283/">https://git.reviewboard.kde.org/r/128283/</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;">Nice feature addition :)
Is there a plan for plugins to this plugin, so one could extend it with other checksum calculations? Not that serious here though ;)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Here some more quick more serious comments on the code:</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/128283/diff/6/?file=470493#file470493line2628" style="color: black; font-weight: bold; text-decoration: underline;">src/widgets/kpropertiesdialog.cpp</a>
<span style="font-weight: normal;">
(Diff revision 6)
</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">2628</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_defaultColor</span> <span class="o">=</span> <span class="n">m_widget</span><span class="p">.</span><span class="n">palette</span><span class="p">().</span><span class="n">color</span><span class="p">(</span><span class="n">QPalette</span><span class="o">::</span><span class="n">Base</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;">I propose to not cache the colors here, but instead fetch them when needed. There is no real performance gain to cache them.
And the cache might only result in conflicts in case somebody changes the system colors while the cache is active.</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/128283/diff/6/?file=470493#file470493line2807" style="color: black; font-weight: bold; text-decoration: underline;">src/widgets/kpropertiesdialog.cpp</a>
<span style="font-weight: normal;">
(Diff revision 6)
</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">2807</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">regex</span><span class="p">.</span><span class="n">match</span><span class="p">(</span><span class="n">input</span><span class="p">).</span><span class="n">hasMatch</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;">Not sure, but is QRegularExpression doing exact matches by default? IIRC other than QRegExp it does not, but also returns true on partial matches in the given string.
No time to check in details myself right now, so just asking you to give this a check, unless you are sure it is not an issue.</p></pre>
</div>
</div>
<br />
<p>- Friedrich W. H. Kossebau</p>
<br />
<p>On June 29th, 2016, 2:37 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 Frameworks, KDE Usability and David Faure.</div>
<div>By Elvis Angelaccio.</div>
<p style="color: grey;"><i>Updated June 29, 2016, 2:37 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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 adds a Checksums tab in the properties dialog, where the user can retrieve and verify the most popular checksum algorithms (md5, sha1 and sha256). </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To simplify the implementation, the checksums are computed as soon as the user opens the dialog. This can take a while if the file is huge (in particular with sha256), but the computation happens in another thread and in practice this should not be a performance problem.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The tab is available only for readable local files (no simlinks) and only when there is a single selection.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please note that some of the labels in the screenshots are clipped due to a bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426</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;"><ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Check whether the computed values match the values from md5sum, sha1sum, sha256sum.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Check whether the line edits get a green background if the computed and expected values match.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Check whether the line edits get a red background if the computed and expected values differ.</li>
</ul></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>src/widgets/CMakeLists.txt <span style="color: grey">(f906577)</span></li>
<li>src/widgets/checksumswidget.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/widgets/kpropertiesdialog.cpp <span style="color: grey">(d0a2faa)</span></li>
<li>src/widgets/kpropertiesdialog_p.h <span style="color: grey">(c01554e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128283/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/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png">MD5 ready to be shared</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png">Default dialog</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png">Mismatch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png">Match</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png">Invalid checksum</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>