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` if
the 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 and
the attribute was `undefined` because maybe the `parent` face define
the filter parameter to `non-nil`.
So now `merge_named_face` calls `merge_face_vectors` if the parameter is
undefined too, and `merge_face_vectors` internally conditions if the
merge is needed. The problem is that the merge with the parent faces was
always made writing the TO vector (in place) as it was not conditional, but we
don't know in advance if the parent (or the grand-parents) define the
filter attribute to non-nil
So what I did in the `undefined` case was to make a copy of the TO
vector (tmp) and merge_ref there instead of for TO, after the merge I
check if the attribute is set, and if so, then I copy it into TO and
continue with the merge.
The two copies and the extra call to merge_ref (in some cases) probably are not the most
efficient solution, but it is the only way I found to do it right
without repeating the merge (that must be more expensive) or modifying
the TO vector. But the vector so far is not too big (only 20 elements up
to now) so I thing it is good enough. There are some small optimizations
to avoid the extra call to merge_ref I set in order to optimize a bit
when possible (you know my obsession about that); like return in advance when some parent sets the filter to nil and
so on.
-----Original Message-----
From: Eli Zaretskii <address@hidden>
To: Ergus <address@hidden>
Cc: emacs-devel <address@hidden>; ingo.lohmar <address@hidden>
Sent: Sat, Oct 26, 2019 9:29 am
Subject: Re: :extend t inheritance
> Date: Sat, 26 Oct 2019 03:49:13 +0200
> From: Ergus <
address@hidden>
> Cc: Ingo Lohmar <
address@hidden>,
address@hidden>
> 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.