On Mon, Jan 17, 2022 at 04:47 PM Stefan Monnier wrote: >>> - In `face-remap.el`, refrain from modifying the >>> `face-remapping-alist` >>> by side-effects (i.e. avoid `delq`, `setcdr`, and friends). >>> - Add a `make-indirect-buffer-hook` and arrange for >>> `face-remap.el` to >>> add a function there that does a deep enough copy of >>> `face-remapping-alist`. >>> - Remember that indirect buffers are an attractive nuisance >>> and should >>> be deprecated (but note that I suspect the same bug affects >>> `clone-buffer` because it doesn't make a deep enough copy of >>> `face-remapping-alist` either). >> >> The last one tells me we are better with leaving this sleeping >> dog lie. > > Actually, looking back at the code, I see we already have the > needed hook (tho called from `clone-indirect-buffer`), so I > think the better option is to change `make-indirect-buffer` so > it runs `clone-indirect-buffer-hook` (when called with a non-nil > `clone` arg) instead of having `clone-indirect-buffer` run it. > And then in `face-remap.el` add a function to that hook which > copies `face-remapping-alist`. > > An intermediate option is to change the Org code to use > `clone-indirect-buffer` instead of `make-indirect-buffer` (but > that still requires the `face-remap.el` addition to the hook). > > [ Sorry Andrew for not noticing your earlier patch which does > something > similar to my first suggestion above. ] > > Andrew, would you be willing to write that patch? > > Stefan That sounds reasonable, I can do that. In fact, I just did this based on your suggestions, and it does work well. The only weird thing is that I had to pull `clone-indirect-buffer-hook' into the c code, because that's where `make-indirect-buffer' is. Let me know if that seems wrong. I've attached the patch, please let me know if there is an issue. I have commit access, so I can just commit it myself after your OK (if so, I'll wait for a week or so to see if Eli has a comment as well before checkin).