* face-attribute and face-remapping-alist @ 2021-03-30 18:53 gliao.tw 2021-03-30 19:05 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: gliao.tw @ 2021-03-30 18:53 UTC (permalink / raw) To: emacs-devel@gnu.org The `face-attribute' function defined in `faces.el' is the foundation of many face property inquiry functions such as `face-background'. However, `face-attribute' is not aware of buffer-local variable 'face-remapping-alist' which is used for buffer-specific theming. Since there are many modes, such as `term-mode', rely on `face-attribute' to obtain face information while `face-attribute' is only capabale of obtaining frame-specific, rather than buffer-specific face infomation, these modes cannot display faces sufficiently well while a buffer-local theme is applied via `face-remap-add-relative' or `face-remap-set-base' (both functions, defined in `face-remap.el', add items to buffer-specific variable `face-remapping-alist'). Therefore, I propose the following prototype of an enhanced version of `face-attribute' function that is aware of the existence of `face-remapping-alist' in current buffer and return face property from `face-remapping-alist' instead of frame-specific (single frame also means global) face property. Any comments are greatly appreciated, thanks. Kiong-Gē. --------------------------- ;; (defun face-attribute (face attribute &optional frame inherit) "Return the value of FACE's ATTRIBUTE on FRAME or current buffer. If the optional argument FRAME is given, report on face FACE in that frame. If FRAME is t, report on the defaults for face FACE (for new frames). If FRAME is omitted or nil, use the selected frame. If INHERIT is nil, only attributes directly defined by FACE are considered, so the return value may be `unspecified', or a relative value. If INHERIT is non-nil, FACE's definition of ATTRIBUTE is merged with the faces specified by its `:inherit' attribute; however the return value may still be `unspecified' or relative. If INHERIT is a face or a list of faces, then the result is further merged with that face (or faces), until it becomes specified and absolute. To ensure that the return value is always specified and absolute, use a value of `default' for INHERIT; this will resolve any unspecified or relative values by merging with the `default' face (which is always completely specified)." ;; check if `face-remapping-alist' exist as a buffer-local variable ;; in current buffer (let* ((local-faces (if (local-variable-p 'face-remapping-alist (current-buffer)) ;; if so, take faces information from it (buffer-local-value 'face-remapping-alist (current-buffer)))) ;; try to fetch buffer-local face proprety from face-remapping-alist (local-face-prop (if local-faces (plist-get (car (alist-get face local-faces)) attribute)))) ;; if there is no inquired face informatio available in ;; face-remapping-alist, fallback to frame-specific values (or local-face-prop (let ((value (internal-get-lisp-face-attribute face attribute frame))) (when (and inherit (face-attribute-relative-p attribute value)) ;; VALUE is relative, so merge with inherited faces (let ((inh-from (face-attribute face :inherit frame))) (unless (or (null inh-from) (eq inh-from 'unspecified)) (condition-case nil (setq value (face-attribute-merged-with attribute value inh-from frame)) ;; The `inherit' attribute may point to non existent faces. (error nil))))) (when (and inherit (not (eq inherit t)) (face-attribute-relative-p attribute value)) ;; We should merge with INHERIT as well (setq value (face-attribute-merged-with attribute value inherit frame))) value)))) ---------------------------------------- ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-30 18:53 face-attribute and face-remapping-alist gliao.tw @ 2021-03-30 19:05 ` Eli Zaretskii 2021-03-30 19:13 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2021-03-30 19:05 UTC (permalink / raw) To: gliao.tw@pm.me; +Cc: emacs-devel > Date: Tue, 30 Mar 2021 18:53:28 +0000 > From: "gliao.tw@pm.me" <gliao.tw@pm.me> > > The `face-attribute' function defined in `faces.el' is the foundation of many face property inquiry functions such as `face-background'. However, `face-attribute' is not aware of buffer-local variable 'face-remapping-alist' which is used for buffer-specific theming. > > Since there are many modes, such as `term-mode', rely on `face-attribute' to obtain face information while `face-attribute' is only capabale of obtaining frame-specific, rather than buffer-specific face infomation, these modes cannot display faces sufficiently well while a buffer-local theme is applied via `face-remap-add-relative' or `face-remap-set-base' (both functions, defined in `face-remap.el', add items to buffer-specific variable `face-remapping-alist'). > > Therefore, I propose the following prototype of an enhanced version of `face-attribute' function that is aware of the existence of `face-remapping-alist' in current buffer and return face property from `face-remapping-alist' instead of frame-specific (single frame also means global) face property. Thanks, but unconditionally changing the behavior of face-attribute in such fundamental ways is a non-starter. There must be a way to still get the frame-specific face attributes, un-effected by buffer-specific remapping. face-attribute is quite a low-level API, and face remapping is a higher-level feature. So if we want face-attribute to pay attention to face remapping, that must be an optional behavior under control of some optional argument or a variable that could be let-bound. Alternatively, we could introduce a new API for what you want. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-30 19:05 ` Eli Zaretskii @ 2021-03-30 19:13 ` Eli Zaretskii 2021-03-31 3:05 ` gliao.tw 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2021-03-30 19:13 UTC (permalink / raw) To: gliao.tw; +Cc: emacs-devel > Date: Tue, 30 Mar 2021 22:05:00 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: emacs-devel@gnu.org > > Thanks, but unconditionally changing the behavior of face-attribute in > such fundamental ways is a non-starter. There must be a way to still > get the frame-specific face attributes, un-effected by buffer-specific > remapping. face-attribute is quite a low-level API, and face > remapping is a higher-level feature. > > So if we want face-attribute to pay attention to face remapping, that > must be an optional behavior under control of some optional argument > or a variable that could be let-bound. > > Alternatively, we could introduce a new API for what you want. And btw, I don't really understand the difficulty: fetching the remapped face from face-remapping-alist is trivial. Thereafter, face-attribute will do what you want. So I don't even think I understand the problem you are trying to solve. I guess a detailed description of some specific example would help here. In any case, please don't forget that frame-local and buffer-local values cannot be intermixed without producing undesirable effects. Face remapping was introduced to allow finer resolution in face attributes than the per-frame one, under the control of the application. Your proposal pushes the buffer-local aspects to the lower levels, and out of the application's control, which I don't think is TRT. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-30 19:13 ` Eli Zaretskii @ 2021-03-31 3:05 ` gliao.tw 2021-03-31 3:34 ` gliao.tw 2021-03-31 6:58 ` Eli Zaretskii 0 siblings, 2 replies; 10+ messages in thread From: gliao.tw @ 2021-03-31 3:05 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 7097 bytes --] Eli, thanks for the comments and questions. I would like to ues the following an example to expalint why I want to propose such a change in `face-attribute'. * The issue In the following emacs lisp transcript, I set `leuven' as the global theme and load but not yet enable `tango' theme, and then ask for its `default' face attributes under `leuven' globla them. The corresponding display are shown in attacthed "Pic_1_global_leuven_theme_applied.png". (load-theme 'leuven t nil) (load-theme 'tango t t) (face-attribute 'default :background) (face-attribute 'default :foreground) Next, I set the `ielm' buffer's default face using `wombat' theme's `default' face via `face-remap-add-relative' and inquire the 'default' face's foreground and background via `face-attribute` again using the following command: (face-attribute 'default :background) (face-attribute 'default :foreground) As we can see the results shown in the attached "Pic_2_local_wombat_theme_applied.png", although the backgound and foreground have been changed according to `wombat' theme's settings , but `face-attribute' function still returns the 'default face attribute of `leuven', instead of `wombat' theme's. 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'. Another aspect of this issue is that many modes rely on `face-attribute' to obtain appropriate face setting and it is known that `face-attribute' is unlikely to return correct results in buffer whose appearence is specificed by buffer-local `face-remapping-alist'. The following code are extracted from `term.el', which shows a heavy dependency on `face-attribute'-based `face-foreground` and `face-background' functions to extract face information. (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)) In the attacched "Pic_3_invalid_local_theme_rendering_in_ansi_term.png" screenshot, we can see that `ansi-term' from `term.el' is unable to render buffer local theme properly (the top) while eshell (the bottom) is able to render the buffer local color theme (relatively) properly. * Potential solutions/directions The following table summarizes some directions/solutions I can think of at the time being: |-------------------------+---------------+-----------------| | Approach | Benefit | Risk / Cost | |-------------------------+---------------+-----------------| | make `face-attribute' | No need | (Potentially) | | `face-remapping-alist'- | to change | mix frame-local | | aware | any exisitng | and buffer- | | | libraries/ | local settings | | | packages | | | | | | |-------------------------+---------------+-----------------| | New Emacs APIs that are | No changes | Require porting | | `face-remapping-alist'- | in existing | efforts from | | aware, potentially use | Emacs API | libraries / | | current | | packages | | `face-attribute' as the | | developers; | | fallback option | | marketing cost | | | | on the new APIs | | | | | |-------------------------+---------------+-----------------| | No change in Emacs at | Users pay | Current issues | | all: package/library | as they go: | remain; wild | | developers come up with | spend your | west of wheel | | their own solutions to | own resources | re-invention. | | deal with buffer-local | on features | | | face setting | you want. | | | | | | |-------------------------+---------------+-----------------| The above table just represents my current best, and definitely incomprehensive, thoughts on how can we make buffer-local face setting functions and face information functions work more consistently. The attached "Pic_4_buffer_local_aware_fa_ansi_term_and_eshell_consistent.png" shows a result using appoarch #1. The top `eshell' buffer and bottom `ansi-term' buffer can have very cosistent (although some difference in bold face), almost identical appearence with same buffer-local theme applied. Compare to "Pic_3_invalid_local_theme_rendering_in_ansi_term.png", we can see approach #1 does yield some favorable results (comes with the risk stated in above tabel). I hope above material can do some little help in clarifying my questions and thoughts on issues caused by current `face-remapping-alist'-unaware implementation of `face-attribute'. Any comments are greatly appreciated. Thanks, Kiong-Gē. ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Tuesday, March 30, 2021 2:13 PM, Eli Zaretskii <eliz@gnu.org> wrote: > > Date: Tue, 30 Mar 2021 22:05:00 +0300 > > > From: Eli Zaretskii eliz@gnu.org > > Cc: emacs-devel@gnu.org > > Thanks, but unconditionally changing the behavior of face-attribute in > > such fundamental ways is a non-starter. There must be a way to still > > get the frame-specific face attributes, un-effected by buffer-specific > > remapping. face-attribute is quite a low-level API, and face > > remapping is a higher-level feature. > > So if we want face-attribute to pay attention to face remapping, that > > must be an optional behavior under control of some optional argument > > or a variable that could be let-bound. > > Alternatively, we could introduce a new API for what you want. > > And btw, I don't really understand the difficulty: fetching the > remapped face from face-remapping-alist is trivial. Thereafter, > face-attribute will do what you want. So I don't even think I > understand the problem you are trying to solve. I guess a detailed > description of some specific example would help here. > > In any case, please don't forget that frame-local and buffer-local > values cannot be intermixed without producing undesirable effects. > Face remapping was introduced to allow finer resolution in face > attributes than the per-frame one, under the control of the > application. Your proposal pushes the buffer-local aspects to the > lower levels, and out of the application's control, which I don't > think is TRT. [-- Attachment #2: Pic_1_global_leuven_theme_applied.png --] [-- Type: image/png, Size: 49079 bytes --] [-- Attachment #3: Pic_4_buffer_local_aware_fa_ansi_term_and_eshell_consistent.png --] [-- Type: image/png, Size: 23160 bytes --] [-- Attachment #4: Pic_3_invalid_local_theme_rendering_in_ansi_term.png --] [-- Type: image/png, Size: 26768 bytes --] [-- Attachment #5: Pic_2_local_wombat_theme_applied.png --] [-- Type: image/png, Size: 73758 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-31 3:05 ` gliao.tw @ 2021-03-31 3:34 ` gliao.tw 2021-03-31 6:58 ` Eli Zaretskii 1 sibling, 0 replies; 10+ messages in thread From: gliao.tw @ 2021-03-31 3:34 UTC (permalink / raw) To: eliz; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 6625 bytes --] Sorry, in the first example, tango theme should be wombat theme. Sent from ProtonMail mobile -------- Original Message -------- On Mar 30, 2021, 22:05, gliao.tw@pm.me wrote: > Eli, > > thanks for the comments and questions. I would like to ues the following > an example to expalint why I want to propose such a change in `face-attribute'. > > * The issue > > In the following emacs lisp transcript, I set `leuven' as the global theme and > load but not yet enable `tango' theme, and then ask for its `default' > face attributes under `leuven' globla them. The corresponding display > are shown in attacthed "Pic_1_global_leuven_theme_applied.png". > > (load-theme 'leuven t nil) > > (load-theme 'tango t t) > > (face-attribute 'default :background) > > (face-attribute 'default :foreground) > > Next, I set the `ielm' buffer's default face using `wombat' theme's `default' > face via `face-remap-add-relative' and inquire the 'default' face's foreground > and background via `face-attribute` again using the following command: > > (face-attribute 'default :background) > > (face-attribute 'default :foreground) > > As we can see the results shown in the attached > "Pic_2_local_wombat_theme_applied.png", although the backgound and > foreground have been changed according to `wombat' theme's settings , > but `face-attribute' function still returns the 'default face attribute of > `leuven', instead of `wombat' theme's. > > 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'. > > Another aspect of this issue is that many modes rely on `face-attribute' to > obtain appropriate face setting and it is known that `face-attribute' is unlikely > to return correct results in buffer whose appearence is specificed by > buffer-local `face-remapping-alist'. > > The following code are extracted from `term.el', which shows a heavy > dependency on `face-attribute'-based `face-foreground` and > `face-background' functions to extract face information. > > (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)) > > In the attacched "Pic_3_invalid_local_theme_rendering_in_ansi_term.png" > screenshot, we can see that `ansi-term' from `term.el' is unable to render > buffer local theme properly (the top) while eshell (the bottom) is able to > render the buffer local color theme (relatively) properly. > > * Potential solutions/directions > > The following table summarizes some directions/solutions I can think of > at the time being: > > |-------------------------+---------------+-----------------| > | Approach | Benefit | Risk / Cost | > |-------------------------+---------------+-----------------| > | make `face-attribute' | No need | (Potentially) | > | `face-remapping-alist'- | to change | mix frame-local | > | aware | any exisitng | and buffer- | > | | libraries/ | local settings | > | | packages | | > | | | | > |-------------------------+---------------+-----------------| > | New Emacs APIs that are | No changes | Require porting | > | `face-remapping-alist'- | in existing | efforts from | > | aware, potentially use | Emacs API | libraries / | > | current | | packages | > | `face-attribute' as the | | developers; | > | fallback option | | marketing cost | > | | | on the new APIs | > | | | | > |-------------------------+---------------+-----------------| > | No change in Emacs at | Users pay | Current issues | > | all: package/library | as they go: | remain; wild | > | developers come up with | spend your | west of wheel | > | their own solutions to | own resources | re-invention. | > | deal with buffer-local | on features | | > | face setting | you want. | | > | | | | > |-------------------------+---------------+-----------------| > > The above table just represents my current best, and definitely > incomprehensive, thoughts on how can we make buffer-local face > setting functions and face information functions work more > consistently. > > The attached > "Pic_4_buffer_local_aware_fa_ansi_term_and_eshell_consistent.png" shows > a result using appoarch #1. The top `eshell' buffer and bottom `ansi-term' > buffer can have very cosistent (although some difference in bold face), > almost identical appearence with same buffer-local theme applied. Compare > to "Pic_3_invalid_local_theme_rendering_in_ansi_term.png", we can > see approach #1 does yield some favorable results (comes with the risk > stated in above tabel). > > I hope above material can do some little help in clarifying my questions > and thoughts on issues caused by current `face-remapping-alist'-unaware > implementation of `face-attribute'. > > Any comments are greatly appreciated. > > Thanks, > Kiong-Gē. > > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ > On Tuesday, March 30, 2021 2:13 PM, Eli Zaretskii <eliz@gnu.org> wrote: > >> > Date: Tue, 30 Mar 2021 22:05:00 +0300 >> >> > From: Eli Zaretskii eliz@gnu.org >> > Cc: emacs-devel@gnu.org >> > Thanks, but unconditionally changing the behavior of face-attribute in >> > such fundamental ways is a non-starter. There must be a way to still >> > get the frame-specific face attributes, un-effected by buffer-specific >> > remapping. face-attribute is quite a low-level API, and face >> > remapping is a higher-level feature. >> > So if we want face-attribute to pay attention to face remapping, that >> > must be an optional behavior under control of some optional argument >> > or a variable that could be let-bound. >> > Alternatively, we could introduce a new API for what you want. >> >> And btw, I don't really understand the difficulty: fetching the >> remapped face from face-remapping-alist is trivial. Thereafter, >> face-attribute will do what you want. So I don't even think I >> understand the problem you are trying to solve. I guess a detailed >> description of some specific example would help here. >> >> In any case, please don't forget that frame-local and buffer-local >> values cannot be intermixed without producing undesirable effects. >> Face remapping was introduced to allow finer resolution in face >> attributes than the per-frame one, under the control of the >> application. Your proposal pushes the buffer-local aspects to the >> lower levels, and out of the application's control, which I don't >> think is TRT. [-- Attachment #2: Type: text/html, Size: 7613 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-31 3:05 ` gliao.tw 2021-03-31 3:34 ` gliao.tw @ 2021-03-31 6:58 ` Eli Zaretskii 2021-03-31 13:00 ` Clément Pit-Claudel 2021-03-31 22:41 ` gliao.tw 1 sibling, 2 replies; 10+ messages in thread From: Eli Zaretskii @ 2021-03-31 6:58 UTC (permalink / raw) To: gliao.tw@pm.me; +Cc: emacs-devel > 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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-31 6:58 ` Eli Zaretskii @ 2021-03-31 13:00 ` Clément Pit-Claudel 2021-03-31 13:27 ` Eli Zaretskii 2021-03-31 22:41 ` gliao.tw 1 sibling, 1 reply; 10+ messages in thread From: Clément Pit-Claudel @ 2021-03-31 13:00 UTC (permalink / raw) To: emacs-devel On 3/31/21 2:58 AM, Eli Zaretskii wrote: > 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. You need to look it up recursively, which adds to the complexity. And it's more complicated than that if you use the "inherit" argument, since you need to check whether the default face is remapped as well, and merge that with the real default face. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-31 13:00 ` Clément Pit-Claudel @ 2021-03-31 13:27 ` Eli Zaretskii 0 siblings, 0 replies; 10+ messages in thread From: Eli Zaretskii @ 2021-03-31 13:27 UTC (permalink / raw) To: Clément Pit-Claudel; +Cc: emacs-devel > From: Clément Pit-Claudel <cpitclaudel@gmail.com> > Date: Wed, 31 Mar 2021 09:00:34 -0400 > > On 3/31/21 2:58 AM, Eli Zaretskii wrote: > > 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. > > You need to look it up recursively, which adds to the complexity. And it's more complicated than that if you use the "inherit" argument, since you need to check whether the default face is remapped as well, and merge that with the real default face. The issue described by the OP was of accessing the attribute's value, not merging face attributes. Merging faces is a separate issue. It is rare that an application should need to merge faces, as Emacs does that automatically. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-31 6:58 ` Eli Zaretskii 2021-03-31 13:00 ` Clément Pit-Claudel @ 2021-03-31 22:41 ` gliao.tw 2021-04-01 7:08 ` Eli Zaretskii 1 sibling, 1 reply; 10+ messages in thread From: gliao.tw @ 2021-03-31 22:41 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel@gnu.org #+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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: face-attribute and face-remapping-alist 2021-03-31 22:41 ` gliao.tw @ 2021-04-01 7:08 ` Eli Zaretskii 0 siblings, 0 replies; 10+ messages in thread From: Eli Zaretskii @ 2021-04-01 7:08 UTC (permalink / raw) To: gliao.tw@pm.me; +Cc: emacs-devel > Date: Wed, 31 Mar 2021 22:41:24 +0000 > From: "gliao.tw@pm.me" <gliao.tw@pm.me> > Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org> > > 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 3. do not change any existing face-related functions but look up the relevant face in face-remapping-alist before calling face-attribute. > 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? "Economical" in what sense? Changing long-standing default behavior of face-attribute is out of the question: we cannot make such changes in such old APIs. Item 3 is so easy to implement in those packages that really want to cater to buffer-local face customizations is so easy that I see no need for any change in core. Which then makes option 3 "the most economical" from my POV. And that's even before we discussed whether sensitivity to buffer-local face changes is at all a good idea in general, something that I'm not at all convinced in. Faces are more often than not related to major and minor modes, so having them change depending on buffer-local customizations would mean 2 buffers under the same mode will look differently. Why is that a good idea? ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-01 7:08 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-30 18:53 face-attribute and face-remapping-alist gliao.tw 2021-03-30 19:05 ` Eli Zaretskii 2021-03-30 19:13 ` Eli Zaretskii 2021-03-31 3:05 ` gliao.tw 2021-03-31 3:34 ` gliao.tw 2021-03-31 6:58 ` Eli Zaretskii 2021-03-31 13:00 ` Clément Pit-Claudel 2021-03-31 13:27 ` Eli Zaretskii 2021-03-31 22:41 ` gliao.tw 2021-04-01 7:08 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.