Review Request 118879: Revert to old shell script code for parsing rgb.txt

[Alexander Potashev]

> On June 23, 2014, 8:28 a.m., Aurélien Gâteau wrote:
> > Out of curiosity: why is it necessary to skip colors with spaces?
> 
> Alexander Potashev wrote:
>     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
> 
> Aurélien Gâteau wrote:
>     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.

Your AWK code is more readable than sed, thanks! I'll commit it later today adding some comments and a reference to KColorTable::readNamedColor().


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118879/#review60754
-----------------------------------------------------------


On June 22, 2014, 12:30 p.m., Alexander Potashev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118879/
> -----------------------------------------------------------
> 
> (Updated June 22, 2014, 12:30 p.m.)
> 
> 
> Review request for KDE Frameworks and Aurélien Gâteau.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/Messages.sh 9b918fa 
> 
> Diff: https://git.reviewboard.kde.org/r/118879/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Potashev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140625/1cc61a08/attachment-0001.html>