From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Ergus Newsgroups: gmane.emacs.devel Subject: Re: :extend t inheritance Date: Sat, 26 Oct 2019 19:21:40 +0000 (UTC) Message-ID: <357181555.902089.1572117701015@mail.yahoo.com> References: <87zhhrmcaq.fsf@kenko.localhost.com> <83o8y6xnmn.fsf@gnu.org> <20191026014911.7riwc5oq6epjhdiq@Ergus> <8336fgvuby.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_902088_1893349334.1572117701013" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="107216"; mail-complaints-to="usenet@blaine.gmane.org" Cc: emacs-devel@gnu.org, ingo.lohmar@posteo.net To: eliz@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Oct 26 21:22:00 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1iORdP-000RhI-R7 for ged-emacs-devel@m.gmane.org; Sat, 26 Oct 2019 21:22:00 +0200 Original-Received: from localhost ([::1]:41574 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iORdO-00043c-06 for ged-emacs-devel@m.gmane.org; Sat, 26 Oct 2019 15:21:58 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49610) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iORdE-0003lh-EJ for emacs-devel@gnu.org; Sat, 26 Oct 2019 15:21:49 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iORdB-0003m1-O0 for emacs-devel@gnu.org; Sat, 26 Oct 2019 15:21:47 -0400 Original-Received: from sonic315-14.consmr.mail.bf2.yahoo.com ([74.6.134.124]:38816) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iORdB-0003lL-3e for emacs-devel@gnu.org; Sat, 26 Oct 2019 15:21:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aol.com; s=a2048; t=1572117703; bh=6Qk2rQEOpK7rGSE0U7C7z/F1kDqr51/qHegZv49YqHE=; h=Date:From:To:Cc:In-Reply-To:References:Subject:From:Subject; b=ZszdbThH/6ncSCoGvQoAwZGI/HU70GS9v4foFBatrAs3BhFcZb0BzlDn8bk3+e81VXrErj1v4WPG7KX1361ASzePDZcfLpGY5/3gk3m29WYfUz8gjcPsXQIvTnhRmb9wBa6yYg2WV6R9fslxlypzXZu+Ec3jG/2URwDXwZT6xsXTUHMHjIySYMuCEk1ZdOZIQ0+CyJOh81Go0Eb03pHuwxhnSiYAWQnKfAbpKFr/Yp4qdUYrkAktYbLqVTyjUJ+Ga3/3cBTbfqBRJF7RRK5ZN/0e491xJ8eqqxK/RXsUvF5zMZqjzFfX5njbdel38nqeEPNzLpGskRi8Xm0jMj6iIA== X-YMail-OSG: 2AgmkwQVM1nVPQiA0XPbgxHX5rL7CFTt9zYLBjP.IhV1XHb_CVXFHy96yCgBf.Q qZP4CzlfUUem07NnzGrC_.k.MwJXZKh_6xfwbk.dKv.HmPEnF0es_iDsVSsqkj7Y9eKWVgZp5wEn mbGe0qKIDC2DJv.Y0dvBXTuIJMF_HFyWmFe.R4Tt5Dh7BKAJugaDNcPobninL329_9ZdpMMI9GBs COo8dvzx6A5v2ZkBLo3ng96gMaC43cyCy22VKG2YQKRA6ZyMQdWy_EcgcHYiLNRMHQFLBD1c6Jxz 279kqLdkHd61Q6fHgaXVG.lBoH5LP_AE2dv.noFbTnkjwxy0j0S7yY2H34HySsLnQvEOb5iiRUe9 EMdIgEtM36q6SInQqDYOY1BqqhOX_XZB2MFhO36TKfLGOA9hWnEc42Wn9DZdwOIYEbpiHx3KWR7X iKd_nfj9Aju.j22R189ZUQdtOcQm7PaXBpKCT2jIL0Djkqeq2MvTsRbHnwvKYDRpPCFDcd5iWSSr r5n0at9ecAFSkAYbM5LLcT_JlGGKoM4Kq1RzkCvfkCJi9wA3lzr2FlzKKQYcPaVW_3OzROE1uaIU BK5APo2r1JLavpcjiD_BcJj7aX6TmXpQv2oO5QFx2vy8RXHfS1sf7AdlnsRPblQKoVvgWeBMeTQA 9A4Wf5rPWBsoDEsok3ZwObOopicQP059pcy3fkVIWecw_ycNzczqoVzQU7Af3Og9BaRscAZ5Yt9L gemlt5F_kO6REzPljTOFXxIiOsfngHD_jv32ALu48AneOLFSJW38ECpAFG.sJMJIajygs6SFaJQM jzEnJOIjlS9Tjb._w0zl82WrLaix1ErV6Rr1Lpza8i Original-Received: from sonic.gate.mail.ne1.yahoo.com by sonic315.consmr.mail.bf2.yahoo.com with HTTP; Sat, 26 Oct 2019 19:21:43 +0000 In-Reply-To: <8336fgvuby.fsf@gnu.org> X-Mailer: WebService/1.1.14593 aolwebmail Mozilla/5.0 (X11; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 74.6.134.124 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:241475 Archived-At: ------=_Part_902088_1893349334.1572117701013 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Eli:=20 The change is actually simple but somehow unexpected for me (as I forgot th= e inheritance before).=20 Before `merge_named_face` filtered the call to `merge_face_vectors` ifthe f= ilter parameter (lets say :extend in our case) was `nil` or`undefined`. Tha= t was how the attr_filter worked.=20 But in this case it is actually wrong when the face was inherited andthe at= tribute was `undefined` because maybe the `parent` face definethe filter pa= rameter to `non-nil`.=20 So now `merge_named_face` calls `merge_face_vectors` if the parameter isund= efined too, and `merge_face_vectors` internally conditions if themerge is n= eeded. The problem is that the merge with the parent faces wasalways made w= riting 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=20 So what I did in the `undefined` case was to make a copy of the TOvector (t= mp) and merge_ref there instead of for TO, after the merge Icheck if the at= tribute is set, and if so, then I copy it into TO andcontinue with the merg= e.=20 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 ri= ghtwithout repeating the merge (that must be more expensive) or modifyingth= e TO vector. But the vector so far is not too big (only 20 elements upto no= w) 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.=20 =20 =20 =20 =20 -----Original Message----- From: Eli Zaretskii To: Ergus Cc: emacs-devel ; ingo.lohmar Sent: Sat, Oct 26, 2019 9:29 am Subject: Re: :extend t inheritance > Date: Sat, 26 Oct 2019 03:49:13 +0200 > From: Ergus > Cc: Ingo Lohmar , emacs-devel@gnu.org >=20 > 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. >=20 > Please give it a look and if everything is fine I'll move it to master > ASAP. I took a look.=C2=A0 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. ------=_Part_902088_1893349334.1572117701013 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit
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 <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.


------=_Part_902088_1893349334.1572117701013--