GNU bug report logs - #67246
30.0.50; elixir-ts-mode uses faces inconsistently

Previous Next

Full log

Message #74 received at 67246 <at> debbugs.gnu.org (full text, mbox):

From: Wilhelm Kirschbaum <wkirschbaum <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>, Stefan Kangas <stefankangas <at> gmail.com>
Cc: Andrey Listopadov <andreyorst <at> gmail.com>, 67246 <at> debbugs.gnu.org
Subject: Re: bug#67246: 30.0.50; elixir-ts-mode uses faces inconsistently
Date: Mon, 5 Feb 2024 19:34:16 +0200

On 2024/02/05 19:05, Wilhelm Kirschbaum wrote:
> On 2024/01/30 03:59, Dmitry Gutov wrote:
>
>> On 29/01/2024 06:08, Dmitry Gutov wrote:
>>> Hi!
>>>
>>> On 13/01/2024 10:50, Wilhelm Kirschbaum wrote:
>>>> + (access_call target: (identifier) @font-lock-variable-name-face) 
>>>> + (access_call "[" key: (identifier) @font-lock-variable-name-face 
>>>> "]"))
>>>
>>> This should use font-lock-variable-use-face. And all other "variable 
>>> reference" highlights should use it too.
>>>
>>> OTOH, the method parameters are still highlighted with 
>>> font-lock-variable-use-face, which should be 
>>> font-lock-variable-name-face.
>>>
>>> This happens inside the first 'elixir-variable' highlight. Perhaps 
>>> elixir-ts--definition-keywords-re could be used there to 
>>> disambiguate as well.
>>
>> See this combined patch:
>>
>> 1. Your additions from the last attachment (access target highlighting).
>> 2. All instances of font-lock-variable-name-face swapped for 
>> font-lock-variable-use-face (since most of those match variable 
>> references).
> Thanks, this makes sense.
>> 3. Added highlighting for method parameters with 
>> font-lock-variable-name-face.
> I had a look and think it covers most instances ( some should arguably 
> not be highlighted as use-face, but can be debated ).
>> 4. Feature elixir-function-name renamed to elixir-definition since it 
>> now touches both function and variable (parameter) definitions.
> Makes sense
>> 5. Feature elixir-variable moved to the feature level 4, since that's 
>> where it is in other built-in ts modes.
>>
> Agreed.
>> Any objections to it?
> Thanks for the effort and I have no objections.
>
> There are however some more issues I spotted on the function-name and 
> function-call matches which I will be looking into.

Adding this as the first item to :feature 'elixir-definition fixes the 
function-call/name issue:

@@ -360,13 +360,19 @@ elixir-ts--indent-rules
 (defvar elixir-ts--font-lock-settings
   (treesit-font-lock-rules
    :language 'elixir
-   :feature 'elixir-function-name
+   :feature 'elixir-definition
    `((call target: (identifier) @target-identifier
+           (arguments
+            (call target: (identifier) @font-lock-function-name-face
+                  (arguments)))
+           (:match ,elixir-ts--definition-keywords-re @target-identifier))
+     (call target: (identifier) @target-identifier
            (arguments (identifier) @font-lock-function-name-face)

I will be working in Elixir this week and will set different fonts for 
to test it during the week, but don't think it should hold up installing 
the suggested patch so long.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.