Re: face-attribute and face-remapping-alist

[address@hidden]

#+begin_src emacs-lisp

  (setq term-current-face
              (list :foreground
                    (face-foreground
                     (elt ansi-term-color-vector term-ansi-current-color)
                     nil 'default)
                    :background
                    (face-background
                     (elt ansi-term-color-vector term-ansi-current-bg-color)
                     nil 'default)
                    :inverse-video term-ansi-current-reverse))

#+end_src

  And we already know that if an `ansi-term' buffer has its
  `term-color-*' faces have been customized through `face-remap-add-relative',
  `face-foreground' and `face-background' in above code won't
  return correct face information due to `face-attribute' is not
  aware of the `ansi-term' buffer's `face-remapping-alist' local variable.

  Therefore, to make `ansi-term' receive correct face information,
  we need to

  1. change 'face-attribute' to make it aware of `face-remapping-alist'
     in a way that is similar to my earlier proposal; or

  2. do not change any existing face-related functions but define new
     functions to retrieve buffer-local face settings such like

#+begin_src emacs-lisp

  (defun face-attribute-blfa (face attribute &optional frame inherit buffer)
    "Get face attribute with buffer-local faces aware"
    ;; retrieve buffer-local `face-remapping-alist', if it exists
    (or (let ((bl-fra (buffer-local-value
                       'face-remapping-alist
                       (or (and buffer (get-buffer buffer))
                           (current-buffer)))))
          (and bl-fra
               (plist-get (car (alist-get face bl-fra)) attribute)))
        (face-attribute face attribute frame inherit)))

  (defun face-foreground-blfa (face &optional frame inherit buffer)
    (face-attribute-blfa face :foreground frame inherit buffer))

  (defun face-background-blfa (face &optional frame inherit buffer)
    (face-attribute-blfa face :background frame inherit buffer))

  (setq term-current-face
        (list :foreground
              (face-foreground-blfa
               (elt ansi-term-color-vector term-ansi-current-color)
               nil 'default)
              :background
              (face-background-blfa
               (elt ansi-term-color-vector term-ansi-current-bg-color)
               nil 'default)
              :inverse-video term-ansi-current-reverse))

#+end_src

As mentioned in my previous mails, introducing new APIs (without changing 
existing
`face-attribute'-based functions) to make buffer-local faces work sufficiently 
well in
various modes do require library/package developers to adapt the new APIs and 
change,
potentially considerable, part of their existing code base to accommodate that.

So my question here is that which of the following options

1. make `face-attribute' return buffer-local face attributes if the face has 
been
   customized (via `face-remapping-alist) in that buffer otherwise return
   frame-local face attributes
2. Introducing new face attribute getting/setting APIs to Emacs code base;
   these new APIs  are `face-remapping-alist' aware and use unchanged
   `face-attribute' as the fallback dispatch option
3.  Do nothing to Emacs code base: let the library/package developer to decide
    whether buffer local face settings should be accommodated or not

is the most economical one?

I personally believe the option #1 is the most favorable one because, ideally,
`face-attribute' should always return face attributes those are consistent with
what have been rendered in any given buffer. Also, this option does not require
changing any existing code base of libraries/packages/modes those manipulate
faces (for example, `term.el'). However, I don't know how much risk of
potential breakage brought by option #1. Therefore, we may treat this option
as a convenient (only need to change `face-attribute') but also a dangerous one.

Option #2 is an acceptable choice to me although it require porting efforts
from the mode/library/package developers to adapt new APIs in Emacs,
but at least we have an official APIs that can be used by all Emacs 
library/package
developers without re-inventing wheels.

Option #3 is the least favorable option to me, as it requires developers to come
up with lots of solutions just to solve the common problem of retrieving
face information those are consistent with what have been rendered in any
given buffers. This wild west approach seems not helpful in solving the
problem.

Thanks.
Kiong-Gē.





‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, March 31, 2021 1:58 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Wed, 31 Mar 2021 03:05:42 +0000
>
> > From: "gliao.tw@pm.me" gliao.tw@pm.me
> > Cc: "emacs-devel@gnu.org" emacs-devel@gnu.org
> >
> > > From the above example, we can see that `face-attribute' will return 
> > > results *incosistent* with what we see in a buffer which has been updated 
> > > with a theme in buffer-local manner via functions implemented 
> > > in`face-remap.el'.
>
> Sure, but the only change needed in all the cases you described in
> order to return attributes that are aware of the remapping is to look
> up the face in face-remapping-alist, before calling face-attribute.
> This solution is so easy that I don't understand the need for changing
> the behavior of face-attribute in such fundamental and incompatible
> ways to produce the same effect.
>
> Am I missing something?