bug#81534: [PATCH] elisp-scope: Improve support for :inherit face specs

close 81534 32.1
quit

zach shaftel <[email protected]> writes:

> Eshel Yaron <[email protected]> writes:
>
>> Hi,
>>
>> zach shaftel <[email protected]> writes:
>>
>>> Tags: patch
>>>
>>> This patches elisp-scope--match-spec-to-arg to recursively process
>>> :inherit face specs, so now even complicated face specs like
>>> (:inherit (:inherit (bold (:inherit underline)))) are correctly parsed.
>>
>> Sounds good, although in (info "(elisp) Face Attributes") I see:
>>
>>   =E2=80=98:inherit=E2=80=99
>>        The name of a face from which to inherit attributes, or a list of
>>        face names. [...]
>>
>> This says that the value of :inherit is always expressed in face names,
>> not as a list of face attributes.  And I couldn't find an example of
>> such a nested attributes list.  Moreover, evaluating the following form
>> indeed yields an "Invalid face inheritance" error:
>>
>>   (defface f '((t :inherit (bold (:inherit success)))) "")
>>
>> So I think we should support a list of face names like (bold success),
>> but not nested attribute lists.  Makes sense?
>
> Ah I didn't realize that, makes sense. I had tested my example spec as a
> face text property and it does render correctly, but I don't think any
> sane person would (or should) write that. This patch changes all
> :inherit specs to (or (symbol . face) (repeat . (symbol . face))).
>
>>
>>> I noticed this after defface forms inheriting from a list of faces
>>> weren't being fontified the same as with a single inherited face name.
>>> I changed a couple uses of (symbol . face) output specs to just `face'
>>> so they utilize this.
>>
>> Nice.  Could you please also add a test or two?
>> See test/lisp/progmodes/elisp-mode-resources/semantic-highlighting.el
>
> Added in this patch.


Great, pushed to master as commit 602cde31636.
Thanks!


Eshel