To test this, do M-x debug-on-entry read-face-name,
             and then M-x
               customize-face font-lock-comment-face. Then, put point on the
               second "sample" text (next to the foreground field),
             and do M-:
               (read-face-name "foo"). Step through the debugger,
             and you'll see
               that the ((foreground-color . "Firebrick")) case falls
               through the crack.

        I believe that it is seeming to do the right thing, for the
        wrong reason.

    If the code reliably does the right thing, it isn't wrong.  Perhaps it
    could be clearer, or perhaps it could use a comment to explain this
    case.

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")...). If such an  old-style attribute list
can be confused with a list of faces, then it will not do the right thing -
it will treat the list as faces.

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.






_______________________________________________
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug

Reply via email to