bug#64347: 30.0.50; Some customize faces shown as edited with -Q

[Mauro Aranda <maurooaranda@gmail.com>]

Eli Zaretskii <eliz@gnu.org> writes:

 >> Date: Sat, 8 Jul 2023 18:32:15 -0300
 >> Cc: 64347@debbugs.gnu.org, stephen.berman@gmx.net
 >> From: Mauro Aranda <maurooaranda@gmail.com>
 >>
 >> Since there are both ways to specify the same information for the
 >> :line-width value, there's always a chance of not sending the expected
 >> format to face-spec-match-p.
 >>
 >> Let's say we have (:line-width 1).  Then the pre-filter converts it into
 >> (:line-width (1 . 1)), and then the post-filter should convert it back
 >> to (:line-width 1), and that works.
 >>
 >> But if we have (:line-width (1 . 1)), the pre-filter does nothing, and
 >> the post-filter will transform it into (:line-width 1), and that way
 >> specs won't match.
 >>
 >> So sometimes we want to post-filter to (:line-width WIDTH) and sometimes
 >> to (VWIDTH . HWIDTH) and there's currently not enough information to do
 >> that, or at least I missed it.
 >>
 >>  > Did you make any progress with the one bug that's left after the 
other
 >>  > two are fixed?
 >>
 >> I did find that face-attr-match-p uses equal even with properties like
 >> :underline and :box that can have plists as values.  But I haven't tried
 >> to fix it, because I got frustrated with the post-filter thing.
 >
 > I guess I'm missing something here: why do we need those pre-filter
 > and post-filter conversions?  The C code understands both forms of
 > :line-width, so there should be no need for Lisp to do any
 > conversions, right?  So why do we do that? why not simply leave the
 > spec as it was originally?

Custom needs the pre-filter in order to present a Custom buffer to edit
the face.
Let's say there's a face:
(defface foo
   '((t (:box (:line-width 1 :color "black"))))
   "...")

And let's say a user wants to customize it via Custom.
M-x customize-face RET foo
should show the user a buffer with all the capabilities to edit it.
Because we have an integer for the :line-width property, the user will
be presented with an integer Widget to edit the value, without giving
the user the opportunity to edit the HWIDTH and VWIDTH separately.

So the pre-filter takes the (:line-width 1), and converts it into
(:line-width (1 . 1)), and now the Custom buffer will have
a cons Widget.  If we didn't do that conversion, that would be a Bug
report, I'm sure.

Of course, if the face is defined like so:
(defface foo
   '((t (:box (:line-width (1 . 1) :color "black"))))
   "...")
then the pre-filter doesn't have to do anything.  So, sometimes it does
something, sometimes not, but I hope I've convinced you about why is
needed.

Now, your question about the post-filter made me question its need a
little bit.  At first sight, I thought it was clear than a post-filter
was needed too.  As I said in my first message, Custom is fiddling with
the value for :line-width, but should present it to the face-spec-match-p
"unfiltered".  And currently, that's a need because when it comes to the
face-attr-match-p function, it will use equal to try to see if
attributes in two specs match.  So the post-filter should take
(:line-width (1 . 1)) and turn it back into (:line-width 1)

That's where I was originally, before your mail.  Now I think that maybe
face-attr-match-p could do something slightly smarter when comparing
these values, since, as you say, the C code understands both forms.
IOW, maybe the fix is that face-attr-match-p shouldn't produce a
mismatch between specs that have these both forms being compared.