bug#51469: [External] : Re: bug#51469: 27.2; Mismatch: `set-face-attribute' and `face-all-attributes'

[Drew Adams <drew.adams@oracle.com>]

> face-all-attributes could return such a plist, but we usually prefer to
> return alists from functions instead of plists.
> 
> > Is there a good reason for this status quo?  If not, can we change it
> > to get a better fit and not require conversion?
> 
> We can't change the output of face-all-attributes (because that would
> break code), so I don't think there's anything to do here, and I'm
> closing this bug report.

Do as you like (you already did), but FTR, I disagree.
___

We could certainly allow another optional arg that
specifies that the result be a plist.  All that's
lacking is the will (so far).

If you want a patch for that, and if you'll actually
use it, let me know.

The definition below is what the patch would do.  But I
have a comment and a question first.

A comment is that if you prefer to move the test for
PLIST that's inside the loop outside of it, let me know.

A question is why `face-all-attributes' excludes `:font'
from the list of attributes it returns.

More generally, shouldn't attribute `:font' be included
in the value of defconst `face-attribute-name-alist'?

If it should, then the code would be simpler, as would
use by users (no special value needed for arg PLIST, to
include `:font' for use by `set-face-attribute').

However, that's a "constant", and existing (3rd-party)
code might now depend on `:font' not being included.
So a 2nd part of the question is why we use `defconst'.

If Emacs later adds an attribute (as it did with
`:extend') then the "constant" is effectively broken.
`defconst' is supposed to mean that neither program nor
programmer is expected to change the value.
___

(defun face-all-attributes (face &optional frame plist)
  "Return an alist or plist of the attributes of FACE.
By default, return an alist, with elements of the form
\(ATTR-NAME . ATTR-VALUE).

By default, ATTR-VALUE is the value provided by default for
ATTR-NAME of FACE before it is defined with `defface', which
means it is typically the symbol `unspecified'.  But non-nil
FRAME means ATTR-VALUE is the value of ATTR-NAME for FACE as
it is defined for FRAME.

Non-nil PLIST means return a plist, not an alist:
\( ATTR-NAME-1 ATTR-VALUE-1 ATTR-NAME-2 ATTR-VALUE-2...).

If PLIST is `font-also' then include attribute `:font'.
In that case, the result includes all attributes valid as
arguments to function `set-face-attribute'."
  (let ((names   (if (eq plist 'font-also)
                     (cons (cons :font "font")
                           face-attribute-name-alist)
                   face-attribute-name-alist))
        (result  ())
        att val)
    (while names
      (setq att    (caar names)
            val    (face-attribute face att (or frame  t))
            names  (cdr names))
      (if (not plist)
          (push (cons att val) result)
        (push att result)
        (push val result)))
    (when plist (setq result  (nreverse result)))
    result))

Eli recently made some kind of improvement to the doc
of this function for bug #51465, to clarify what
"default" attribute values mean for this when FRAME
is nil.  Dunno just what change he made, but the doc
I show here can be changed to say whatever he said or
whatever you like.  Let me know the text you prefer,
before I send any patch.