[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: read-face-name bugs
From: |
Drew Adams |
Subject: |
RE: read-face-name bugs |
Date: |
Sun, 2 Jul 2006 15:56:05 -0700 |
Well, I don't really care what you do with my bug report,
but my point is
that (I believe that) it does *not* reliably do the right
thing. It does the
right thing only if there is in fact no list of faces that matches
((foreground-color . "Firebrick")...).
They have different data layouts; there is no way one could
match the other.
OK. So why (seemingly) test for (not (memq (car faceprop) '(foreground-color
background-color))))? That test presumably is meant to exclude such an
old-format attribute list. The test 1) doesn't do that and 2) is
unnecessary, as you point out: such a list can never be confused with a list
of faces.
As I said:
If you don't care about this case, then you are better off
removing this part of the code - it simply does not serve any
purpose. Either you rely upon no feasible confusion of a face
list with the form above (in which case the test is not
needed), or you worry about such possible confusion (in which
case it does not work).
In sum, the code is either useless or ineffective in doing what
it tries to do.
(foreground-color . "Firebrick") will *never be encountered*, so there is no
reason to test for it. That is what (not (memq (car faceprop)
'(foreground-color background-color)))) does - uselessly.
(:foreground "Firebrick"...) will be encountered, but it is filtered out by
this test: (not (keywordp (car faceprop))).
((foreground-color . "Firebrick")...) will also be encountered, and it is
not filtered out by the keywordp test, but, as you say, this form will never
be taken for a list of faces. It gets excluded not by any explicit test, but
by a failed attempt to treat it as a list of faces (the dolist).
All I'm saying is that the test (not (memq (car faceprop) '(foreground-color
background-color))) is 100% useless - it corresponds to nothing.