unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
@ 2023-02-20 15:54 Jacob Faibussowitsch
  2023-02-20 17:03 ` Eli Zaretskii
  2023-02-21  8:28 ` Yuan Fu
  0 siblings, 2 replies; 24+ messages in thread
From: Jacob Faibussowitsch @ 2023-02-20 15:54 UTC (permalink / raw)
  To: 61655

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

Hello,

Is it possible to have the builtin tree sitter give differentiate font-locking for function calls and function definitions?

The 3rd party tree-sitter package (https://github.com/emacs-tree-sitter/elisp-tree-sitter) has this feature and it is quite nice. In fact it goes further, allowing you to additionally distinguish between builtin calls, macro calls, method calls, etc. (see https://github.com/emacs-tree-sitter/elisp-tree-sitter/blob/master/lisp/tree-sitter-hl.el).

As far as I could see, the builtin mode only provides `font-lock-function-name-face`. I have set treesit-font-lock-level to 4. 

Examples below are for C/C++ mode, but this would apply to any number of languages.

Desired (i.e. what 3rd party package produces):



Current:



Best regards,

Jacob Faibussowitsch
(Jacob Fai - booss - oh - vitch)


[-- Attachment #2.1: Type: text/html, Size: 2101 bytes --]

[-- Attachment #2.2: PastedGraphic-4.png --]
[-- Type: image/png, Size: 19909 bytes --]

[-- Attachment #2.3: PastedGraphic-3.png --]
[-- Type: image/png, Size: 19934 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-20 15:54 bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately Jacob Faibussowitsch
@ 2023-02-20 17:03 ` Eli Zaretskii
  2023-02-20 20:24   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-21  8:28 ` Yuan Fu
  1 sibling, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-20 17:03 UTC (permalink / raw)
  To: Jacob Faibussowitsch, Yuan Fu, Theodor Thornhill; +Cc: 61655

> From: Jacob Faibussowitsch <jacob.fai@gmail.com>
> Date: Mon, 20 Feb 2023 10:54:42 -0500
> 
> Is it possible to have the builtin tree sitter give differentiate font-locking for function calls and function definitions?

AFAIU, it already does.  Yuan and Theo, am I wrong?





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-20 17:03 ` Eli Zaretskii
@ 2023-02-20 20:24   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-20 20:45     ` Jacob Faibussowitsch
  0 siblings, 1 reply; 24+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-20 20:24 UTC (permalink / raw)
  To: Eli Zaretskii, Jacob Faibussowitsch, Yuan Fu; +Cc: 61655

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jacob Faibussowitsch <jacob.fai@gmail.com>
>> Date: Mon, 20 Feb 2023 10:54:42 -0500
>> 
>> Is it possible to have the builtin tree sitter give differentiate font-locking for function calls and function definitions?
>
> AFAIU, it already does.  Yuan and Theo, am I wrong?

You're not wrong, but it depends on each implementation whether or not
the definition gets a different color or not.  Jacob, do you have any
examples where it is wrong and what you expect instead?

Theo





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-20 20:24   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-20 20:45     ` Jacob Faibussowitsch
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Faibussowitsch @ 2023-02-20 20:45 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: 61655, Eli Zaretskii, Yuan Fu

Hello All,

> Jacob, do you have any examples where it is wrong and what you expect instead?

I had attached some screen grabs in my initial email, if they don’t show up on your end they do show up on the web archives: https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-02/msg01638.html.

The key differences are:

1. `foo()` (i.e. a function definition) is light-blue.
2. `bar()`, and `baz()` (function calls) are dark blue.
3. The member function call `b.mem_func()` highlights the `mem_func()` part in red.

On the other hand, the builtin tree sitter mode font-locks them all using the same light-blue color. Indeed performing M-x describe-face over each separate part yields only `font-lock-function-name-face` for builtin, while 3rd party returns separate faces (which are available to customize) for each case.

I looked through `c-ts-mode.el` as an example and found `c-ts-mode--font-lock-settings`. Treesitter already identifies all of the above cases as different nodes in the AST (as evidenced in treesit-explor-mode) so it seems that adding another set of matchers (and appropriate faces, e.g. font-lock-function-call-face)

:language mode
:feature 'label
'((call_expression
   function: (identifier) @font-lock-function-call-face))

Should do the trick no?

Best regards,

Jacob Faibussowitsch
(Jacob Fai - booss - oh - vitch)

> On Feb 20, 2023, at 15:24, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>>> From: Jacob Faibussowitsch <jacob.fai@gmail.com>
>>> Date: Mon, 20 Feb 2023 10:54:42 -0500
>>> 
>>> Is it possible to have the builtin tree sitter give differentiate font-locking for function calls and function definitions?
>> 
>> AFAIU, it already does.  Yuan and Theo, am I wrong?
> 
> You're not wrong, but it depends on each implementation whether or not
> the definition gets a different color or not.  Jacob, do you have any
> examples where it is wrong and what you expect instead?
> 
> Theo






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function  calls, definitions, separately
  2023-02-20 15:54 bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately Jacob Faibussowitsch
  2023-02-20 17:03 ` Eli Zaretskii
@ 2023-02-21  8:28 ` Yuan Fu
  2023-02-21  9:55   ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Yuan Fu @ 2023-02-21  8:28 UTC (permalink / raw)
  To: jacob.fai; +Cc: 61655


Jacob Faibussowitsch <jacob.fai@gmail.com> writes:

> Hello,
>
> Is it possible to have the builtin tree sitter give differentiate font-locking for function calls and function definitions?
>
> The 3rd party tree-sitter package (https://github.com/emacs-tree-sitter/elisp-tree-sitter) has this feature and it is quite
> nice. In fact it goes further, allowing you to additionally distinguish between builtin calls, macro calls, method calls, etc.
> (see https://github.com/emacs-tree-sitter/elisp-tree-sitter/blob/master/lisp/tree-sitter-hl.el).
>
> As far as I could see, the builtin mode only provides `font-lock-function-name-face`. I have set treesit-font-lock-level to
> 4. 
>
> Examples below are for C/C++ mode, but this would apply to any number of languages.
>
> Desired (i.e. what 3rd party package produces):
>
> *
>
> Current:
>
> *

Hmmm, yeah. The builtin tree-sitter maps syntax queries directly into
faces, where the third-party tree-sitter maps syntax queries to some
syntax types, then maps types to faces. So it would be a bit harder to
do fine-grained control like in the builtin tree-sitter, comparing to
the third-party one.

I’ve thought of this idea before but didn’t pursue it further: Right now
we allow capture names to be face names and functions, eg

(commment) @font-lock-comment-face

or

(comment) @xxx-moode-fortify-comment

Maybe we can add a third type, arbitrary symbols, like

(comment) @comment

and add a variables treesit-font-lock-mapping which maps symbols to
faces or functions:

((comment . font-lock-comment-face))

or

((comment . xxx-mode-fontify-comment))

Then we can easily support differentiating between function call and
function definition.

Yuan





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-21  8:28 ` Yuan Fu
@ 2023-02-21  9:55   ` Dmitry Gutov
  2023-02-21 15:31     ` Jacob Faibussowitsch
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-21  9:55 UTC (permalink / raw)
  To: Yuan Fu, jacob.fai; +Cc: 61655

On 21/02/2023 10:28, Yuan Fu wrote:
> Hmmm, yeah. The builtin tree-sitter maps syntax queries directly into
> faces, where the third-party tree-sitter maps syntax queries to some
> syntax types, then maps types to faces. So it would be a bit harder to
> do fine-grained control like in the builtin tree-sitter, comparing to
> the third-party one.
> 
> I’ve thought of this idea before but didn’t pursue it further: Right now
> we allow capture names to be face names and functions, eg
> 
> (commment) @font-lock-comment-face
> 
> or
> 
> (comment) @xxx-moode-fortify-comment
> 
> Maybe we can add a third type, arbitrary symbols, like
> 
> (comment) @comment
> 
> and add a variables treesit-font-lock-mapping which maps symbols to
> faces or functions:
> 
> ((comment . font-lock-comment-face))
> 
> or
> 
> ((comment . xxx-mode-fontify-comment))
> 
> Then we can easily support differentiating between function call and
> function definition.

Before we do any of that, don't we need actual different faces to use 
for e.g. function definitions and function calls?

Same for variables.

And if we have those, we might not need the indirection, at least not 
right away.

I figured we'd add them in Emacs 30, but it's a relatively small change, 
if people are interested.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-21  9:55   ` Dmitry Gutov
@ 2023-02-21 15:31     ` Jacob Faibussowitsch
  2023-02-21 23:24       ` Dmitry Gutov
  2023-02-22 20:45       ` Yuan Fu
  0 siblings, 2 replies; 24+ messages in thread
From: Jacob Faibussowitsch @ 2023-02-21 15:31 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Yuan Fu, 61655

> but it's a relatively small change,

Indeed, after playing around with the syntax queries a bit more, all that is needed to implement this change is the following patch.

Note for the new faces that I populated the `:foreground` fields to match the colors for the example I showed in my first email, but maybe a better default is to leave these faces totally blank and just purely `:inherit` from `font-lock-function-name-face`.

diff --git a/lisp/font-lock.el b/lisp/font-lock.el
index 9e944fe188a..d170123c2ca 100644
--- a/lisp/font-lock.el
+++ b/lisp/font-lock.el
@@ -2026,6 +2026,16 @@ font-lock-function-name-face
   "Font Lock mode face used to highlight function names."
   :group 'font-lock-faces)
 
+(defface font-lock-function-call-face
+  '((t :inherit font-lock-function-name-face :foreground "royalblue1"))
+  "Font Lock mode face used to highlight function calls."
+  :group 'font-lock-faces)
+
+(defface font-lock-member-function-call-face
+  '((t :inherit font-lock-function-name-face :foreground "brightred"))
+  "Font Lock mode face used to highlight member function calls."
+  :group 'font-lock-faces)
+
 (defface font-lock-variable-name-face
   '((((class grayscale) (background light))
      :foreground "Gray90" :weight bold :slant italic)
diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index b7a487687a8..51a65aa6545 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -529,8 +529,8 @@ c-ts-mode--font-lock-settings
    :feature 'function
    '((call_expression
       function:
-      [(identifier) @font-lock-function-name-face
-       (field_expression field: (field_identifier) @font-lock-function-name-face)]))
+      [(identifier) @font-lock-function-call-face
+       (field_expression field: (field_identifier) @font-lock-member-function-call-face)]))
 
    :language mode
    :feature 'variable

Best regards,

Jacob Faibussowitsch
(Jacob Fai - booss - oh - vitch)

> On Feb 21, 2023, at 04:55, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 21/02/2023 10:28, Yuan Fu wrote:
>> Hmmm, yeah. The builtin tree-sitter maps syntax queries directly into
>> faces, where the third-party tree-sitter maps syntax queries to some
>> syntax types, then maps types to faces. So it would be a bit harder to
>> do fine-grained control like in the builtin tree-sitter, comparing to
>> the third-party one.
>> I’ve thought of this idea before but didn’t pursue it further: Right now
>> we allow capture names to be face names and functions, eg
>> (commment) @font-lock-comment-face
>> or
>> (comment) @xxx-moode-fortify-comment
>> Maybe we can add a third type, arbitrary symbols, like
>> (comment) @comment
>> and add a variables treesit-font-lock-mapping which maps symbols to
>> faces or functions:
>> ((comment . font-lock-comment-face))
>> or
>> ((comment . xxx-mode-fontify-comment))
>> Then we can easily support differentiating between function call and
>> function definition.
> 
> Before we do any of that, don't we need actual different faces to use for e.g. function definitions and function calls?
> 
> Same for variables.
> 
> And if we have those, we might not need the indirection, at least not right away.
> 
> I figured we'd add them in Emacs 30, but it's a relatively small change, if people are interested.






^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-21 15:31     ` Jacob Faibussowitsch
@ 2023-02-21 23:24       ` Dmitry Gutov
  2023-02-22 18:07         ` Jacob Faibussowitsch
  2023-02-22 20:45       ` Yuan Fu
  1 sibling, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-21 23:24 UTC (permalink / raw)
  To: Jacob Faibussowitsch; +Cc: Randy Taylor, Yuan Fu, 61655

On 21/02/2023 17:31, Jacob Faibussowitsch wrote:
 > but maybe a better default is to leave these faces totally blank and
 > just purely `:inherit` from `font-lock-function-name-face`

I believe so.

> +(defface font-lock-function-call-face
> +  '((t :inherit font-lock-function-name-face :foreground "royalblue1"))
> +  "Font Lock mode face used to highlight function calls."
> +  :group 'font-lock-faces)

This one I was thinking of as well.

> +(defface font-lock-member-function-call-face
> +  '((t :inherit font-lock-function-name-face :foreground "brightred"))
> +  "Font Lock mode face used to highlight member function calls."
> +  :group 'font-lock-faces)

What's a "member function"? Is it like a method? If people want this 
distinction, we can add such face. But I'm curious whether some other 
editors use different colors for these cases.

I'm also wondering what face we're supposed to use for "receiver-less" 
method calls, such as calls to the methods defined in the same class, in 
e.g. Ruby and Java. Or C++/C#. They don't use 'this'.

I think more importantly, we need a new face for variables.

font-lock-variable-ref-face ?

I also wonder whether we'll need to separate faces for properties: 
definitions vs. uses. That one we could use to do early, to keep the 
names uniform, e.g. we'd have:

   font-lock-function-name-face
   font-lock-function-call-face
   font-lock-variable-name-face
   font-lock-variable-ref-face
   font-lock-property-name-face
   font-lock-property-ref-face





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-21 23:24       ` Dmitry Gutov
@ 2023-02-22 18:07         ` Jacob Faibussowitsch
  2023-02-22 21:39           ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Jacob Faibussowitsch @ 2023-02-22 18:07 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Randy Taylor, Yuan Fu, 61655


> What's a "member function"? Is it like a method?

Yeah, I suppose method is the more general term for it.

> If people want this distinction, we can add such face. But I'm curious whether some other editors use different colors for these cases.

I personally do not; I brought it up because it was functionality that the 3rd party package had over stock. I think a good representative list is the one vscode offers:

https://code.visualstudio.com/api/language-extensions/semantic-highlight-guide#standard-token-types-and-modifiers

> I'm also wondering what face we're supposed to use for "receiver-less" method calls, such as calls to the methods defined in the same class, in e.g. Ruby and Java. Or C++/C#. They don't use ‘this’.

I don’t think emacs should worry about differentiating these cases. Highlight those tokens as tree-sitter sees them; regular function calls (i.e. `font-lock-function-call-face`). It’s a problem you cannot accurately solve without playing the compiler — with all of the implementation and not to mention performance baggage that comes with it.

> I also wonder whether we'll need to separate faces for properties: definitions vs. uses. That one we could use to do early, to keep the names uniform, e.g. we'd have:

This is something (also 3rd party package) lsp-mode supports through their semantic highlighting. They further distinguish between read and writes to variables. But again, they are able to do so because they hook directly into compilers.

Best regards,

Jacob Faibussowitsch
(Jacob Fai - booss - oh - vitch)

> On Feb 21, 2023, at 18:24, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 21/02/2023 17:31, Jacob Faibussowitsch wrote:
> > but maybe a better default is to leave these faces totally blank and
> > just purely `:inherit` from `font-lock-function-name-face`
> 
> I believe so.
> 
>> +(defface font-lock-function-call-face
>> +  '((t :inherit font-lock-function-name-face :foreground "royalblue1"))
>> +  "Font Lock mode face used to highlight function calls."
>> +  :group 'font-lock-faces)
> 
> This one I was thinking of as well.
> 
>> +(defface font-lock-member-function-call-face
>> +  '((t :inherit font-lock-function-name-face :foreground "brightred"))
>> +  "Font Lock mode face used to highlight member function calls."
>> +  :group 'font-lock-faces)
> 
> What's a "member function"? Is it like a method? If people want this distinction, we can add such face. But I'm curious whether some other editors use different colors for these cases.
> 
> I'm also wondering what face we're supposed to use for "receiver-less" method calls, such as calls to the methods defined in the same class, in e.g. Ruby and Java. Or C++/C#. They don't use 'this'.
> 
> I think more importantly, we need a new face for variables.
> 
> font-lock-variable-ref-face ?
> 
> I also wonder whether we'll need to separate faces for properties: definitions vs. uses. That one we could use to do early, to keep the names uniform, e.g. we'd have:
> 
>  font-lock-function-name-face
>  font-lock-function-call-face
>  font-lock-variable-name-face
>  font-lock-variable-ref-face
>  font-lock-property-name-face
>  font-lock-property-ref-face






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-21 15:31     ` Jacob Faibussowitsch
  2023-02-21 23:24       ` Dmitry Gutov
@ 2023-02-22 20:45       ` Yuan Fu
  2023-02-22 21:38         ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Yuan Fu @ 2023-02-22 20:45 UTC (permalink / raw)
  To: Jacob Faibussowitsch; +Cc: 61655, Dmitry Gutov



> On Feb 21, 2023, at 7:31 AM, Jacob Faibussowitsch <jacob.fai@gmail.com> wrote:
> 
>> but it's a relatively small change,
> 
> Indeed, after playing around with the syntax queries a bit more, all that is needed to implement this change is the following patch.
> 
> Note for the new faces that I populated the `:foreground` fields to match the colors for the example I showed in my first email, but maybe a better default is to leave these faces totally blank and just purely `:inherit` from `font-lock-function-name-face`.
> 
> diff --git a/lisp/font-lock.el b/lisp/font-lock.el
> index 9e944fe188a..d170123c2ca 100644
> --- a/lisp/font-lock.el
> +++ b/lisp/font-lock.el
> @@ -2026,6 +2026,16 @@ font-lock-function-name-face
>   "Font Lock mode face used to highlight function names."
>   :group 'font-lock-faces)
> 
> +(defface font-lock-function-call-face
> +  '((t :inherit font-lock-function-name-face :foreground "royalblue1"))
> +  "Font Lock mode face used to highlight function calls."
> +  :group 'font-lock-faces)
> +
> +(defface font-lock-member-function-call-face
> +  '((t :inherit font-lock-function-name-face :foreground "brightred"))
> +  "Font Lock mode face used to highlight member function calls."
> +  :group 'font-lock-faces)
> +
> (defface font-lock-variable-name-face
>   '((((class grayscale) (background light))
>      :foreground "Gray90" :weight bold :slant italic)
> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
> index b7a487687a8..51a65aa6545 100644
> --- a/lisp/progmodes/c-ts-mode.el
> +++ b/lisp/progmodes/c-ts-mode.el
> @@ -529,8 +529,8 @@ c-ts-mode--font-lock-settings
>    :feature 'function
>    '((call_expression
>       function:
> -      [(identifier) @font-lock-function-name-face
> -       (field_expression field: (field_identifier) @font-lock-function-name-face)]))
> +      [(identifier) @font-lock-function-call-face
> +       (field_expression field: (field_identifier) @font-lock-member-function-call-face)]))
> 
>    :language mode
>    :feature 'variable
> 
> Best regards,
> 
> Jacob Faibussowitsch
> (Jacob Fai - booss - oh - vitch)
> 
>> On Feb 21, 2023, at 04:55, Dmitry Gutov <dgutov@yandex.ru> wrote:
>> 
>> On 21/02/2023 10:28, Yuan Fu wrote:
>>> Hmmm, yeah. The builtin tree-sitter maps syntax queries directly into
>>> faces, where the third-party tree-sitter maps syntax queries to some
>>> syntax types, then maps types to faces. So it would be a bit harder to
>>> do fine-grained control like in the builtin tree-sitter, comparing to
>>> the third-party one.
>>> I’ve thought of this idea before but didn’t pursue it further: Right now
>>> we allow capture names to be face names and functions, eg
>>> (commment) @font-lock-comment-face
>>> or
>>> (comment) @xxx-moode-fortify-comment
>>> Maybe we can add a third type, arbitrary symbols, like
>>> (comment) @comment
>>> and add a variables treesit-font-lock-mapping which maps symbols to
>>> faces or functions:
>>> ((comment . font-lock-comment-face))
>>> or
>>> ((comment . xxx-mode-fontify-comment))
>>> Then we can easily support differentiating between function call and
>>> function definition.
>> 
>> Before we do any of that, don't we need actual different faces to use for e.g. function definitions and function calls?
>> 
>> Same for variables.
>> 
>> And if we have those, we might not need the indirection, at least not right away.
>> 
>> I figured we'd add them in Emacs 30, but it's a relatively small change, if people are interested.
> 

Yeah that’s just an idea, and I don’t have problem adding faces. But we probably can’t keep adding more and more specific faces. At one point we’ll need to either add indirection, or ask users to just add their own fontification rules, if it is really specific. We’ll see. 

Function definition & call is totally reasonable. But adapting all the major modes to use them is might be too big a change for emacs-29.

Yuan






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-22 20:45       ` Yuan Fu
@ 2023-02-22 21:38         ` Dmitry Gutov
  2023-02-23 18:15           ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-22 21:38 UTC (permalink / raw)
  To: Yuan Fu, Jacob Faibussowitsch; +Cc: 61655

On 22/02/2023 22:45, Yuan Fu wrote:
> Yeah that’s just an idea, and I don’t have problem adding faces. But we probably can’t keep adding more and more specific faces. At one point we’ll need to either add indirection, or ask users to just add their own fontification rules, if it is really specific. We’ll see.

An indirection seems like a separate new feature. Might be useful for 
some, but probably unnecessary for this discussion.

> Function definition & call is totally reasonable. But adapting all the major modes to use them is might be too big a change for emacs-29.

The change itself should be very straightforward. If we agree on the set 
of faces (for variables and properties as well, right?), I don't mind 
posting a patch for review.

Whether it gets accepted or not.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-22 18:07         ` Jacob Faibussowitsch
@ 2023-02-22 21:39           ` Dmitry Gutov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-22 21:39 UTC (permalink / raw)
  To: Jacob Faibussowitsch; +Cc: Randy Taylor, Yuan Fu, 61655

On 22/02/2023 20:07, Jacob Faibussowitsch wrote:
>> What's a "member function"? Is it like a method?
> Yeah, I suppose method is the more general term for it.
> 
>> If people want this distinction, we can add such face. But I'm curious whether some other editors use different colors for these cases.
> I personally do not; I brought it up because it was functionality that the 3rd party package had over stock. I think a good representative list is the one vscode offers:
> 
> https://code.visualstudio.com/api/language-extensions/semantic-highlight-guide#standard-token-types-and-modifiers
> 
>> I'm also wondering what face we're supposed to use for "receiver-less" method calls, such as calls to the methods defined in the same class, in e.g. Ruby and Java. Or C++/C#. They don't use ‘this’.
> I don’t think emacs should worry about differentiating these cases. Highlight those tokens as tree-sitter sees them; regular function calls (i.e. `font-lock-function-call-face`). It’s a problem you cannot accurately solve without playing the compiler — with all of the implementation and not to mention performance baggage that comes with it.
> 
>> I also wonder whether we'll need to separate faces for properties: definitions vs. uses. That one we could use to do early, to keep the names uniform, e.g. we'd have:
> This is something (also 3rd party package) lsp-mode supports through their semantic highlighting. They further distinguish between read and writes to variables. But again, they are able to do so because they hook directly into compilers.

Thank you, then I guess we can skip that distinction, at least for now.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-22 21:38         ` Dmitry Gutov
@ 2023-02-23 18:15           ` Eli Zaretskii
  2023-02-24  2:31             ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-23 18:15 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: casouri, 61655, jacob.fai

> Cc: 61655@debbugs.gnu.org
> Date: Wed, 22 Feb 2023 23:38:04 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 22/02/2023 22:45, Yuan Fu wrote:
> > Yeah that’s just an idea, and I don’t have problem adding faces. But we probably can’t keep adding more and more specific faces. At one point we’ll need to either add indirection, or ask users to just add their own fontification rules, if it is really specific. We’ll see.
> 
> An indirection seems like a separate new feature. Might be useful for 
> some, but probably unnecessary for this discussion.
> 
> > Function definition & call is totally reasonable. But adapting all the major modes to use them is might be too big a change for emacs-29.
> 
> The change itself should be very straightforward. If we agree on the set 
> of faces (for variables and properties as well, right?), I don't mind 
> posting a patch for review.
> 
> Whether it gets accepted or not.

I'm okay with adding a few more faces to emacs-29, but please hurry,
as we don't have too much time for more additions.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-23 18:15           ` Eli Zaretskii
@ 2023-02-24  2:31             ` Dmitry Gutov
  2023-02-24  7:56               ` Eli Zaretskii
  2023-02-25  1:06               ` Randy Taylor
  0 siblings, 2 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-24  2:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Randy Taylor, casouri, Theodor Thornhill, 61655, jacob.fai

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

On 23/02/2023 20:15, Eli Zaretskii wrote:
>> Cc:61655@debbugs.gnu.org
>> Date: Wed, 22 Feb 2023 23:38:04 +0200
>> From: Dmitry Gutov<dgutov@yandex.ru>
>>
>> On 22/02/2023 22:45, Yuan Fu wrote:
>>> Yeah that’s just an idea, and I don’t have problem adding faces. But we probably can’t keep adding more and more specific faces. At one point we’ll need to either add indirection, or ask users to just add their own fontification rules, if it is really specific. We’ll see.
>> An indirection seems like a separate new feature. Might be useful for
>> some, but probably unnecessary for this discussion.
>>
>>> Function definition & call is totally reasonable. But adapting all the major modes to use them is might be too big a change for emacs-29.
>> The change itself should be very straightforward. If we agree on the set
>> of faces (for variables and properties as well, right?), I don't mind
>> posting a patch for review.
>>
>> Whether it gets accepted or not.
> I'm okay with adding a few more faces to emacs-29, but please hurry,
> as we don't have too much time for more additions.

Here's the patch which adds the faces and their uses in all ts modes.

Comments welcome from all the interested parties. The patch is mostly 
straightforward, but there are some changes added as well, where it was 
needed to differentiate between declarations and references.

The important question here, I think, is whether we want to split 
font-lock-property-faces in two, like I did here.

By analogy with the other faces, I think it's going to be useful to 
differentiate between property definitions and property references. Not 
many of the languages modes used font-lock-property-face for 
property/attribute definitions, but some did.

[-- Attachment #2: font-lock-more-finer-faces.diff --]
[-- Type: text/x-patch, Size: 30277 bytes --]

diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index 052a10e3797..4c40f414ca0 100644
--- a/doc/lispref/modes.texi
+++ b/doc/lispref/modes.texi
@@ -3679,10 +3679,20 @@ Faces for Font Lock
 @vindex font-lock-function-name-face
 for the name of a function being defined or declared.
 
+@item font-lock-function-call-face
+@vindex font-lock-function-call-face
+for the name of a function being called.  This face inherits, by
+default, from @code{font-lock-function-name-face}.
+
 @item font-lock-variable-name-face
 @vindex font-lock-variable-name-face
 for the name of a variable being defined or declared.
 
+@item font-lock-variable-ref-face
+@vindex font-lock-variable-ref-face
+for the name of a variable being referenced.  This face inherits, by
+default, from @code{font-lock-variable-name-face}.
+
 @item font-lock-keyword-face
 @vindex font-lock-keyword-face
 for a keyword with special syntactic significance, like @samp{for} and
@@ -3756,11 +3766,16 @@ Faces for Font Lock
 @vindex font-lock-operator-face
 for operators.
 
-@item font-lock-property-face
-@vindex font-lock-property-face
-for properties of an object, such as the declaration and use of fields
-in a struct.
-This face inherits, by default, from @code{font-lock-variable-name-face}.
+@item font-lock-property-name-face
+@vindex font-lock-property-name-face
+for properties of an object, such as the declaration of fields in a
+struct.  This face inherits, by default, from
+@code{font-lock-variable-name-face}.
+
+@item font-lock-property-ref-face
+@vindex font-lock-property-ref-face
+for properties of an object, such as use of fields in a struct.  This
+face inherits, by default, from @code{font-lock-property-name-face}.
 
 For example,
 
diff --git a/etc/NEWS b/etc/NEWS
index 48f743fc9da..59371d6b4c1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -828,11 +828,12 @@ filter/sentinel error has been handled.
 +++
 ** New faces for font-lock.
 These faces are primarily meant for use with tree-sitter.  They are:
+'font-lock-function-call-face', ''font-lock-variable-ref-face',
 'font-lock-bracket-face', 'font-lock-delimiter-face',
 'font-lock-escape-face', 'font-lock-misc-punctuation-face',
 'font-lock-number-face', 'font-lock-operator-face',
-'font-lock-property-face', and 'font-lock-punctuation-face',
-'font-lock-regexp-face'.
+'font-lock-property-name-face', 'font-lock-property-ref-face', and
+'font-lock-punctuation-face', 'font-lock-regexp-face'.
 
 +++
 ** New face 'variable-pitch-text'.
diff --git a/lisp/cus-theme.el b/lisp/cus-theme.el
index dea5dbe9410..46e41dd046c 100644
--- a/lisp/cus-theme.el
+++ b/lisp/cus-theme.el
@@ -68,13 +68,16 @@ custom-theme--listed-faces
   font-lock-comment-delimiter-face font-lock-comment-face
   font-lock-constant-face font-lock-delimiter-face
   font-lock-doc-face font-lock-doc-markup-face
-  font-lock-escape-face font-lock-function-name-face
+  font-lock-escape-face font-lock-function-call-face
+  font-lock-function-name-face
   font-lock-keyword-face font-lock-negation-char-face
   font-lock-number-face font-lock-misc-punctuation-face
   font-lock-operator-face font-lock-preprocessor-face
-  font-lock-property-face font-lock-punctuation-face
+  font-lock-property-name-face font-lock-property-ref-face
+  font-lock-punctuation-face
   font-lock-regexp-grouping-backslash font-lock-regexp-grouping-construct
   font-lock-string-face font-lock-type-face font-lock-variable-name-face
+  font-lock-variable-ref-face
   font-lock-warning-face button link link-visited fringe
   header-line tooltip mode-line mode-line-buffer-id
   mode-line-emphasis mode-line-highlight mode-line-inactive
diff --git a/lisp/font-lock.el b/lisp/font-lock.el
index 9e944fe188a..b82b7648797 100644
--- a/lisp/font-lock.el
+++ b/lisp/font-lock.el
@@ -2026,6 +2026,12 @@ font-lock-function-name-face
   "Font Lock mode face used to highlight function names."
   :group 'font-lock-faces)
 
+(defface font-lock-function-call-face
+  '((t :inherit font-lock-function-name-face))
+  "Font Lock mode face used to highlight function calls."
+  :group 'font-lock-faces
+  :version "29.1")
+
 (defface font-lock-variable-name-face
   '((((class grayscale) (background light))
      :foreground "Gray90" :weight bold :slant italic)
@@ -2040,6 +2046,12 @@ font-lock-variable-name-face
   "Font Lock mode face used to highlight variable names."
   :group 'font-lock-faces)
 
+(defface font-lock-variable-ref-face
+  '((t :inherit font-lock-variable-name-face))
+  "Font Lock mode face used to highlight variable references."
+  :group 'font-lock-faces
+  :version "29.1")
+
 (defface font-lock-type-face
   '((((class grayscale) (background light)) :foreground "Gray90" :weight bold)
     (((class grayscale) (background dark))  :foreground "DimGray" :weight bold)
@@ -2115,10 +2127,17 @@ font-lock-operator-face
   :group 'font-lock-faces
   :version "29.1")
 
-(defface font-lock-property-face
+(defface font-lock-property-name-face
   '((t :inherit font-lock-variable-name-face))
   "Font Lock mode face used to highlight properties of an object.
-For example, the declaration and use of fields in a struct."
+For example, the declaration of fields in a struct."
+  :group 'font-lock-faces
+  :version "29.1")
+
+(defface font-lock-property-ref-face
+  '((t :inherit font-lock-property-name-face))
+  "Font Lock mode face used to highlight property references.
+For example, property lookup of fields in a struct."
   :group 'font-lock-faces
   :version "29.1")
 
diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 6d70dc3198e..90f1688e0ab 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -506,7 +506,10 @@ c-ts-mode--font-lock-settings
       declarator: (_) @c-ts-mode--fontify-declarator)
 
      (parameter_declaration
-      declarator: (_) @c-ts-mode--fontify-declarator))
+      declarator: (_) @c-ts-mode--fontify-declarator)
+
+     (enumerator
+      name: (identifier) @font-lock-property-name-face))
 
    :language mode
    :feature 'assignment
@@ -516,7 +519,7 @@ c-ts-mode--font-lock-settings
    '((assignment_expression
       left: (identifier) @font-lock-variable-name-face)
      (assignment_expression
-      left: (field_expression field: (_) @font-lock-property-face))
+      left: (field_expression field: (_) @font-lock-property-ref-face))
      (assignment_expression
       left: (pointer_expression
              (identifier) @font-lock-variable-name-face))
@@ -529,8 +532,8 @@ c-ts-mode--font-lock-settings
    :feature 'function
    '((call_expression
       function:
-      [(identifier) @font-lock-function-name-face
-       (field_expression field: (field_identifier) @font-lock-function-name-face)]))
+      [(identifier) @font-lock-function-call-face
+       (field_expression field: (field_identifier) @font-lock-function-call-face)]))
 
    :language mode
    :feature 'variable
@@ -552,9 +555,7 @@ c-ts-mode--font-lock-settings
 
    :language mode
    :feature 'property
-   '((field_identifier) @font-lock-property-face
-     (enumerator
-      name: (identifier) @font-lock-property-face))
+   '((field_identifier) @font-lock-property-ref-face)
 
    :language mode
    :feature 'bracket
@@ -614,6 +615,7 @@ c-ts-mode--fontify-declarator
          (face (pcase (treesit-node-type (treesit-node-parent
                                           (or qualified-root
                                               identifier)))
+                 ("field_declaration" 'font-lock-property-name-face)
                  ("function_declarator" 'font-lock-function-name-face)
                  (_ 'font-lock-variable-name-face))))
     (when identifier
@@ -630,7 +632,7 @@ c-ts-mode--fontify-variable
                     "call_expression"))
     (treesit-fontify-with-override
      (treesit-node-start node) (treesit-node-end node)
-     'font-lock-variable-name-face override start end)))
+     'font-lock-variable-ref-face override start end)))
 
 (defun c-ts-mode--fontify-defun (node override start end &rest _)
   "Correctly fontify the DEFUN macro.
diff --git a/lisp/progmodes/cmake-ts-mode.el b/lisp/progmodes/cmake-ts-mode.el
index 04f5d6bdac8..a3f9279ec1c 100644
--- a/lisp/progmodes/cmake-ts-mode.el
+++ b/lisp/progmodes/cmake-ts-mode.el
@@ -125,7 +125,7 @@ cmake-ts-mode--font-lock-settings
 
    :language 'cmake
    :feature 'function
-   '((normal_command (identifier) @font-lock-function-name-face))
+   '((normal_command (identifier) @font-lock-function-call-face))
 
    :language 'cmake
    :feature 'keyword
@@ -154,7 +154,7 @@ cmake-ts-mode--font-lock-settings
    :language 'cmake
    :feature 'variable
    :override t
-   '((variable) @font-lock-variable-name-face)
+   '((variable) @font-lock-variable-ref-face)
 
    :language 'cmake
    :feature 'error
diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index 293a910081c..2b49b49dc62 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -739,8 +739,8 @@ csharp-ts-mode--font-lock-settings
    :language 'c-sharp
    :override t
    :feature 'property
-   `((attribute (identifier) @font-lock-property-face (attribute_argument_list))
-     (attribute (identifier) @font-lock-property-face))
+   `((attribute (identifier) @font-lock-property-ref-face (attribute_argument_list))
+     (attribute (identifier) @font-lock-property-ref-face))
 
    :language 'c-sharp
    :override t
diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index 7802c1fbfcc..d4bafdb577b 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -133,7 +133,7 @@ go-ts-mode--font-lock-settings
      (method_spec
       name: (field_identifier) @font-lock-function-name-face)
      (field_declaration
-      name: (field_identifier) @font-lock-property-face)
+      name: (field_identifier) @font-lock-property-name-face)
      (parameter_declaration
       name: (identifier) @font-lock-variable-name-face)
      (short_var_declaration
@@ -146,10 +146,10 @@ go-ts-mode--font-lock-settings
    :language 'go
    :feature 'function
    '((call_expression
-      function: (identifier) @font-lock-function-name-face)
+      function: (identifier) @font-lock-function-call-face)
      (call_expression
       function: (selector_expression
-                 field: (field_identifier) @font-lock-function-name-face)))
+                 field: (field_identifier) @font-lock-function-call-face)))
 
    :language 'go
    :feature 'keyword
@@ -177,12 +177,12 @@ go-ts-mode--font-lock-settings
 
    :language 'go
    :feature 'property
-   '((field_identifier) @font-lock-property-face
-     (keyed_element (_ (identifier) @font-lock-property-face)))
+   '((selector_expression field: (field_identifier) @font-lock-property-ref-face)
+     (keyed_element (_ (identifier) @font-lock-property-ref-face)))
 
    :language 'go
    :feature 'variable
-   '((identifier) @font-lock-variable-name-face)
+   '((identifier) @font-lock-variable-ref-face)
 
    :language 'go
    :feature 'escape-sequence
diff --git a/lisp/progmodes/java-ts-mode.el b/lisp/progmodes/java-ts-mode.el
index 6948ebba631..a1f3ad692c2 100644
--- a/lisp/progmodes/java-ts-mode.el
+++ b/lisp/progmodes/java-ts-mode.el
@@ -220,7 +220,7 @@ java-ts-mode--font-lock-settings
 
      (method_reference (identifier) @font-lock-type-face)
 
-     (scoped_identifier (identifier) @font-lock-variable-name-face)
+     (scoped_identifier (identifier) @font-lock-constant-face)
 
      ((scoped_identifier name: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
@@ -244,7 +244,7 @@ java-ts-mode--font-lock-settings
       name: (identifier) @font-lock-variable-name-face)
 
      (element_value_pair
-      key: (identifier) @font-lock-property-face)
+      key: (identifier) @font-lock-property-ref-face)
 
      (formal_parameter
       name: (identifier) @font-lock-variable-name-face)
@@ -255,14 +255,14 @@ java-ts-mode--font-lock-settings
    :override t
    :feature 'expression
    '((method_invocation
-      object: (identifier) @font-lock-variable-name-face)
+      object: (identifier) @font-lock-variable-ref-face)
 
      (method_invocation
-      name: (identifier) @font-lock-function-name-face)
+      name: (identifier) @font-lock-function-call-face)
 
      (argument_list (identifier) @font-lock-variable-name-face)
 
-     (expression_statement (identifier) @font-lock-variable-name-face))
+     (expression_statement (identifier) @font-lock-variable-ref-face))
 
    :language 'java
    :feature 'bracket
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 027d6053f6e..e53a80bd499 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3544,11 +3544,10 @@ js--treesit-font-lock-settings
       value: [(function) (arrow_function)])
 
      (variable_declarator
-      name: (array_pattern
-             (identifier)
-             (identifier)
-             @font-lock-function-name-face)
-      value: (array (number) (function)))
+      name: [(array_pattern (identifier) @font-lock-variable-name-face)
+             (object_pattern
+              (shorthand_property_identifier_pattern) @font-lock-variable-name-face)])
+
      ;; full module imports
      (import_clause (identifier) @font-lock-variable-name-face)
      ;; named imports with aliasing
@@ -3564,15 +3563,13 @@ js--treesit-font-lock-settings
 
    :language 'javascript
    :feature 'property
-   '(((property_identifier) @font-lock-property-face
+   '(((property_identifier) @font-lock-property-ref-face
       (:pred js--treesit-property-not-function-p
-             @font-lock-property-face))
-
-     (pair value: (identifier) @font-lock-variable-name-face)
+             @font-lock-property-ref-face))
 
-     ((shorthand_property_identifier) @font-lock-property-face)
+     (pair value: (identifier) @font-lock-variable-ref-face)
 
-     ((shorthand_property_identifier_pattern) @font-lock-property-face))
+     ((shorthand_property_identifier) @font-lock-property-ref-face))
 
    :language 'javascript
    :feature 'assignment
@@ -3582,14 +3579,14 @@ js--treesit-font-lock-settings
    :language 'javascript
    :feature 'function
    '((call_expression
-      function: [(identifier) @font-lock-function-name-face
+      function: [(identifier) @font-lock-function-call-face
                  (member_expression
                   property:
-                  (property_identifier) @font-lock-function-name-face)])
+                  (property_identifier) @font-lock-function-call-face)])
      (method_definition
       name: (property_identifier) @font-lock-function-name-face)
      (function_declaration
-      name: (identifier) @font-lock-function-name-face)
+      name: (identifier) @font-lock-function-call-face)
      (function
       name: (identifier) @font-lock-function-name-face))
 
@@ -3597,15 +3594,15 @@ js--treesit-font-lock-settings
    :feature 'jsx
    '((jsx_opening_element
       [(nested_identifier (identifier)) (identifier)]
-      @font-lock-function-name-face)
+      @font-lock-function-call-face)
 
      (jsx_closing_element
       [(nested_identifier (identifier)) (identifier)]
-      @font-lock-function-name-face)
+      @font-lock-function-call-face)
 
      (jsx_self_closing_element
       [(nested_identifier (identifier)) (identifier)]
-      @font-lock-function-name-face)
+      @font-lock-function-call-face)
 
      (jsx_attribute
       (property_identifier)
@@ -3684,8 +3681,8 @@ js--treesit-fontify-assignment-lhs
     (treesit-fontify-with-override
      (treesit-node-start node) (treesit-node-end node)
      (pcase (treesit-node-type node)
-       ("identifier" 'font-lock-variable-name-face)
-       ("property_identifier" 'font-lock-property-face))
+       ("identifier" 'font-lock-variable-ref-face)
+       ("property_identifier" 'font-lock-property-ref-face))
      override start end)))
 
 (defun js--treesit-defun-name (node)
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 0d714c31e9e..0e4757be65b 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1106,24 +1106,25 @@ python--treesit-settings
    :language 'python
    '((interpolation) @python--treesit-fontify-string-interpolation)
 
+   :feature 'keyword
+   :language 'python
+   `([,@python--treesit-keywords] @font-lock-keyword-face
+     ((identifier) @font-lock-keyword-face
+      (:match "^self$" @font-lock-keyword-face)))
+
    :feature 'definition
    :language 'python
    '((function_definition
       name: (identifier) @font-lock-function-name-face)
      (class_definition
-      name: (identifier) @font-lock-type-face))
+      name: (identifier) @font-lock-type-face)
+     (parameters (identifier) @font-lock-variable-name-face))
 
    :feature 'function
    :language 'python
-   '((call function: (identifier) @font-lock-function-name-face)
+   '((call function: (identifier) @font-lock-function-call-face)
      (call function: (attribute
-                      attribute: (identifier) @font-lock-function-name-face)))
-
-   :feature 'keyword
-   :language 'python
-   `([,@python--treesit-keywords] @font-lock-keyword-face
-     ((identifier) @font-lock-keyword-face
-      (:match "^self$" @font-lock-keyword-face)))
+                      attribute: (identifier) @font-lock-function-call-face)))
 
    :feature 'builtin
    :language 'python
@@ -1146,7 +1147,7 @@ python--treesit-settings
                  @font-lock-variable-name-face)
      (assignment left: (attribute
                         attribute: (identifier)
-                        @font-lock-property-face))
+                        @font-lock-property-ref-face))
      (pattern_list (identifier)
                    @font-lock-variable-name-face)
      (tuple_pattern (identifier)
@@ -1183,12 +1184,12 @@ python--treesit-settings
    :feature 'property
    :language 'python
    '((attribute
-      attribute: (identifier) @font-lock-property-face)
+      attribute: (identifier) @font-lock-property-ref-face)
      (class_definition
       body: (block
              (expression_statement
               (assignment left:
-                          (identifier) @font-lock-property-face)))))
+                          (identifier) @font-lock-property-ref-face)))))
 
    :feature 'operator
    :language 'python
@@ -1211,10 +1212,10 @@ python--treesit-variable-p
   "Check whether NODE is a variable.
 NODE's type should be \"identifier\"."
   ;; An identifier can be a function/class name, a property, or a
-  ;; variables.  This function filters out function/class names and
-  ;; properties.
+  ;; variables.  This function filters out function/class names,
+  ;; properties and method parameters.
   (pcase (treesit-node-type (treesit-node-parent node))
-    ((or "function_definition" "class_definition") nil)
+    ((or "function_definition" "class_definition" "parameters") nil)
     ("attribute"
      (pcase (treesit-node-field-name node)
        ("object" t)
diff --git a/lisp/progmodes/ruby-ts-mode.el b/lisp/progmodes/ruby-ts-mode.el
index 01b0cd784a3..fedba200f83 100644
--- a/lisp/progmodes/ruby-ts-mode.el
+++ b/lisp/progmodes/ruby-ts-mode.el
@@ -292,11 +292,11 @@ ruby-ts--font-lock-settings
 
    :language language
    :feature 'global
-   '((global_variable) @font-lock-variable-name-face)
+   '((global_variable) @font-lock-variable-ref-face)
 
    :language language
    :feature 'instance
-   '((instance_variable) @font-lock-variable-name-face)
+   '((instance_variable) @font-lock-variable-ref-face)
 
    :language language
    :feature 'method-definition
@@ -350,7 +350,7 @@ ruby-ts--font-lock-settings
    :language language
    :feature 'function
    '((call
-      method: (identifier) @font-lock-function-name-face))
+      method: (identifier) @font-lock-function-call-face))
 
    :language language
    :feature 'assignment
diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index ee73ebf7a9f..a46d442a0e5 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -157,7 +157,7 @@ rust-ts-mode--font-lock-settings
    '((function_item name: (identifier) @font-lock-function-name-face)
      (macro_definition "macro_rules!" @font-lock-constant-face)
      (macro_definition (identifier) @font-lock-preprocessor-face)
-     (field_declaration name: (field_identifier) @font-lock-property-face)
+     (field_declaration name: (field_identifier) @font-lock-property-name-face)
      (parameter pattern: (_) @rust-ts-mode--fontify-pattern)
      (closure_parameters (_) @rust-ts-mode--fontify-pattern)
      (let_declaration pattern: (_) @rust-ts-mode--fontify-pattern)
@@ -174,17 +174,17 @@ rust-ts-mode--font-lock-settings
    :feature 'function
    '((call_expression
       function:
-      [(identifier) @font-lock-function-name-face
+      [(identifier) @font-lock-function-call-face
        (field_expression
-        field: (field_identifier) @font-lock-function-name-face)
+        field: (field_identifier) @font-lock-function-call-face)
        (scoped_identifier
-        name: (identifier) @font-lock-function-name-face)])
+        name: (identifier) @font-lock-function-call-face)])
      (generic_function
-      function: [(identifier) @font-lock-function-name-face
+      function: [(identifier) @font-lock-function-call-face
                  (field_expression
-                  field: (field_identifier) @font-lock-function-name-face)
+                  field: (field_identifier) @font-lock-function-call-face)
                  (scoped_identifier
-                  name: (identifier) @font-lock-function-name-face)])
+                  name: (identifier) @font-lock-function-call-face)])
      (macro_invocation macro: (identifier) @font-lock-preprocessor-face))
 
    :language 'rust
@@ -239,8 +239,8 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'property
-   '((field_identifier) @font-lock-property-face
-     (shorthand_field_initializer (identifier) @font-lock-property-face))
+   '((field_identifier) @font-lock-property-ref-face
+     (shorthand_field_initializer (identifier) @font-lock-property-ref-face))
 
    ;; Must be under type, otherwise some imports can be highlighted as constants.
    :language 'rust
@@ -251,25 +251,25 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'variable
-   '((arguments (identifier) @font-lock-variable-name-face)
-     (array_expression (identifier) @font-lock-variable-name-face)
-     (assignment_expression right: (identifier) @font-lock-variable-name-face)
-     (binary_expression left: (identifier) @font-lock-variable-name-face)
-     (binary_expression right: (identifier) @font-lock-variable-name-face)
-     (block (identifier) @font-lock-variable-name-face)
-     (compound_assignment_expr right: (identifier) @font-lock-variable-name-face)
-     (field_expression value: (identifier) @font-lock-variable-name-face)
-     (field_initializer value: (identifier) @font-lock-variable-name-face)
-     (if_expression condition: (identifier) @font-lock-variable-name-face)
-     (let_condition value: (identifier) @font-lock-variable-name-face)
-     (let_declaration value: (identifier) @font-lock-variable-name-face)
-     (match_arm value: (identifier) @font-lock-variable-name-face)
-     (match_expression value: (identifier) @font-lock-variable-name-face)
-     (reference_expression value: (identifier) @font-lock-variable-name-face)
-     (return_expression (identifier) @font-lock-variable-name-face)
-     (tuple_expression (identifier) @font-lock-variable-name-face)
-     (unary_expression (identifier) @font-lock-variable-name-face)
-     (while_expression condition: (identifier) @font-lock-variable-name-face))
+   '((arguments (identifier) @font-lock-variable-ref-face)
+     (array_expression (identifier) @font-lock-variable-ref-face)
+     (assignment_expression right: (identifier) @font-lock-variable-ref-face)
+     (binary_expression left: (identifier) @font-lock-variable-ref-face)
+     (binary_expression right: (identifier) @font-lock-variable-ref-face)
+     (block (identifier) @font-lock-variable-ref-face)
+     (compound_assignment_expr right: (identifier) @font-lock-variable-ref-face)
+     (field_expression value: (identifier) @font-lock-variable-ref-face)
+     (field_initializer value: (identifier) @font-lock-variable-ref-face)
+     (if_expression condition: (identifier) @font-lock-variable-ref-face)
+     (let_condition value: (identifier) @font-lock-variable-ref-face)
+     (let_declaration value: (identifier) @font-lock-variable-ref-face)
+     (match_arm value: (identifier) @font-lock-variable-ref-face)
+     (match_expression value: (identifier) @font-lock-variable-ref-face)
+     (reference_expression value: (identifier) @font-lock-variable-ref-face)
+     (return_expression (identifier) @font-lock-variable-ref-face)
+     (tuple_expression (identifier) @font-lock-variable-ref-face)
+     (unary_expression (identifier) @font-lock-variable-ref-face)
+     (while_expression condition: (identifier) @font-lock-variable-ref-face))
 
    :language 'rust
    :feature 'escape-sequence
diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
index 41cf4ff08f6..b560869af18 100644
--- a/lisp/progmodes/typescript-ts-mode.el
+++ b/lisp/progmodes/typescript-ts-mode.el
@@ -245,16 +245,13 @@ typescript-ts-mode--font-lock-settings
    :language language
    :feature 'property
    `((property_signature
-      name: (property_identifier) @font-lock-property-face)
+      name: (property_identifier) @font-lock-property-name-face)
      (public_field_definition
-      name: (property_identifier) @font-lock-property-face)
+      name: (property_identifier) @font-lock-property-name-face)
 
      (pair key: (property_identifier) @font-lock-variable-name-face)
 
-     ((shorthand_property_identifier) @font-lock-property-face)
-
-     ((shorthand_property_identifier_pattern)
-      @font-lock-property-face))
+     ((shorthand_property_identifier) @font-lock-property-ref-face))
 
    :language language
    :feature 'expression
@@ -268,30 +265,32 @@ typescript-ts-mode--font-lock-settings
    :feature 'function
    '((call_expression
       function:
-      [(identifier) @font-lock-function-name-face
+      [(identifier) @font-lock-function-call-face
        (member_expression
-        property: (property_identifier) @font-lock-function-name-face)]))
+        property: (property_identifier) @font-lock-function-call-face)]))
 
    :language language
    :feature 'pattern
    `((pair_pattern
-      key: (property_identifier) @font-lock-property-face)
+      key: (property_identifier) @font-lock-property-ref-face)
+
+     (array_pattern (identifier) @font-lock-variable-name-face)
 
-     (array_pattern (identifier) @font-lock-variable-name-face))
+     ((shorthand_property_identifier_pattern) @font-lock-variable-name-face))
 
    :language language
    :feature 'jsx
    `((jsx_opening_element
       [(nested_identifier (identifier)) (identifier)]
-      @font-lock-function-name-face)
+      @font-lock-function-call-face)
 
      (jsx_closing_element
       [(nested_identifier (identifier)) (identifier)]
-      @font-lock-function-name-face)
+      @font-lock-function-call-face)
 
      (jsx_self_closing_element
       [(nested_identifier (identifier)) (identifier)]
-      @font-lock-function-name-face)
+      @font-lock-function-call-face)
 
      (jsx_attribute (property_identifier) @font-lock-constant-face))
 
diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index a1d7d4bbbec..39e38179359 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -1399,9 +1399,8 @@ css--treesit-settings
 
    :feature 'query
    :language 'css
-   '((keyword_query) @font-lock-property-face
-     (feature_name) @font-lock-property-face)
-
+   '((keyword_query) @font-lock-property-ref-face
+     (feature_name) @font-lock-property-ref-face)
 
    :feature 'bracket
    :language 'css
diff --git a/lisp/textmodes/toml-ts-mode.el b/lisp/textmodes/toml-ts-mode.el
index 416542084f1..2ff9d07d13b 100644
--- a/lisp/textmodes/toml-ts-mode.el
+++ b/lisp/textmodes/toml-ts-mode.el
@@ -92,8 +92,8 @@ toml-ts-mode--font-lock-settings
    :language 'toml
    :feature 'pair
    :override t            ; Needed for overriding string face on keys.
-   '((bare_key) @font-lock-property-face
-     (quoted_key) @font-lock-property-face
+   '((bare_key) @font-lock-property-ref-face
+     (quoted_key) @font-lock-property-ref-face
      (table ("[" @font-lock-bracket-face
              (_) @font-lock-type-face
              "]" @font-lock-bracket-face))
diff --git a/lisp/textmodes/yaml-ts-mode.el b/lisp/textmodes/yaml-ts-mode.el
index a25230e6e61..3153ff5a169 100644
--- a/lisp/textmodes/yaml-ts-mode.el
+++ b/lisp/textmodes/yaml-ts-mode.el
@@ -94,22 +94,22 @@ yaml-ts-mode--font-lock-settings
    :feature 'property
    :override t
    '((block_mapping_pair
-      key: (flow_node (plain_scalar (string_scalar) @font-lock-property-face)))
+      key: (flow_node (plain_scalar (string_scalar) @font-lock-property-name-face)))
      (block_mapping_pair
       key: (flow_node
-            [(double_quote_scalar) (single_quote_scalar)] @font-lock-property-face))
+            [(double_quote_scalar) (single_quote_scalar)] @font-lock-property-name-face))
      (flow_mapping
-      (_ key: (flow_node (plain_scalar (string_scalar) @font-lock-property-face))))
+      (_ key: (flow_node (plain_scalar (string_scalar) @font-lock-property-name-face))))
      (flow_mapping
       (_ key:
          (flow_node
-          [(double_quote_scalar) (single_quote_scalar)] @font-lock-property-face)))
+          [(double_quote_scalar) (single_quote_scalar)] @font-lock-property-name-face)))
      (flow_sequence
-      (_ key: (flow_node (plain_scalar (string_scalar) @font-lock-property-face))))
+      (_ key: (flow_node (plain_scalar (string_scalar) @font-lock-property-name-face))))
      (flow_sequence
       (_ key:
          (flow_node
-          [(double_quote_scalar) (single_quote_scalar)] @font-lock-property-face))))
+          [(double_quote_scalar) (single_quote_scalar)] @font-lock-property-name-face))))
 
    :language 'yaml
    :feature 'error

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-24  2:31             ` Dmitry Gutov
@ 2023-02-24  7:56               ` Eli Zaretskii
  2023-02-24 11:31                 ` Dmitry Gutov
  2023-02-25  1:06               ` Randy Taylor
  1 sibling, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-24  7:56 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: dev, casouri, theo, 61655, jacob.fai

> Date: Fri, 24 Feb 2023 04:31:08 +0200
> Cc: casouri@gmail.com, 61655@debbugs.gnu.org, jacob.fai@gmail.com,
>  Randy Taylor <dev@rjt.dev>, Theodor Thornhill <theo@thornhill.no>
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> > I'm okay with adding a few more faces to emacs-29, but please hurry,
> > as we don't have too much time for more additions.
> 
> Here's the patch which adds the faces and their uses in all ts modes.

LGTM, thanks.

> The important question here, I think, is whether we want to split 
> font-lock-property-faces in two, like I did here.

What harm can it do, if done like you did, i.e. the faces are
indistinguishable by default?





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-24  7:56               ` Eli Zaretskii
@ 2023-02-24 11:31                 ` Dmitry Gutov
  2023-02-24 11:44                   ` Eli Zaretskii
  2023-02-24 14:24                   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-24 11:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dev, casouri, theo, 61655, jacob.fai

On 24/02/2023 09:56, Eli Zaretskii wrote:
>> The important question here, I think, is whether we want to split
>> font-lock-property-faces in two, like I did here.
> What harm can it do, if done like you did, i.e. the faces are
> indistinguishable by default?

Not much. We do rename the face this late into the pretest, though.

And if not many modes use it, then it could not be worth it. OTOH, modes 
sometimes need additional faces, to use for their unique syntactic 
elements. So more faces could be a good idea, just from that perspective.

So if nobody has objections, I'll install the patch later today.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-24 11:31                 ` Dmitry Gutov
@ 2023-02-24 11:44                   ` Eli Zaretskii
  2023-02-24 14:24                   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-24 11:44 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: dev, casouri, theo, 61655, jacob.fai

> Date: Fri, 24 Feb 2023 13:31:59 +0200
> Cc: dev@rjt.dev, casouri@gmail.com, theo@thornhill.no, 61655@debbugs.gnu.org,
>  jacob.fai@gmail.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 24/02/2023 09:56, Eli Zaretskii wrote:
> >> The important question here, I think, is whether we want to split
> >> font-lock-property-faces in two, like I did here.
> > What harm can it do, if done like you did, i.e. the faces are
> > indistinguishable by default?
> 
> Not much. We do rename the face this late into the pretest, though.

That's fine: they were introduced just a few months ago.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-24 11:31                 ` Dmitry Gutov
  2023-02-24 11:44                   ` Eli Zaretskii
@ 2023-02-24 14:24                   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 24+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-24 14:24 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: dev, casouri, 61655, jacob.fai

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 24/02/2023 09:56, Eli Zaretskii wrote:
>>> The important question here, I think, is whether we want to split
>>> font-lock-property-faces in two, like I did here.
>> What harm can it do, if done like you did, i.e. the faces are
>> indistinguishable by default?
>
> Not much. We do rename the face this late into the pretest, though.
>
> And if not many modes use it, then it could not be worth it. OTOH, modes 
> sometimes need additional faces, to use for their unique syntactic 
> elements. So more faces could be a good idea, just from that perspective.
>
> So if nobody has objections, I'll install the patch later today.

No objections from me :)





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-24  2:31             ` Dmitry Gutov
  2023-02-24  7:56               ` Eli Zaretskii
@ 2023-02-25  1:06               ` Randy Taylor
  2023-02-25  2:28                 ` Dmitry Gutov
  2023-02-25  8:05                 ` Eli Zaretskii
  1 sibling, 2 replies; 24+ messages in thread
From: Randy Taylor @ 2023-02-25  1:06 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61655, Eli Zaretskii, Theodor Thornhill, casouri, jacob.fai

On Thursday, February 23rd, 2023 at 21:31, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 23/02/2023 20:15, Eli Zaretskii wrote:
> 
> > > Cc:61655@debbugs.gnu.org
> > > Date: Wed, 22 Feb 2023 23:38:04 +0200
> > > From: Dmitry Gutovdgutov@yandex.ru
> > > 
> > > On 22/02/2023 22:45, Yuan Fu wrote:
> > > 
> > > > Yeah that’s just an idea, and I don’t have problem adding faces. But we probably can’t keep adding more and more specific faces. At one point we’ll need to either add indirection, or ask users to just add their own fontification rules, if it is really specific. We’ll see.
> > > > An indirection seems like a separate new feature. Might be useful for
> > > > some, but probably unnecessary for this discussion.
> > > 
> > > > Function definition & call is totally reasonable. But adapting all the major modes to use them is might be too big a change for emacs-29.
> > > > The change itself should be very straightforward. If we agree on the set
> > > > of faces (for variables and properties as well, right?), I don't mind
> > > > posting a patch for review.
> > > 
> > > Whether it gets accepted or not.
> > > I'm okay with adding a few more faces to emacs-29, but please hurry,
> > > as we don't have too much time for more additions.
> 
> 
> Here's the patch which adds the faces and their uses in all ts modes.
> 
> Comments welcome from all the interested parties. The patch is mostly
> straightforward, but there are some changes added as well, where it was
> needed to differentiate between declarations and references.
> 
> The important question here, I think, is whether we want to split
> font-lock-property-faces in two, like I did here.
> 
> By analogy with the other faces, I think it's going to be useful to
> differentiate between property definitions and property references. Not
> many of the languages modes used font-lock-property-face for
> property/attribute definitions, but some did.

I'm not sure about the naming of font-lock-variable-ref-face. It's confusing for languages that support actual references like C++ and Rust.

Maybe the opposite direction is better: font-lock-variable-def-face (or something similar) for definitions (or whatnot), and font-lock-variable-name-face to refer to uses (same goes for property). Or font-lock-variable-use-face. I don't know, naming is hard :).

Personally, I don't really see the value in differentiating these for variables. I can understand it a little more for properties. But I guess it doesn't hurt to add if folks want it.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-25  1:06               ` Randy Taylor
@ 2023-02-25  2:28                 ` Dmitry Gutov
  2023-02-25  3:59                   ` Randy Taylor
  2023-02-25  8:05                 ` Eli Zaretskii
  1 sibling, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-25  2:28 UTC (permalink / raw)
  To: Randy Taylor; +Cc: 61655, Eli Zaretskii, Theodor Thornhill, casouri, jacob.fai

Hi Randy,

Thanks for the reply.

On 25/02/2023 03:06, Randy Taylor wrote:

>> Here's the patch which adds the faces and their uses in all ts modes.
>>
>> Comments welcome from all the interested parties. The patch is mostly
>> straightforward, but there are some changes added as well, where it was
>> needed to differentiate between declarations and references.
>>
>> The important question here, I think, is whether we want to split
>> font-lock-property-faces in two, like I did here.
>>
>> By analogy with the other faces, I think it's going to be useful to
>> differentiate between property definitions and property references. Not
>> many of the languages modes used font-lock-property-face for
>> property/attribute definitions, but some did.
> 
> I'm not sure about the naming of font-lock-variable-ref-face. It's confusing for languages that support actual references like C++ and Rust.

But even there "variable reference" is probably a suitable term for any 
occurrence of a variable other than its definition. While the references 
you're talking about are "value references".

> Maybe the opposite direction is better: font-lock-variable-def-face (or something similar) for definitions (or whatnot), and font-lock-variable-name-face to refer to uses (same goes for property). Or font-lock-variable-use-face. I don't know, naming is hard :).

I, uh, pushed the change before I noticed your email. ^^;

But perhaps we could refine, if there is enough support.

Indeed, I was slightly uneasy about the -ref- names, if only because 
they might seem a little cryptic. Using the name -def-face is something 
I thought about too, but it sounded a bit like the name of a macro.

A bigger problem, though, is that existing themes customize 
font-lock-variable-name-face. So we'd have to create inheritance the 
other way around (for the themes to continue working unchanged). Or we'd 
have to create face alias and use a yet different name for "variable 
references" (or "uses", or whatever we'd call them).

Inheritance "the other way around" would break the usage scenario 1 
below for users of existing themes. Or at least make it more difficult.

> Personally, I don't really see the value in differentiating these for variables. I can understand it a little more for properties. But I guess it doesn't hurt to add if folks want it.

I see two potential uses:

1. Customize treesit-font-lock-level to 4 but 
font-lock-variable-ref-face to copy 'default' (or close to it), to skip 
variable reference highlighting or make it less noticeable.

2. Pattern matching or comparably complex syntax which at a first glance 
may look like variable reference, but actually creates new bindings (or 
vice versa, creates new binding when one wanted to refer to an existing 
value).

Emphasizing the difference can help people, beginners especially [in a 
particular language].





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-25  2:28                 ` Dmitry Gutov
@ 2023-02-25  3:59                   ` Randy Taylor
  2023-02-25 13:05                     ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Randy Taylor @ 2023-02-25  3:59 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: casouri, 61655, Theodor Thornhill, Eli Zaretskii, jacob.fai

On Friday, February 24th, 2023 at 21:28, Dmitry Gutov <dgutov@yandex.ru> wrote:
> Hi Randy,
> 
> Thanks for the reply.
> 
> On 25/02/2023 03:06, Randy Taylor wrote:
> 
> > > Here's the patch which adds the faces and their uses in all ts modes.
> > > 
> > > Comments welcome from all the interested parties. The patch is mostly
> > > straightforward, but there are some changes added as well, where it was
> > > needed to differentiate between declarations and references.
> > > 
> > > The important question here, I think, is whether we want to split
> > > font-lock-property-faces in two, like I did here.
> > > 
> > > By analogy with the other faces, I think it's going to be useful to
> > > differentiate between property definitions and property references. Not
> > > many of the languages modes used font-lock-property-face for
> > > property/attribute definitions, but some did.
> > 
> > I'm not sure about the naming of font-lock-variable-ref-face. It's confusing for languages that support actual references like C++ and Rust.
> 
> 
> But even there "variable reference" is probably a suitable term for any
> occurrence of a variable other than its definition. While the references
> you're talking about are "value references".

It is a suitable term, but there is a confusing overlap, at least to me. In C++ parlance, for example, they are referred to as reference variables. I wasn't really getting down to the semantics of it, just when I see something like:

void quack(int& thing)
                ^^^^^
obj_t& thing2 = otherthing;
       ^^^^^^

Those are the things I would expect font-lock-variable-ref-face to highlight if I was just going off of the name, and I would expect many others to think the same.

> 
> > Maybe the opposite direction is better: font-lock-variable-def-face (or something similar) for definitions (or whatnot), and font-lock-variable-name-face to refer to uses (same goes for property). Or font-lock-variable-use-face. I don't know, naming is hard :).
> 
> 
> I, uh, pushed the change before I noticed your email. ^^;
> 
> But perhaps we could refine, if there is enough support.
> 
> Indeed, I was slightly uneasy about the -ref- names, if only because
> they might seem a little cryptic. Using the name -def-face is something
> I thought about too, but it sounded a bit like the name of a macro.
> 
> A bigger problem, though, is that existing themes customize
> font-lock-variable-name-face. So we'd have to create inheritance the
> other way around (for the themes to continue working unchanged). Or we'd
> have to create face alias and use a yet different name for "variable
> references" (or "uses", or whatever we'd call them).
> 
> Inheritance "the other way around" would break the usage scenario 1
> below for users of existing themes. Or at least make it more difficult.

I don't have any better ideas than font-lock-{property,variable}-use-face, so I guess we can see if anyone else has any opinions on the matter.

> 
> > Personally, I don't really see the value in differentiating these for variables. I can understand it a little more for properties. But I guess it doesn't hurt to add if folks want it.
> 
> 
> I see two potential uses:
> 
> 1. Customize treesit-font-lock-level to 4 but
> font-lock-variable-ref-face to copy 'default' (or close to it), to skip
> variable reference highlighting or make it less noticeable.

Wouldn't they just remove the variable feature if they want that?

> 
> 2. Pattern matching or comparably complex syntax which at a first glance
> may look like variable reference, but actually creates new bindings (or
> vice versa, creates new binding when one wanted to refer to an existing
> value).
> 
> Emphasizing the difference can help people, beginners especially [in a
> particular language].
>

No doubt there are uses, I just don't really see them actually getting much use in practice. But maybe I'm wrong :).





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-25  1:06               ` Randy Taylor
  2023-02-25  2:28                 ` Dmitry Gutov
@ 2023-02-25  8:05                 ` Eli Zaretskii
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-25  8:05 UTC (permalink / raw)
  To: Randy Taylor; +Cc: casouri, jacob.fai, theo, 61655, dgutov

> Date: Sat, 25 Feb 2023 01:06:02 +0000
> From: Randy Taylor <dev@rjt.dev>
> Cc: Eli Zaretskii <eliz@gnu.org>, casouri@gmail.com, 61655@debbugs.gnu.org, jacob.fai@gmail.com, Theodor Thornhill <theo@thornhill.no>
> 
> On Thursday, February 23rd, 2023 at 21:31, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> I'm not sure about the naming of font-lock-variable-ref-face. It's confusing for languages that support actual references like C++ and Rust.

That's a different kind of "reference".

> Maybe the opposite direction is better: font-lock-variable-def-face (or something similar) for definitions (or whatnot), and font-lock-variable-name-face to refer to uses (same goes for property).

No, that would rename a widely used face which we had for many years.

>  Or font-lock-variable-use-face.

That could work, but then we'd need to change all the other "-ref-"
faces to "-use-" as well.

> Personally, I don't really see the value in differentiating these for variables. I can understand it a little more for properties. But I guess it doesn't hurt to add if folks want it.

Right, it doesn't hurt.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-25  3:59                   ` Randy Taylor
@ 2023-02-25 13:05                     ` Dmitry Gutov
  2023-02-28  2:09                       ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-25 13:05 UTC (permalink / raw)
  To: Randy Taylor; +Cc: casouri, 61655, Theodor Thornhill, Eli Zaretskii, jacob.fai

On 25/02/2023 05:59, Randy Taylor wrote:

> It is a suitable term, but there is a confusing overlap, at least to me. In C++ parlance, for example, they are referred to as reference variables. I wasn't really getting down to the semantics of it, just when I see something like:
> 
> void quack(int& thing)
>                  ^^^^^
> obj_t& thing2 = otherthing;
>         ^^^^^^
> 
> Those are the things I would expect font-lock-variable-ref-face to highlight if I was just going off of the name, and I would expect many others to think the same.

That's not unreasonable.

> I don't have any better ideas than font-lock-{property,variable}-use-face, so I guess we can see if anyone else has any opinions on the matter.

The only one I could think of is 
font-lock-{property,variable}-value-face. But that one can give wrong 
ideas as well.

I suppose we should go with -use-, unless more alternatives are suggested.

>>> Personally, I don't really see the value in differentiating these for variables. I can understand it a little more for properties. But I guess it doesn't hurt to add if folks want it.
>>
>>
>> I see two potential uses:
>>
>> 1. Customize treesit-font-lock-level to 4 but
>> font-lock-variable-ref-face to copy 'default' (or close to it), to skip
>> variable reference highlighting or make it less noticeable.
> 
> Wouldn't they just remove the variable feature if they want that?

That's the obvious choice (which might be less accessible to those who 
don't venture into the manual, though, or don't write Elisp), but now 
they also have the option of toning down the highlights.

>> 2. Pattern matching or comparably complex syntax which at a first glance
>> may look like variable reference, but actually creates new bindings (or
>> vice versa, creates new binding when one wanted to refer to an existing
>> value).
>>
>> Emphasizing the difference can help people, beginners especially [in a
>> particular language].
>>
> 
> No doubt there are uses, I just don't really see them actually getting much use in practice. But maybe I'm wrong :).

We'll see. I hope we will. Some third-party theme or two might start 
carrying that distinction, to start with.

It might be nice to improve the default theme in that regard too, but 
that's a non-trivial task.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately
  2023-02-25 13:05                     ` Dmitry Gutov
@ 2023-02-28  2:09                       ` Dmitry Gutov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-28  2:09 UTC (permalink / raw)
  To: Randy Taylor
  Cc: casouri, jacob.fai, Theodor Thornhill, 61655-done, Eli Zaretskii

On 25/02/2023 15:05, Dmitry Gutov wrote:
> I suppose we should go with -use-, unless more alternatives are suggested.

And done. Thanks all!





^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-02-28  2:09 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-20 15:54 bug#61655: [Tree sitter] [Feature Request] font-lock function calls, definitions, separately Jacob Faibussowitsch
2023-02-20 17:03 ` Eli Zaretskii
2023-02-20 20:24   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-20 20:45     ` Jacob Faibussowitsch
2023-02-21  8:28 ` Yuan Fu
2023-02-21  9:55   ` Dmitry Gutov
2023-02-21 15:31     ` Jacob Faibussowitsch
2023-02-21 23:24       ` Dmitry Gutov
2023-02-22 18:07         ` Jacob Faibussowitsch
2023-02-22 21:39           ` Dmitry Gutov
2023-02-22 20:45       ` Yuan Fu
2023-02-22 21:38         ` Dmitry Gutov
2023-02-23 18:15           ` Eli Zaretskii
2023-02-24  2:31             ` Dmitry Gutov
2023-02-24  7:56               ` Eli Zaretskii
2023-02-24 11:31                 ` Dmitry Gutov
2023-02-24 11:44                   ` Eli Zaretskii
2023-02-24 14:24                   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-25  1:06               ` Randy Taylor
2023-02-25  2:28                 ` Dmitry Gutov
2023-02-25  3:59                   ` Randy Taylor
2023-02-25 13:05                     ` Dmitry Gutov
2023-02-28  2:09                       ` Dmitry Gutov
2023-02-25  8:05                 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).