Re: :extend t inheritance

[Ergus <spacibba@aol.com>]

[-- Attachment #1: Type: text/plain, Size: 2606 bytes --]

 Hi Eli: 

The change is actually simple but somehow unexpected for me (as I forgot the inheritance before). 

Before `merge_named_face` filtered the call to `merge_face_vectors` ifthe filter parameter (lets say :extend in our case) was `nil` or`undefined`. That was how the attr_filter worked. 

But in this case it is actually wrong when the face was inherited andthe attribute was `undefined` because maybe the `parent` face definethe filter parameter to `non-nil`. 

So now `merge_named_face` calls `merge_face_vectors` if the parameter isundefined too, and `merge_face_vectors` internally conditions if themerge is needed. The problem is that the merge with the parent faces wasalways made writing the TO vector (in place) as it was not conditional, but wedon't know in advance if the parent (or the grand-parents) define thefilter attribute to non-nil 

So what I did in the `undefined` case was to make a copy of the TOvector (tmp) and merge_ref there instead of for TO, after the merge Icheck if the attribute is set, and if so, then I copy it into TO andcontinue with the merge. 

The two copies and the extra call to merge_ref (in some cases) probably are not the mostefficient solution, but it is the only way I found to do it rightwithout repeating the merge (that must be more expensive) or modifyingthe TO vector. But the vector so far is not too big (only 20 elements upto now) so I thing it is good enough. There are some small optimizationsto avoid the extra call to merge_ref I set in order to optimize a bitwhen possible (you know my obsession about that); like return in advance when some parent sets the filter to nil andso on. 



   
 
 
 
-----Original Message-----
From: Eli Zaretskii <eliz@gnu.org>
To: Ergus <spacibba@aol.com>
Cc: emacs-devel <emacs-devel@gnu.org>; ingo.lohmar <ingo.lohmar@posteo.net>
Sent: Sat, Oct 26, 2019 9:29 am
Subject: Re: :extend t inheritance

> Date: Sat, 26 Oct 2019 03:49:13 +0200
> From: Ergus <spacibba@aol.com>
> Cc: Ingo Lohmar <ingo.lohmar@posteo.net>, emacs-devel@gnu.org
> 
> I just added a branch `fix/inherit_extend_face` that fixes the issue
> with inheritance of the extend attribute with filter condition.

Thanks.

> There are two commits because I also detected another issue in gui I
> didn't see before because I don't use gui normally.
> 
> Please give it a look and if everything is fine I'll move it to master
> ASAP.

I took a look.  I don't understand some of the hunks, so please post
the rationale for the non-trivial changes there, otherwise it's hard
to review the changeset.


[-- Attachment #2: Type: text/html, Size: 4339 bytes --]