Re: master 64e25cd: More robust NS hex colour string parsing

[Mattias Engdegård]

13 juni 2020 kl. 20.40 skrev Eli Zaretskii <eliz@gnu.org>:

>> Since "#12345" is malformed it should be rejected, and will be.
> 
> That is definitely a change in behavior, isn't it?

Of course -- it is a bug fix, and as such a change in behaviour by definition.
It was one of the prime motivations behind these changes: I really had it after 
writing #12345 etc and getting nonsense values.
Note that this particular input is already firmly rejected by several if not 
all of the other backends; it isn't as if it provides a reliably mechanism that 
anyone is likely to use.

> Our convention is to use the US English spelling.

Replaced everywhere. (Misspelling on purpose is difficult!)

> Please also describe in more detail the value put in *DST, I don't
> think it's clear enough.

Elaborated.

> The commentary doesn't explain what is the "comp" part of the name
> about.

Names extended to ..._color_comp and comments amended to indicate that they 
parse a colour component.

> I think this doc string is too terse.  I would rephrase the beginning
> as follows:
> 
>    Convert a color SPEC into a list of standard RGB values.
> 
>  Value is a list of the form (R G B), where R, G, and B are the
>  integer values, the intensities of the primary colors.
>  The argument SPEC should be a string in one of the following formats:

Thank you, but I don't think we need to describe how RGB works here. I've 
provided more detail about the return value according to your example in other 
respects.

> In the "rgbi" description, I think we should mention explicitly that
> the components are floating-point numbers.

Done.

> What happens if the argument is not a string?  What should happen?

Thanks for catching that! An accidental omission.

> Finally, the Lisp primitive needs a NEWS entry and perhaps also a
> place in the ELisp manual.

Actually, now that you made me think of it, I'd rather make it internal since 
it isn't intended for general use; it's only exported to Lisp because one 
backend (TTY) needs to access it that way. In the attached patch, it has an 
internal- prefix. Not extremely important either way, of course.

(Or should it be internal-- with a double hyphen? Not sure how we do it 
nowadays.)

> Thanks.

Thanks for reviewing again! Oh, and if you (or someone else) could confirm that 
it compiles and works on Windows, I'd be most grateful.

0001-Consolidate-RGB-string-parsers.patch
Description: Binary data