bug#39812: 26.1; face-remapping-alist is sometimes set to an unexpected value

bug#39812: 26.1; face-remapping-alist is sometimes set to an unexpected value

[Markus Triska]

To reproduce this issue, please start Emacs with "emacs -Q", then place
the following forms in the appearing *scratch* buffer:

    (defun a ()
      (set (make-local-variable 'face-remapping-alist)
           '((default bold))))

    (a)
    (face-remap-add-relative 'default 'italic)
    (a)

and then evaluate the buffer with M-x eval-buffer RET.

Then, inspect the value of face-remapping-alist with:

    C-h v face-remapping-alist RET

In my case, the value is: '((default italic bold)).

How can this be explained? The final form (a) is supposed to set
face-remapping-alist to '((default bold)), and hence I expect the
value of face-remapping-alist to be '((default bold)) instead.

When I comment out the first invocation of "(a)", i.e., when I write:

    (defun a ()
      (set (make-local-variable 'face-remapping-alist)
           '((default bold))))

    ;; (a) ; now this line is commented out
    (face-remap-add-relative 'default 'italic)
    (a)

and then evaluate the buffer, then face-remapping-alist is set to
'((default bold)), which is the expected value.

As guideline for the definition of the function `a', I used the snippet
from the variable description of face-remapping-alist, which states:

   "For instance, the mode my-mode could define a
    face ‘my-mode-default’, and then in the mode setup function, do:

      (set (make-local-variable 'face-remapping-alist)
           '((default my-mode-default))))."

Is this still a recommended way to set this variable, should the above
example work? Is there a reliable way to set face-remapping-alist to
'((default bold)) in the function `a' so that the first example works?

Thank you and all the best!
Markus


In GNU Emacs 26.1 (build 1, x86_64-apple-darwin15.3.0, X toolkit, Xaw scroll bars)
 of 2018-09-22 built on mt-mb
Windowing system distributor 'The X.Org Foundation', version 11.0.11502000

Configured using:
 'configure --prefix=/opt/local --without-ns --without-dbus
 --without-gconf --without-libotf --without-m17n-flt --without-gpm
 --with-gnutls --with-xml2 --with-modules --infodir
 /opt/local/share/info/emacs --with-x-toolkit=lucid --without-xaw3d
 --without-imagemagick --with-xpm --with-jpeg --with-tiff --with-gif
 --with-png --with-lcms2 --without-rsvg --with-xft 'CFLAGS=-pipe -Os
 -arch x86_64' CPPFLAGS=-I/opt/local/include 'LDFLAGS=-L/opt/local/lib
 -Wl,-headerpad_max_install_names -lfreetype -lfontconfig -Wl,-no_pie
 -arch x86_64''

Configured features:
XPM JPEG TIFF GIF PNG GSETTINGS NOTIFY ACL GNUTLS LIBXML2 FREETYPE XFT
ZLIB TOOLKIT_SCROLL_BARS LUCID X11 MODULES THREADS LCMS2

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

bug#39812: 26.1; face-remapping-alist is sometimes set to an unexpected value

[Noam Postavsky]

Markus Triska <triska@metalevel.at> writes:

> To reproduce this issue, please start Emacs with "emacs -Q", then place
> the following forms in the appearing *scratch* buffer:
>
>     (defun a ()
>       (set (make-local-variable 'face-remapping-alist)
>            '((default bold))))
>
>     (a)
>     (face-remap-add-relative 'default 'italic)
>     (a)

face-remap-add-relative destructively modifies the list value, so
setting face-remapping-alist to a quoted literal gives unexpected
results like this.  Similar to the example at the bottom of (info
"(elisp) Rearrangement")

> Is this still a recommended way to set this variable, should the above
> example work? Is there a reliable way to set face-remapping-alist to
> '((default bold)) in the function `a' so that the first example works?

   (defun a ()
      (set (make-local-variable 'face-remapping-alist)
           (copy-tree '((default bold)))))

Or

   (defun a ()
      (set (make-local-variable 'face-remapping-alist)
           (list (list 'default 'bold))))

bug#39812: 26.1; face-remapping-alist is sometimes set to an unexpected value

[Markus Triska]

Noam Postavsky <npostavs@gmail.com> writes:

> face-remap-add-relative destructively modifies the list value, so
> setting face-remapping-alist to a quoted literal gives unexpected
> results like this.

Thank you for looking into this! Using copy-tree makes it work.

However, I find it very unexpected that face remapping modifies a value
that appears as a literal constant in my own code. Is it possible to
make face remapping word without such destructive side-effects?

Alternatively, would you please consider documenting how to work around
this, for example at the place where the sample snippet is mentioned?

Thank you and all the best!
Markus

bug#39812: 26.1; face-remapping-alist is sometimes set to an unexpected value

[Noam Postavsky]

Markus Triska <triska@metalevel.at> writes:

>> face-remap-add-relative destructively modifies the list value, so
>> setting face-remapping-alist to a quoted literal gives unexpected
>> results like this.
>
> Thank you for looking into this! Using copy-tree makes it work.
>
> However, I find it very unexpected that face remapping modifies a value
> that appears as a literal constant in my own code. Is it possible to
> make face remapping word without such destructive side-effects?

I guess it should be easy enough to add a copy-tree call in
face-remap-add-relative, but it looks like a couple of other functions
also modify the value destructively.

> Alternatively, would you please consider documenting how to work around
> this, for example at the place where the sample snippet is mentioned?

So updating the docs might be easier than trying to catch all the
potential trouble spots.

bug#39812: 26.1; face-remapping-alist is sometimes set to an unexpected value

[Noam Postavsky]

close 39812 27.1
quit

>> Alternatively, would you please consider documenting how to work around
>> this, for example at the place where the sample snippet is mentioned?

> So updating the docs might be easier than trying to catch all the
> potential trouble spots.

I've done that now on emacs-27.

[1: ee47e00f4e]: 2020-03-28 21:22:49 -0400
  Don't suggest setting face-remapping-alist to a literal (Bug#39812)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ee47e00f4e0a644a0948743ac43892710663b243>