emacs-pretest-bug
[Top][All Lists]
Advanced

[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.





reply via email to

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