GNU bug report logs -
#67246
30.0.50; elixir-ts-mode uses faces inconsistently
Previous Next
Full log
Message #35 received at 67246 <at> debbugs.gnu.org (full text, mbox):
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.
>
This bug report was last modified 1 year and 168 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.