Re: [PATCH v2] Fix background color of latex previews

[Roshan Shariff]

Hi Timothy,

On Mon, 8 Jan 2024 at 03:24, Timothy <orgmode@tec.tecosaur.net> wrote:
> Thanks for this info and the patch! I've had a look at both, and I'm hopeful 
> that https://git.tecosaur.net/tec/org-mode/commit/3b3d48d3bf0 might solve the 
> problem? Do let me know what your think 🙂

Thanks for your prompt bugfix! I can confirm that it works beautifully
on both Emacs 28 and 29.

Looking at the code, I just had a small suggestion:

diff --git a/lisp/org-latex-preview.el b/lisp/org-latex-preview.el
index 144449e34..973eed47b 100644
--- a/lisp/org-latex-preview.el
+++ b/lisp/org-latex-preview.el
@@ -541,7 +541,7 @@ Faces in `org-latex-preview--ignored-faces' are ignored."
         (normalising-face
          (if (>= emacs-major-version 29) 'default '(:inherit default
:extend t))))
     (cond
-     ((consp face)
+     ((and (consp face) (not (keywordp (car face))) (listp (cdr face)))
       (nconc (cl-set-difference face
org-latex-preview--ignored-faces) (list normalising-face)))
      ((and face (not (memq face org-latex-preview--ignored-faces)))
       (list face normalising-face))

The (not (keywordp (car face))) condition ensures that the face isn't
a single anonymous face, i.e. a plist. The (listp (cdr face))
condition ensures that it's not just a cons like (foreground-color .
color). In both cases, the face spec is not a list of faces, so it
wouldn't be correct to append another face to the end. I'm not sure
how commonly this can happen in practice, but it covers all the cases
handled by merge_face_ref in xfaces.c.

Regards,
Roshan

> All the best,
> Timothy
>
> --
> Timothy (‘tecosaur’/‘TEC’), Org mode contributor.
> Learn more about Org mode at <https://orgmode.org/>.
> Support Org development at <https://liberapay.com/org-mode>,
> or support my work at <https://liberapay.com/tec>.