[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eli Zaretskii
Subject: Re: master 64e25cd: More robust NS hex colour string parsing
Date: Sat, 13 Jun 2020 20:09:02 +0300

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 13 Jun 2020 18:44:04 +0200
> Cc: pipcet@gmail.com, emacs-devel@gnu.org
> 13 juni 2020 kl. 17.58 skrev Eli Zaretskii <eliz@gnu.org>:
> >> Try (color-values "#123"). The correct result is (#x1111 #x2222 #x3333).
> > 
> > Why is that the correct value?  I get (#x1010 #x2020 #x3030); why is
> > that wrong?
> It violates the HTML/CSS convention which was agreed upon in bug#36304 and 
> followed by the other backends. Single-digit hex numbers are scaled by 
> 65535/15, two-digit numbers by 65535/255, and three-digit numbers by 
> 65535/4095.

But the code you want to supplant explicitly does something different:

              value = strtoul (color, &end, 16);
              color[size] = t;
              if (errno == ERANGE || end - color != size)
              switch (size)
                case 1:
                  value = value * 0x10;
                case 2:
                case 3:
                  value /= 0x10;
                case 4:
                  value /= 0x100;

That multiplication by 0x10 cannot be a typo, it's a deliberate
interpretation of #f as #f0, not as #0f or #ff.

Of course, single-digit hex RGB specs are rarely if ever used these
days, AFAIK, so maybe this isn't a problem in practice.  But still, I
wonder what could we lose here or break.

> > Just follow the code, it should be very clear: those two branches
> > always return a list of values.  No example should be needed.
> An example could help resolve misunderstanding, and if we go back-and-forth 
> on what you think is a simple matter it's a clear sign that one is definitely 
> needed.
> > No, that's not true, as should be obvious from examining the code.
> > Previously, any "#..." string whose length was 4 or longer would
> > return a list of values, even if it wasn't well-formed; now some of
> > them will return nil.
> (tty-color-values "#xyz") returned nil (and still does), contradicting your 
> claim.

How about (tty-color-values "#12345")? does it contradict yours?

> I meant that the manner of rejection remains unchanged, not the set of 
> rejected arguments, which is a consequence of improved error-checking, very 
> much by design.
> Not only was it previously lacking, its coverage varied wildly between 
> backends. That means that hardly any code could have abused the lax checking 
> while still working on multiple platforms. Of course, the unpredictable 
> behaviour on malformed input made this a very dubious endeavour in the first 
> place.

You are changing the behavior, so get ready to hear bug reports, is
all I'm saying.

> > color-values-from-rgb-spec?
> Thank you, but that would preclude addition of non-RGB formats in the future, 
> such as HSV or XIE XYZ. Nothing in the interface forces the specification to 
> be RGB. In fact, Xlib accepts several non-RGB formats.

Then color-values-from-color-spec, I guess.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]