Subject: RE: 23.0.95; facemenu-read-color should not require match

From: "Drew Adams" <[email-masked]>
Date: Sun, 16 Aug 2009 07:56:44 -0700

> >   M-x set-cursor-color RET #9ef RET
> >
> >   -> [no match]
> >
> > The same thing happens with set-foreground-color and
> > set-background-color, all of which call facemenu-read-color, but not
> > with set-face-foreground and set-face-background, which do not.  All
> > above mentioned functions do completing reads on color 
> names, which is
> > appropriate, but they should also accept #xxx, #xxxxxx, 
> rgb:xx/xx/xx,
> > etc.  If you do M-: (set-cursor-color "#9ef"), it works, so this is
> > clearly a case of the wrong kind of completing read being done.
> 
> Thanks for the bug report.  I've hacked up the completion function so
> that it allows any defined colors, including RGB triplets.

1. I don't understand your change. Why define a `completer' function here,
instead of just using `completing-read' directly as before? And you still pass
`t' as the REQUIRE-MATCH parameter, so it's not clear to me how this accepts
#RRRGGGBBB colors. I didn't try it - I assume it works if you say it does, but I
don't see why it would or why the completer function is needed here. 

2. Why don't you just use the function `read-color', defined in faces.el? That's
exactly what it's for - it handles all colors correctly, including both named
colors and #RRRGGGBBB (hex RGB string) colors.

,----
| read-color is an interactive compiled Lisp function in `faces.el'.
| 
| (read-color &optional PROMPT CONVERT-TO-RGB-P ALLOW-EMPTY-NAME-P
| MSG-P)
| 
| Read a color name or RGB hex value: #RRRRGGGGBBBB.
| Completion is available for color names, but not for RGB hex strings.
| If the user inputs an RGB hex string, it must have the form
| #XXXXXXXXXXXX or XXXXXXXXXXXX, where each X is a hex digit.  The
| number of Xs must be a multiple of 3, with the same number of Xs for
| each of red, green, and blue.  The order is red, green, blue.
| 
| In addition to standard color names and RGB hex values, the following
| are available as color candidates.  In each case, the corresponding
| color is used.
| 
|  * `foreground at point'   - foreground under the cursor
|  * `background at point'   - background under the cursor
| 
| Checks input to be sure it represents a valid color.  If not, raises
| an error (but see exception for empty input with non-nil
| ALLOW-EMPTY-NAME-P).
| 
| Optional arg PROMPT is the prompt; if nil, uses a default prompt.
| 
| Interactively, or with optional arg CONVERT-TO-RGB-P non-nil, converts
| an input color name to an RGB hex string.  Returns the RGB hex string.
| 
| Optional arg ALLOW-EMPTY-NAME-P controls what happens if the user
| enters an empty color name (that is, just hits `RET').  If non-nil,
| then returns an empty color name, "".  If nil, then raises an error.
| Programs must test for "" if ALLOW-EMPTY-NAME-P is non-nil.  They
| can then perform an appropriate action in case of empty input.
| 
| Interactively, or with optional arg MSG-P non-nil, echoes the color in
| a message.
`----

You can forget about #1 - I'm just curious. I'm probably missing something here.
But #2 seems pertinent, at least. Using `read-color' seems like the better fix.