[Geany-Devel] small bug in utils_strtod()

[Matthew Brush]

On 13-12-02 06:16 PM, Colomban Wendling wrote:
> Le 02/12/2013 21:54, Colomban Wendling a écrit :
>> Le 02/12/2013 21:54, Colomban Wendling a écrit :
>>> Le 01/12/2013 23:45, Matthew Brush a écrit :
>>>> On 13-12-01 01:26 PM, Lex Trotman wrote:
>>>>> Hi,
>>>>>
>>>>> **gint** utils_strtod()??  Sheesh.
>>>>>
>>>>> s/b renamed utils_hexcolor() or similar, thats what it does.
>>>>>
>>>>> As for "end", the original strtod() and friends used it to extract
>>>>> several
>>>>> whitespace separated strings from the one string by just passing "end" to
>>>>> the next call, but if we don't use it and its wrong anyways we might as
>>>>> well drop it when we change the name to utils_hexcolor().
>>>>>
>>>>
>>>> Agree the name is bad, and also the colour parsing isn't very robust. I
>>>> meant to improve it when adding the named_colors support but never got
>>>> around to it.
>>>>
>>>> Attached patch uses GDK colour parsing instead of own-rolled thing which
>>>> supports more colour specification formats as well as cleans up some
>>>> related code while remaining compatible with legacy colour notation and
>>>> GTK2 and GTK3 builds.
>>>>
>>>> If nobody objects, I can commit the patch.
>>>
>>> As said on IRC, I don't want the color parsing to change depending on
>>> the GTK version Geany is built against.
>>>
>>> Attached is a proposal using pango_color_parse() -- which is what
>>> gdk_color_parse() uses.  It supports the 3 and 4-digit channels, even
>>> though we don't actually use such a precision, so it's a bit of a lie,
>>> but I guess nobody cares anyway.
>>
>> Hum.
>
> Committed a different version that keeps our own parser, because as Lex
> pointed out on IRC the named colors are weird, and as I mentioned 3 and
> 4-digit channels don't make much sense to us since this precision would
> be lost anyway.  And it's not that complex.
>

I missed the IRC conversation but IMO named colours are not weird[1], 
actually it's quite readable to write "red" instead of some "cryptic" 
hex intensity value. Even decimal RGB support is quite nice since most 
people IME think in decimal and not hex, and most/all colour choosers 
and related tools give the RGB values in decimal (although most give hex 
values too).

IMO, even though it's fun to write a hand-made colour parser (which is 
something I specifically avoided doing in my patch, kind of the whole 
point of it actually :), we should still use the existing function 
already used elsewhere in the code and linked into the binary anyway, 
for the sake of avoiding duplicating code with our own not 
widely-tested, debugged and less robust/featureful version.

That being said, I don't really care much, it's not a huge deal and as 
you said the limited hand-rolled version isn't actually that complex.

> https://github.com/geany/geany/commit/3522e81d7344c584a78c2f52cf8bcd32f14dd38d
>
> @Matthew as said I only touched what was strictly related to
> utils_strtod(), you may want to commit your additional cleanups.
>

I did it once already and you didn't base your changes on mine so you 
can have the joy of repeating the effort if you would like :)

Cheers,
Matthew Brush

[1]: Go to a paint store and see how many paint swatches have colour 
names as opposed to hexadecimal notation printed on them :)