<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/118879/">https://git.reviewboard.kde.org/r/118879/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 23rd, 2014, 8:28 a.m. UTC, <b>Aurélien Gâteau</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;">Out of curiosity: why is it necessary to skip colors with spaces?</pre>
</blockquote>
<p>On June 24th, 2014, 12:15 a.m. UTC, <b>Alexander Potashev</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;">I guess it's a heuristic to remove duplicate entries for same {R,G,B}. However, it doesn't work perfectly, e.g. we have two entries "navy" and "NavyBlue" for the same RGB={0, 0, 128}.
A better algorithm would be to group all lines in rgb.txt by RGB and then choose one text from each group somehow, may be take the shortest one.
More important, we are now talking about Messages.sh which only populates translation templates with strings. The rgb.txt file parser used at run-time is elsewhere: see KColorTable::readNamedColor() in kdelibs4support/src/kdeui/kcolordialog.cpp</pre>
</blockquote>
<p>On June 24th, 2014, 8:27 a.m. UTC, <b>Aurélien Gâteau</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;">I see. Since it's in kdelibs4support, I don't think we should change the C++ code there.
I rewrote the shell line because I found it difficult to read. I thought removing color with spaces was a bug but I should have looked at the code using those strings. The awk script can be adjusted to skip names with spaces by only outputting the name if there are exactly 4 fields (a name with a space would result in 5 fields). This can be done like this:
awk '$1 ~ "[0-9]+" && $4 !~ "gr[ae]y" && NF == 4 { print $4 }' kdeui/rgb.txt
Use whichever version you prefer, I'd just ask adding a comment on top of the shell code explaining those strings are used by KColorTable::readNamedColor(), from src/kdeui/kcolordialog.cpp.</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;">Your AWK code is more readable than sed, thanks! I'll commit it later today adding some comments and a reference to KColorTable::readNamedColor().</pre>
<br />
<p>- Alexander</p>
<br />
<p>On June 22nd, 2014, 12:30 p.m. UTC, Alexander Potashev wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and Aurélien Gâteau.</div>
<div>By Alexander Potashev.</div>
<p style="color: grey;"><i>Updated June 22, 2014, 12:30 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs4support
</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;">The code using AWK did not properly throw away entries containing
spaces.
The code inserted in this commit was used successfully in KDE SC 4, see
kdelibs/kdeui/colors/Messages.sh</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/Messages.sh <span style="color: grey">(9b918fa)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118879/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>