GNU bug report logs - #3677
23.0.95; facemenu-read-color should not require match

Previous Next

Full log

Message #13 received at 3677-done <at> emacsbugs.donarmstrong.com (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Chong Yidong'" <cyd <at> stupidchicken.com>
Cc: "'Jay Berkenbilt'" <ejb <at> ql.org>, <3677-done <at> debbugs.gnu.org>
Subject: RE: 23.0.95; facemenu-read-color should not require match
Date: Sun, 16 Aug 2009 11:48:07 -0700

> > 1. I don't understand your change. Why define a `completer'
> > functionhere, 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.
> 
> The completer function checks `color-defined-p' if the color passed by
> the user is not a predefined color name.  That's how it accepts RGB
> triplets.

Ah, right; got it. Thx.

> > 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.
> 
> That's true, we should probably merge facemenu-read-color and
> read-color.

Just use `read-color'. There is nothing additional that `facemenu-read-color'
offers, except that it recognizes `facemenu-color-alist'. `facemenu-color-alist'
is only an internal variable, and it is not defined (beyond nil) or used
anywhere in the Emacs code. Its only purpose was to provide a list of colors for
completion, AFAIK. And if all supported colors are now included as completions
anyway, then it no longer serves a purpose. 

I suspect perhaps it was provided originally as a parallel to
`facemenu-listed-faces', which is now a defcustom. In any case, it seems that it
is only a vestige (cruft).

BTW, using your definition of `facemenu-read-color', I see two curious things
(probably having to do with the general completion code, not
`facemenu-read-color'):

1. Hitting TAB without typing anything at the prompt shows "Complete, but not
unique". That is clearly incorrect. The empty string is not a valid color name.

2. Typing #333333333 TAB shows "Complete, but not unique". Hitting TAB again
then shows "Sole completion". That sounds contradictory.

I suspect (without investigating, so probably wrongly) that it thinks the
completion is not unique because it cannot know what the completer function
might also include as another valid completion.

If so, then it would presumably always say "Complete, but not unique" whenever
you use a function-valued COLLECTION arg. If that's the case, then the message
should perhaps be changed to not be so definitive.

Currently, it is saying that there are in fact other completions with the same
prefix, even though #333333333 is complete. (That's true; #333333333333 is valid
too, but Emacs support doesn't reach beyond 4 hex digits per component, even
internally, AFAIK.) Is it saying that because it knows that there are such other
completions, or is it saying that because it just doesn't know? If the latter
(which I suspect), then the message should be something more like "Complete, but
not necessarily unique".

If we use `read-color' instead, then the behavior is more natural and more
usual:

1. TAB with no typed input immediately shows the completion candidates (instead
of "Complete, but not unique").

2. TAB after typing #333333333 shows "No match", which is true - it matches no
color name (all of the candidates are color names). However, completion is lax,
so when you hit RET the #333333333 is accepted correctly. This is the normal
behavior for lax completion - it is what users are used to.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.