[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: :extend t inheritance

From: Ergus
Subject: Re: :extend t inheritance
Date: Sat, 26 Oct 2019 19:21:40 +0000 (UTC)

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.


> 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

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]