Re: :extend t inheritance

[Ergus <spacibba@aol.com>]

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

 Ohh right, that's the other commit. When fixing this issue I foundanother error I didn't see in tui.After EOL we need to fill with the merged filtered face. To do that weneed to add a stretch glyph; but we were just adding a normal glyph witha ' ' so the face was not really extended, just the background colorbecause the X display engine always do that. But, for example, underlinewas not extended.This commit fixes just that. stretch_ascent was moved out of theif(column_indicator) because the new stretch_glyphs needs it any way andthe two comments were 1) for old moved code not there anymore and 2) todescribe the situation of the glyphs color extension (mention inprevious paragraph) that we don't need to work around anymore becausethe glyph fills the entire screen now.. 
 
-----Original Message-----
From: Eli Zaretskii <eliz@gnu.org>
To: Ergus <spacibba@aol.com>
Cc: ingo.lohmar <ingo.lohmar@posteo.net>; emacs-devel <emacs-devel@gnu.org>
Sent: Sat, Oct 26, 2019 9:47 pm
Subject: Re: :extend t inheritance

> Date: Sat, 26 Oct 2019 19:21:40 +0000 (UTC)
> From: Ergus <spacibba@aol.com>
> Cc: emacs-devel@gnu.org, ingo.lohmar@posteo.net
> 
> 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. 

I understood all that, but the changes include more than what you
described (and your original message even said that you fixed a couple
of other bugs as well).  Those other things is what I was asking
about.  Just do

  git diff ...origin/fix/inherit_extend_face

in the master branch, and you will see those other changes right away.
For example, I see 2 comments deleted, and something about
stretch_ascent(?).  What is this stuff, and what does it solve?

Thanks.


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