bug#67246: 30.0.50; elixir-ts-mode uses faces inconsistently

[Dmitry Gutov]

On 24/11/2023 21:47, Wilhelm Kirschbaum wrote:

>     I see that the latest work
>     (https://github.com/wkirschbaum/elixir-ts-mode/pull/36
>     <https://github.com/wkirschbaum/elixir-ts-mode/pull/36>) anticipated at
>     least some of my comments.
>
>     Remainder:
>
>     1) font-lock-variable-name-face is intended for declarations and
>     perhaps
>     initial assignments (where it's also an implicit declaration), but for
>     other variable references it's better to use
>     font-lock-variable-use-face. With my test file, I see examples to the
>     contrary, even though some attempt to use the latter face had been made
>     (inside the 'elixir-function-call' feature's query).
>
>
> Thanks, this has been fixed.

Sorry, is there a specific commit in the upstream repo I could look at?

>     2) Moving highlighting of function calls and variable (and/or property)
>     references to the 4th feature level. Looking at your font-lock
>     rules, it
>     seems like the elixir-function-call matches is more targeted than the
>     elixir-variable one, but still the other built-in modes put both at 4,
>     so it would be good for uniformity. Function definitions (and variable
>     definitions/declarations, if they're highlighted separately) can remain
>     in the 'declaration' or 'definition' feature which is put in a lower
>     feature level (e.g. ruby/js/typescript modes have it on level 1).
>
>
> Level 4 is strange to me, because the default is 3.  I read the docs and 
> tried to
> follow it with the new changes, but was hesitant to remove much from the
> highlighting as not many people might discover there is a 4th level.  
> Then again, if there is a query it
> is pretty easy just to communicate this.

The idea was to balance the new look between the "old" major modes and 
the newer, shinier ones. So that the overall style still somewhat 
appeals to the existing audience, just with added features and more 
precision. Here's a Reddit recent thread about the same sentiment: 
https://www.reddit.com/r/emacs/comments/18152qo/overcolorization_everything_is_purple/ 
It discusses a post written by Andrey, BTW.

One could basically say that a function call and a properly lookup are 
easy to distinguish from glancing at the text, there's not much need to 
highlight them. As opposed to e.g. implicit variable declaration or 
function declaration.

And here's another aspect: the default built-in theme doesn't 
distinguish many of the faces (and the same is true for many other 
built-in themes). E.g. it doesn't distinguish variable-name-face from 
variable-use-face or function-name-face from function-call-face.

So if the -use- and -call- are used freely, in the default setup they 
will get muddled with the function and variable declaration.

OTOH, if the user installs a theme which has this more advanced support 
for the new faces (or customize the faces manually to be distinct), they 
might as well set treesit-font-lock-level to 4, that's very little extra 
effort.

>     Something else I would recommend:
>
>     3) Removing the '-face' suffix from the face names. This is the best
>     practice across most modes, and it's something the manual entry for
>     'defface' recommends:
>
>         You should not quote the symbol face, and it should not end in
>     ‘-face’ (that would be redundant).
>
>     Face names inside font-lock.el are a historical exception (and we
>     followed it when adding new faces recently), but if you search for
>     'defface' inside the Emacs codebase, such names are in the minority.
>
>
> Okay thanks, I had a look and it makes sense.  I see a new mode with the 
> same -face suffixes.
> The defface docs does not mention this, so might be a good idea to add 
> it there?

Probably. I rarely read the manual myself, and this is useful information.

> I am not entirely sure what "you should not quote the symbol face" means 
> wrt to the changes, because
> it does not look like I am quoting it.

Indeed you're not, I only put that in the quote so that the sentence is 
not cut off from the beginning. Sorry if that was confusing.

>     I haven't done too much testing myself. Perhaps Andrey will take the
>     upstream version for a spin. Or we'll wait for the changes to be merged
>     here and continue.