bug#36304: 27.0.50; request: switch to the superior HTML #RGB convention for colors

[Pip Cet <pipcet@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]

On Fri, Jun 28, 2019 at 9:28 PM Andy Moreton <andrewjmoreton@gmail.com> wrote:
> On Fri 28 Jun 2019, Pip Cet wrote:
>
> > This patch also incorporates Andy's suggestions, and adds some tests.
>
> There is an unintended change to src/atimer.c, but otherwise looks good.

Rebased and revised patch attached.

[-- Attachment #2: 0001-Use-the-CSS-convention-for-RGB-colors-bug-36304.patch --]
[-- Type: text/x-patch, Size: 11940 bytes --]

From be8465a394466ed126b34c1b37cebf13ac5e4e61 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Mon, 22 Jul 2019 02:40:35 +0000
Subject: [PATCH] Use the CSS convention for #RGB colors (bug#36304)

* src/xterm.c (x_parse_color): Change interpretation of #RGB color
triplets to match CSS rather than X conventions.

* lisp/term/tty-colors.el (tty-color-standard-values): Change
interpretation of #RGB color triplets to match CSS rather than X
conventions.  Allow upper-case digits.  Fix rgb:R/G/B
interpretation.

* doc/emacs/display.texi (Colors): Specify the convention used for
"#RGB" color triplets.

* test/lisp/tty-colors-tests.el: New file.

* etc/NEWS: Mention the change.
---
 doc/emacs/display.texi        | 18 +++++----
 etc/NEWS                      |  7 +++-
 lisp/term/tty-colors.el       | 74 +++++++++++++++++++----------------
 src/xterm.c                   | 33 +++++++++++++++-
 test/lisp/tty-colors-tests.el | 38 ++++++++++++++++++
 5 files changed, 126 insertions(+), 44 deletions(-)
 create mode 100644 test/lisp/tty-colors-tests.el

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 0ce291335a..8e842bea17 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -556,14 +556,14 @@ Faces
 
 @node Colors
 @section Colors for Faces
-@cindex color name
-@cindex RGB triplet
 
   Faces can have various foreground and background colors.  When you
 specify a color for a face---for instance, when customizing the face
 (@pxref{Face Customization})---you can use either a @dfn{color name}
 or an @dfn{RGB triplet}.
 
+@subsection Color Names
+@cindex color name
 @findex list-colors-display
 @vindex list-colors-sort
   A color name is a pre-defined name, such as @samp{dark orange} or
@@ -578,12 +578,16 @@ Colors
 text terminals; if a face is given a color specified by an X11 color
 name, it is displayed using the closest-matching terminal color.
 
+@subsection RGB Triplets
+@cindex RGB triplet
   An RGB triplet is a string of the form @samp{#RRGGBB}.  Each of the
-R, G, and B components is a hexadecimal number specifying the
-component's relative intensity, one to four digits long (usually two
-digits are used).  The components must have the same number of digits.
-For hexadecimal values A to F, either upper or lower case are
-acceptable.
+primary color components is represented by a hexadecimal number
+between @samp{00} (intensity 0) and @samp{FF} (the maximum intensity).
+It is also possible to use one, three, or four hex digits for each
+component, so @samp{red} can be represented as @samp{#F00},
+@samp{#fff000000}, or @samp{#ffff00000000}.  The components must have
+the same number of digits.  For hexadecimal values A to F, either
+upper or lower case are acceptable.
 
   The @kbd{M-x list-colors-display} command also shows the equivalent
 RGB triplet for each named color.  For instance, @samp{medium sea
diff --git a/etc/NEWS b/etc/NEWS
index 41debac50e..62ea50b239 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -402,6 +402,12 @@ mode they are described in the manual "(emacs) Display".
 names in xref buffers.
 ** New variable `file-size-function' controls how file sizes are displayed
 
++++
+** Emacs now interprets RGB triplets like HTML, SVG, and CSS do.
+
+The X convention previously used differed slightly, particularly for
+RGB triplets with a single hexadecimal digit per component.
+
 \f
 * Editing Changes in Emacs 27.1
 
@@ -1477,7 +1483,6 @@ automatically updates.  In the buffer, you can use 's q' or 's e' to
 signal a thread with quit or error respectively, or get a snapshot
 backtrace with 'b'.
 
-
 ** thingatpt.el
 
 ---
diff --git a/lisp/term/tty-colors.el b/lisp/term/tty-colors.el
index 5af8170203..43c1071ceb 100644
--- a/lisp/term/tty-colors.el
+++ b/lisp/term/tty-colors.el
@@ -919,57 +919,63 @@ tty-color-standard-values
 The result is a list of integer RGB values--(RED GREEN BLUE).
 These values range from 0 to 65535; white is (65535 65535 65535).
 
-The returned value reflects the standard X definition of COLOR,
-regardless of whether the terminal can display it, so the return value
-should be the same regardless of what display is being used."
+The returned value reflects the standard Emacs definition of
+COLOR (see the info node `(emacs) Colors'), regardless of whether
+the terminal can display it, so the return value should be the
+same regardless of what display is being used."
   (let ((len (length color)))
-    (cond ((and (>= len 4) ;; X-style "#XXYYZZ" color spec
+    (cond ((and (>= len 4) ;; HTML/CSS/SVG-style "#XXYYZZ" color spec
 		(eq (aref color 0) ?#)
 		(member (aref color 1)
 			'(?0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9
-			     ?a ?b ?c ?d ?e ?f)))
-	   ;; Translate the string "#XXYYZZ" into a list
-	   ;; of numbers (XX YY ZZ).  If the primary colors
-	   ;; are specified with less than 4 hex digits,
-	   ;; the used digits represent the most significant
-	   ;; bits of the value (e.g. #XYZ = #X000Y000Z000).
+			     ?a ?b ?c ?d ?e ?f
+                             ?A ?B ?C ?D ?E ?F)))
+	   ;; Translate the string "#XXYYZZ" into a list of numbers
+	   ;; (XX YY ZZ), scaling each to the {0..65535} range.  This
+	   ;; follows the HTML color convention, where both "#fff" and
+	   ;; "#ffffff" represent the same color, white.
 	   (let* ((ndig (/ (- len 1) 3))
+		  (maxval (1- (ash 1 (* 4 ndig))))
 		  (i1 1)
 		  (i2 (+ i1 ndig))
-		  (i3 (+ i2 ndig)))
+		  (i3 (+ i2 ndig))
+                  (i4 (+ i3 ndig)))
 	     (list
-	      (ash
-	       (string-to-number (substring color i1 i2) 16)
-	       (* 4 (- 4 ndig)))
-	      (ash
-	       (string-to-number (substring color i2 i3) 16)
-	       (* 4 (- 4 ndig)))
-	      (ash
-	       (string-to-number (substring color i3) 16)
-	       (* 4 (- 4 ndig))))))
-	  ((and (>= len 9) ;; X-style RGB:xx/yy/zz color spec
+	      (/ (* (string-to-number
+		     (substring color i1 i2) 16)
+		    65535)
+		 maxval)
+	      (/ (* (string-to-number
+		     (substring color i2 i3) 16)
+		    65535)
+		 maxval)
+	      (/ (* (string-to-number
+		     (substring color i3 i4) 16)
+		    65535)
+		 maxval))))
+	  ((and (>= len 9) ;; X-style rgb:xx/yy/zz color spec
 		(string= (substring color 0 4) "rgb:"))
-	   ;; Translate the string "RGB:XX/YY/ZZ" into a list
-	   ;; of numbers (XX YY ZZ).  If fewer than 4 hex
-	   ;; digits are used, they represent the fraction
-	   ;; of the maximum value (RGB:X/Y/Z = #XXXXYYYYZZZZ).
+	   ;; Translate the string "rgb:XX/YY/ZZ" into a list of
+	   ;; numbers (XX YY ZZ), scaling each to the {0..65535}
+	   ;; range.  "rgb:F/F/F" is white.
 	   (let* ((ndig (/ (- len 3) 3))
 		  (maxval (1- (ash 1 (* 4 (- ndig 1)))))
 		  (i1 4)
 		  (i2 (+ i1 ndig))
-		  (i3 (+ i2 ndig)))
+		  (i3 (+ i2 ndig))
+                  (i4 (+ i3 ndig)))
 	     (list
 	      (/ (* (string-to-number
 		     (substring color i1 (- i2 1)) 16)
-		    255)
+		    65535)
 		 maxval)
 	      (/ (* (string-to-number
 		     (substring color i2 (- i3 1)) 16)
-		    255)
+		    65535)
 		 maxval)
 	      (/ (* (string-to-number
-		     (substring color i3) 16)
-		    255)
+		     (substring color i3 (1- i4)) 16)
+		    65535)
 		 maxval))))
 	  (t
 	   (cdr (assoc color color-name-rgb-alist))))))
@@ -977,9 +983,9 @@ tty-color-standard-values
 (defun tty-color-translate (color &optional frame)
   "Given a color COLOR, return the index of the corresponding TTY color.
 
-COLOR must be a string that is either the color's name, or its X-style
-specification like \"#RRGGBB\" or \"RGB:rr/gg/bb\", where each primary.
-color can be given with 1 to 4 hex digits.
+COLOR must be a string that is either the color's name, or its
+color triplet specification like \"#RRGGBB\" or \"rgb:RR/GG/BB\",
+where each primary color can be given with 1 to 4 hex digits.
 
 If COLOR is a color name that is found among supported colors in
 `tty-color-alist', the associated index is returned.  Otherwise, the
@@ -987,7 +993,7 @@ tty-color-translate
 looking up the name in `color-name-rgb-alist', are used to find the
 supported color that is the best approximation for COLOR in the RGB
 space.
-If COLOR is neither a valid X RGB specification of the color, nor a
+If COLOR is neither a valid RGB specification of the color, nor a
 name of a color in `color-name-rgb-alist', the returned value is nil.
 
 If FRAME is unspecified or nil, it defaults to the selected frame."
diff --git a/src/xterm.c b/src/xterm.c
index c96aa74a7a..75568a82a1 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2381,6 +2381,8 @@ x_query_frame_background_color (struct frame *f, XColor *bgcolor)
   x_query_colors (f, bgcolor, 1);
 }
 
+#define HEX_COLOR_NAME_LENGTH 32
+
 /* On frame F, translate the color name to RGB values.  Use cached
    information, if possible.
 
@@ -2398,9 +2400,36 @@ x_query_frame_background_color (struct frame *f, XColor *bgcolor)
 
   if (color_name[0] == '#')
     {
-      /* The hex form is parsed directly by XParseColor without
+      /* Don't pass #RGB strings directly to XParseColor, because that
+	 follows the X convention of zero-extending each channel
+	 value: #f00 means #f00000.  We want the convention of scaling
+	 channel values, so #f00 means #ff0000, just as it does for
+	 HTML, SVG, and CSS.
+
+	 So we translate #f00 to rgb:f/0/0, which X handles
+	 differently. */
+      char rgb_color_name[HEX_COLOR_NAME_LENGTH];
+      int len = strlen (color_name);
+      int digits_per_channel;
+      if (len == 4)
+	digits_per_channel = 1;
+      else if (len == 7)
+	digits_per_channel = 2;
+      else if (len == 10)
+	digits_per_channel = 3;
+      else if (len == 13)
+	digits_per_channel = 4;
+      else
+	return 0;
+
+      snprintf (rgb_color_name, sizeof rgb_color_name, "rgb:%.*s/%.*s/%.*s",
+		digits_per_channel, color_name + 1,
+		digits_per_channel, color_name + digits_per_channel + 1,
+		digits_per_channel, color_name + 2 * digits_per_channel + 1);
+
+      /* The rgb form is parsed directly by XParseColor without
 	 talking to the X server.  No need for caching.  */
-      return XParseColor (dpy, cmap, color_name, color);
+      return XParseColor (dpy, cmap, rgb_color_name, color);
     }
 
   for (cache_entry = FRAME_DISPLAY_INFO (f)->color_names; cache_entry;
diff --git a/test/lisp/tty-colors-tests.el b/test/lisp/tty-colors-tests.el
new file mode 100644
index 0000000000..0570d1bf5b
--- /dev/null
+++ b/test/lisp/tty-colors-tests.el
@@ -0,0 +1,38 @@
+;;; tty-colors-tests.el --- tests for tty-colors.el  -*- lexical-binding: t -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+\f
+;;; Code:
+(require 'ert)
+(require 'term/tty-colors)
+
+(ert-deftest tty-colors-test-standard-colors ()
+  (should (equal (tty-color-standard-values "white") '(65535 65535 65535)))
+  (should (equal (tty-color-standard-values "#F00") '(65535 0 0)))
+  (should (equal (tty-color-standard-values "#00FF00") '(0 65535 0)))
+  (should (equal (tty-color-standard-values "#00000000FFFF") '(0 0 65535)))
+  (should (equal (tty-color-standard-values "rgb:0/0/7") '(0 0 30583)))
+  (should (equal (tty-color-standard-values "rgb:0/ff/0") '(0 65535 0)))
+  (should (equal (tty-color-standard-values "rgb:ffFF/0000/0000") '(65535 0 0))))
+
+(provide 'term-tests)
+
+;;; term-tests.el ends here
-- 
2.22.0