unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
@ 2023-02-01  2:08 Dmitry Gutov
  2023-02-01  5:18 ` Yuan Fu
  2023-02-02  2:34 ` Randy Taylor
  0 siblings, 2 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-01  2:08 UTC (permalink / raw)
  To: 61205; +Cc: yuan fu, randy taylor

X-Debbugs-Cc: Yuan Fu <casouri@gmail.com>, Randy Taylor <dev@rjt.dev>

Some new built-in modes has 'function' feature highlighting thus enabled 
by default.

rust-ts-mode, go-ts-mode, cmake-mode

Should we move it to 4 for consistency with the rest?

Previously, we talked about that and concluded that function calls are 
usually everywhere and are easy to notice without additional highlighting.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-01  2:08 bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list Dmitry Gutov
@ 2023-02-01  5:18 ` Yuan Fu
  2023-02-02  2:34   ` Dmitry Gutov
  2023-02-02 20:25   ` Dmitry Gutov
  2023-02-02  2:34 ` Randy Taylor
  1 sibling, 2 replies; 24+ messages in thread
From: Yuan Fu @ 2023-02-01  5:18 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, randy taylor



> On Jan 31, 2023, at 6:08 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> X-Debbugs-Cc: Yuan Fu <casouri@gmail.com>, Randy Taylor <dev@rjt.dev>
> 
> Some new built-in modes has 'function' feature highlighting thus enabled by default.
> 
> rust-ts-mode, go-ts-mode, cmake-mode
> 
> Should we move it to 4 for consistency with the rest?
> 
> Previously, we talked about that and concluded that function calls are usually everywhere and are easy to notice without additional highlighting.

Right, I think they should be level 4.

Yuan




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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-01  2:08 bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list Dmitry Gutov
  2023-02-01  5:18 ` Yuan Fu
@ 2023-02-02  2:34 ` Randy Taylor
  2023-02-02  2:44   ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Randy Taylor @ 2023-02-02  2:34 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, yuan fu

On Tuesday, January 31st, 2023 at 21:08, Dmitry Gutov <dgutov@yandex.ru> wrote:
> X-Debbugs-Cc: Yuan Fu casouri@gmail.com, Randy Taylor dev@rjt.dev
> 
> 
> Some new built-in modes has 'function' feature highlighting thus enabled
> by default.
> 
> rust-ts-mode, go-ts-mode, cmake-mode
> 
> Should we move it to 4 for consistency with the rest?
> 
> Previously, we talked about that and concluded that function calls are
> usually everywhere and are easy to notice without additional highlighting.

If that's what folks decided on then we should keep everything consistent.

Personally, I find it odd that out of the box default highlighting wouldn't highlight function calls.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-01  5:18 ` Yuan Fu
@ 2023-02-02  2:34   ` Dmitry Gutov
  2023-02-02  3:18     ` Randy Taylor
  2023-02-02 20:25   ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-02  2:34 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 61205, randy taylor

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

On 01/02/2023 07:18, Yuan Fu wrote:
> 
> 
>> On Jan 31, 2023, at 6:08 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
>>
>> X-Debbugs-Cc: Yuan Fu <casouri@gmail.com>, Randy Taylor <dev@rjt.dev>
>>
>> Some new built-in modes has 'function' feature highlighting thus enabled by default.
>>
>> rust-ts-mode, go-ts-mode, cmake-mode
>>
>> Should we move it to 4 for consistency with the rest?
>>
>> Previously, we talked about that and concluded that function calls are usually everywhere and are easy to notice without additional highlighting.
> 
> Right, I think they should be level 4.

OK, I tried simply changing that, and it turned out none of these modes 
have a 'definition' feature, so highlightings get stripped from all 
definitions as well.

And in a couple of cases function calls got highlighted either as a 
type, or as a property. I fixed the latter by deleting one selector, and 
the latter didn't touch per se.

But since the same modes had 'variable' and 'property' features in the 
3rd level as well, I moved them to the 4th (that resolved the incorrect 
highlighting as property mentioned above, but it's probably need to be 
improved later, in case someone will want to enable 'property' but not 
'function' highlighting).

And I added the 'definition' features and moved some highlighting rules 
there. And added some.

So the patch looks a bit more complex than expected, see attached.

cmake-ts-mode, in the end, I ended up keeping as-is. The 'variable' 
selector is more functional there than everywhere else (the grammar uses 
those nodes for template expansion), and if 'function' is removed, the 
buffer looks almost devoid of highlighting.

Also none of these modes have highlighting for function parameters or 
assignments. That can be improved later.

[-- Attachment #2: ts-modes-refine-features.diff --]
[-- Type: text/x-patch, Size: 4556 bytes --]

diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index 5f3e1ea3e68..3536a6cb0cd 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -119,17 +119,27 @@ go-ts-mode--font-lock-settings
    :feature 'delimiter
    '((["," "." ";" ":"]) @font-lock-delimiter-face)
 
+   :language 'go
+   :feature 'definition
+   '((function_declaration
+      name: (identifier) @font-lock-function-name-face)
+     (method_declaration
+      name: (field_identifier) @font-lock-function-name-face)
+     (method_spec
+      name: (field_identifier) @font-lock-function-name-face))
+
+   :language 'go
+   :feature 'definition
+   '((field_declaration
+      name: (field_identifier) @font-lock-property-face))
+
    :language 'go
    :feature 'function
    '((call_expression
       function: (identifier) @font-lock-function-name-face)
      (call_expression
       function: (selector_expression
-                 field: (field_identifier) @font-lock-function-name-face))
-     (function_declaration
-      name: (identifier) @font-lock-function-name-face)
-     (method_declaration
-      name: (field_identifier) @font-lock-function-name-face))
+                 field: (field_identifier) @font-lock-function-name-face)))
 
    :language 'go
    :feature 'keyword
@@ -218,10 +228,10 @@ go-ts-mode
     (setq-local treesit-font-lock-settings go-ts-mode--font-lock-settings)
     (setq-local treesit-font-lock-feature-list
                 '(( comment)
-                  ( keyword string type)
-                  ( constant escape-sequence function label number
-                    property variable)
-                  ( bracket delimiter error operator)))
+                  ( keyword string type definition)
+                  ( constant escape-sequence label number)
+                  ( bracket delimiter error operator function variable
+                    property)))
 
     (treesit-major-mode-setup)))
 
diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index e317793d211..20ee2e420d3 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -155,6 +155,16 @@ rust-ts-mode--font-lock-settings
    :feature 'delimiter
    '((["," "." ";" ":" "::"]) @font-lock-delimiter-face)
 
+   :language 'rust
+   :feature 'definition
+   '((function_item name: (identifier) @font-lock-function-name-face)
+     (macro_definition "macro_rules!" @font-lock-constant-face)
+     (macro_definition (identifier) @font-lock-preprocessor-face))
+
+   :language 'rust
+   :feature 'definition
+   '((field_declaration name: (field_identifier) @font-lock-property-face))
+
    :language 'rust
    :feature 'function
    '((call_expression
@@ -164,15 +174,12 @@ rust-ts-mode--font-lock-settings
         field: (field_identifier) @font-lock-function-name-face)
        (scoped_identifier
         name: (identifier) @font-lock-function-name-face)])
-     (function_item (identifier) @font-lock-function-name-face)
      (generic_function
       function: [(identifier) @font-lock-function-name-face
                  (field_expression
                   field: (field_identifier) @font-lock-function-name-face)
                  (scoped_identifier
                   name: (identifier) @font-lock-function-name-face)])
-     (macro_definition "macro_rules!" @font-lock-constant-face)
-     (macro_definition (identifier) @font-lock-preprocessor-face)
      (macro_invocation macro: (identifier) @font-lock-preprocessor-face))
 
    :language 'rust
@@ -208,7 +215,6 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     (scoped_identifier name: (identifier) @font-lock-type-face)
      (scoped_identifier path: (identifier) @font-lock-constant-face)
      (scoped_identifier
       (scoped_identifier
@@ -318,10 +324,10 @@ rust-ts-mode
     (setq-local treesit-font-lock-settings rust-ts-mode--font-lock-settings)
     (setq-local treesit-font-lock-feature-list
                 '(( comment)
-                  ( keyword string)
+                  ( keyword string definition)
                   ( attribute builtin constant escape-sequence
-                    function number property type variable)
-                  ( bracket delimiter error operator)))
+                    number type)
+                  ( bracket delimiter error operator function property variable)))
 
     ;; Imenu.
     (setq-local treesit-simple-imenu-settings

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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-02  2:34 ` Randy Taylor
@ 2023-02-02  2:44   ` Dmitry Gutov
  2023-02-02  3:29     ` Randy Taylor
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-02  2:44 UTC (permalink / raw)
  To: Randy Taylor; +Cc: 61205, yuan fu

On 02/02/2023 04:34, Randy Taylor wrote:
> On Tuesday, January 31st, 2023 at 21:08, Dmitry Gutov<dgutov@yandex.ru>  wrote:
>> X-Debbugs-Cc: Yuan Fucasouri@gmail.com, Randy Taylordev@rjt.dev
>>
>>
>> Some new built-in modes has 'function' feature highlighting thus enabled
>> by default.
>>
>> rust-ts-mode, go-ts-mode, cmake-mode
>>
>> Should we move it to 4 for consistency with the rest?
>>
>> Previously, we talked about that and concluded that function calls are
>> usually everywhere and are easy to notice without additional highlighting.
> If that's what folks decided on then we should keep everything consistent.
> 
> Personally, I find it odd that out of the box default highlighting wouldn't highlight function calls.

I think that's more useful in some languages, and less in others.

I guess we settled on this particular convention to be more consistent 
with existing major modes in Emacs. But I can see how it can be 
appealing, especially in languages with more complex syntax such as Rust.

Same for 'property'.

The one feature that I'm fairly certain is currently useless is 
'variable', because we don't have any variable scope tracking (yet), and 
the grammars don't do it for us. So too many tokens get highlighted with 
font-lock-variable-name-face. Try the current rust-ts-mode, for example: 
almost everything ends up with that face.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-02  2:34   ` Dmitry Gutov
@ 2023-02-02  3:18     ` Randy Taylor
  2023-02-02 11:03       ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Randy Taylor @ 2023-02-02  3:18 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, Yuan Fu

On Wednesday, February 1st, 2023 at 21:34, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 01/02/2023 07:18, Yuan Fu wrote:
> 
> > > On Jan 31, 2023, at 6:08 PM, Dmitry Gutov dgutov@yandex.ru wrote:
> > > 
> > > X-Debbugs-Cc: Yuan Fu casouri@gmail.com, Randy Taylor dev@rjt.dev
> > > 
> > > Some new built-in modes has 'function' feature highlighting thus enabled by default.
> > > 
> > > rust-ts-mode, go-ts-mode, cmake-mode
> > > 
> > > Should we move it to 4 for consistency with the rest?
> > > 
> > > Previously, we talked about that and concluded that function calls are usually everywhere and are easy to notice without additional highlighting.
> > 
> > Right, I think they should be level 4.
> 
> 
> OK, I tried simply changing that, and it turned out none of these modes
> have a 'definition' feature, so highlightings get stripped from all
> definitions as well.
> 
> And in a couple of cases function calls got highlighted either as a
> type, or as a property. I fixed the latter by deleting one selector, and
> the latter didn't touch per se.
> 
> But since the same modes had 'variable' and 'property' features in the
> 3rd level as well, I moved them to the 4th (that resolved the incorrect
> highlighting as property mentioned above, but it's probably need to be
> improved later, in case someone will want to enable 'property' but not
> 'function' highlighting).
> 
> And I added the 'definition' features and moved some highlighting rules
> there. And added some.
> 
> So the patch looks a bit more complex than expected, see attached.
> 
> cmake-ts-mode, in the end, I ended up keeping as-is. The 'variable'
> selector is more functional there than everywhere else (the grammar uses
> those nodes for template expansion), and if 'function' is removed, the
> buffer looks almost devoid of highlighting.
> 
> Also none of these modes have highlighting for function parameters or
> assignments. That can be improved later.

I just took a quick look (will have more time tomorrow hopefully), but:

Why are there 2 separate definition features for both go and rust?
There should just be one each.

> -     (scoped_identifier name: (identifier) @font-lock-type-face)

Why was this removed? I think this will lead to some imports not being highlighted at the very least.

And please keep treesit-font-lock-feature-list alphabetized :).





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-02  2:44   ` Dmitry Gutov
@ 2023-02-02  3:29     ` Randy Taylor
  2023-02-02 11:11       ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Randy Taylor @ 2023-02-02  3:29 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, yuan fu

On Wednesday, February 1st, 2023 at 21:44, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 02/02/2023 04:34, Randy Taylor wrote:
> 
> > On Tuesday, January 31st, 2023 at 21:08, Dmitry Gutovdgutov@yandex.ru wrote:
> > 
> > > X-Debbugs-Cc: Yuan Fucasouri@gmail.com, Randy Taylordev@rjt.dev
> > > 
> > > Some new built-in modes has 'function' feature highlighting thus enabled
> > > by default.
> > > 
> > > rust-ts-mode, go-ts-mode, cmake-mode
> > > 
> > > Should we move it to 4 for consistency with the rest?
> > > 
> > > Previously, we talked about that and concluded that function calls are
> > > usually everywhere and are easy to notice without additional highlighting.
> > > If that's what folks decided on then we should keep everything consistent.
> > 
> > Personally, I find it odd that out of the box default highlighting wouldn't highlight function calls.
> 
> 
> I think that's more useful in some languages, and less in others.

Sure, and people will have different opinions on it too. But I still think out of the box we should be highlighting most things like other editors do. It's easy enough to take out what you don't like.

> 
> I guess we settled on this particular convention to be more consistent
> with existing major modes in Emacs. But I can see how it can be
> appealing, especially in languages with more complex syntax such as Rust.
> 
> Same for 'property'.
> 
> The one feature that I'm fairly certain is currently useless is
> 'variable', because we don't have any variable scope tracking (yet), and
> the grammars don't do it for us. So too many tokens get highlighted with
> font-lock-variable-name-face. Try the current rust-ts-mode, for example:
> almost everything ends up with that face.

If anything that isn't a variable is getting highlighted, then that's a bug. Anything it's catching that isn't a variable can probably be dealt with similar to how token_tree is dealt with.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-02  3:18     ` Randy Taylor
@ 2023-02-02 11:03       ` Dmitry Gutov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-02 11:03 UTC (permalink / raw)
  To: Randy Taylor; +Cc: 61205, Yuan Fu

On 02/02/2023 05:18, Randy Taylor wrote:
> On Wednesday, February 1st, 2023 at 21:34, Dmitry Gutov <dgutov@yandex.ru> wrote:
>> On 01/02/2023 07:18, Yuan Fu wrote:
>>
>>>> On Jan 31, 2023, at 6:08 PM, Dmitry Gutov dgutov@yandex.ru wrote:
>>>>
>>>> X-Debbugs-Cc: Yuan Fu casouri@gmail.com, Randy Taylor dev@rjt.dev
>>>>
>>>> Some new built-in modes has 'function' feature highlighting thus enabled by default.
>>>>
>>>> rust-ts-mode, go-ts-mode, cmake-mode
>>>>
>>>> Should we move it to 4 for consistency with the rest?
>>>>
>>>> Previously, we talked about that and concluded that function calls are usually everywhere and are easy to notice without additional highlighting.
>>>
>>> Right, I think they should be level 4.
>>
>>
>> OK, I tried simply changing that, and it turned out none of these modes
>> have a 'definition' feature, so highlightings get stripped from all
>> definitions as well.
>>
>> And in a couple of cases function calls got highlighted either as a
>> type, or as a property. I fixed the latter by deleting one selector, and
>> the latter didn't touch per se.
>>
>> But since the same modes had 'variable' and 'property' features in the
>> 3rd level as well, I moved them to the 4th (that resolved the incorrect
>> highlighting as property mentioned above, but it's probably need to be
>> improved later, in case someone will want to enable 'property' but not
>> 'function' highlighting).
>>
>> And I added the 'definition' features and moved some highlighting rules
>> there. And added some.
>>
>> So the patch looks a bit more complex than expected, see attached.
>>
>> cmake-ts-mode, in the end, I ended up keeping as-is. The 'variable'
>> selector is more functional there than everywhere else (the grammar uses
>> those nodes for template expansion), and if 'function' is removed, the
>> buffer looks almost devoid of highlighting.
>>
>> Also none of these modes have highlighting for function parameters or
>> assignments. That can be improved later.
> 
> I just took a quick look (will have more time tomorrow hopefully), but:
> 
> Why are there 2 separate definition features for both go and rust?
> There should just be one each.

It was an arbitrary choice on my part: the queries highlight different 
things (with different faces), so I put them separately.

I don't mind if we combine them.

>> -     (scoped_identifier name: (identifier) @font-lock-type-face)
> 
> Why was this removed? I think this will lead to some imports not being highlighted at the very least.

It led to code like 'test1' in test::test1(); being highlighted with the 
font-lock-property-face. I didn't have a lot of time to spend on this, 
so just removed the rule.

Some imports indeed are missing highlighting as a result. Perhaps the 
queries need to be scoped to 'use_declaration' nodes?

> And please keep treesit-font-lock-feature-list alphabetized :).

Sure.

And in case you wanted to propose the next revision of this patch, I 
certainly wouldn't mind ;-)





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-02  3:29     ` Randy Taylor
@ 2023-02-02 11:11       ` Dmitry Gutov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-02 11:11 UTC (permalink / raw)
  To: Randy Taylor; +Cc: 61205, yuan fu

On 02/02/2023 05:29, Randy Taylor wrote:
> On Wednesday, February 1st, 2023 at 21:44, Dmitry Gutov <dgutov@yandex.ru> wrote:
>> On 02/02/2023 04:34, Randy Taylor wrote:
>>
>>> On Tuesday, January 31st, 2023 at 21:08, Dmitry Gutovdgutov@yandex.ru wrote:
>>>
>>>> X-Debbugs-Cc: Yuan Fucasouri@gmail.com, Randy Taylordev@rjt.dev
>>>>
>>>> Some new built-in modes has 'function' feature highlighting thus enabled
>>>> by default.
>>>>
>>>> rust-ts-mode, go-ts-mode, cmake-mode
>>>>
>>>> Should we move it to 4 for consistency with the rest?
>>>>
>>>> Previously, we talked about that and concluded that function calls are
>>>> usually everywhere and are easy to notice without additional highlighting.
>>>> If that's what folks decided on then we should keep everything consistent.
>>>
>>> Personally, I find it odd that out of the box default highlighting wouldn't highlight function calls.
>>
>>
>> I think that's more useful in some languages, and less in others.
> 
> Sure, and people will have different opinions on it too. But I still think out of the box we should be highlighting most things like other editors do.

Perhaps we should have an extra level between 3 (highlight sparingly 
like other Emacs modes do) and 4 (highlight everything including stuff 
that doesn't always look great).

> It's easy enough to take out what you don't like.

Note that in this case "taking out" some things uncovered that the 
highlighting really isn't great in a different configuration.

>> I guess we settled on this particular convention to be more consistent
>> with existing major modes in Emacs. But I can see how it can be
>> appealing, especially in languages with more complex syntax such as Rust.
>>
>> Same for 'property'.
>>
>> The one feature that I'm fairly certain is currently useless is
>> 'variable', because we don't have any variable scope tracking (yet), and
>> the grammars don't do it for us. So too many tokens get highlighted with
>> font-lock-variable-name-face. Try the current rust-ts-mode, for example:
>> almost everything ends up with that face.
> 
> If anything that isn't a variable is getting highlighted, then that's a bug. Anything it's catching that isn't a variable can probably be dealt with similar to how token_tree is dealt with.

With a list of exceptions? I hadn't thought about that. But it sounds 
like it might be a fair amount of work. Depends on the specific grammar, 
of course.

Note that you'd also need to add such rules for every "feature" that 
might be disabled by the user, for its identifier tokens not to start 
being highlighted as a variable (if the user kept 'variable' in the 
features list). That's the current design we're working off.

And tree-sitter has a recommended mechanism for highlighting locals. We 
just haven't implemented it yet.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-01  5:18 ` Yuan Fu
  2023-02-02  2:34   ` Dmitry Gutov
@ 2023-02-02 20:25   ` Dmitry Gutov
  2023-02-03  2:38     ` Yuan Fu
  2023-02-03  6:46     ` Eli Zaretskii
  1 sibling, 2 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-02 20:25 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 61205, Theodor Thornhill, randy taylor

On 01/02/2023 07:18, Yuan Fu wrote:
> 
>> On Jan 31, 2023, at 6:08 PM, Dmitry Gutov<dgutov@yandex.ru>  wrote:
>>
>> X-Debbugs-Cc: Yuan Fu<casouri@gmail.com>, Randy Taylor<dev@rjt.dev>
>>
>> Some new built-in modes has 'function' feature highlighting thus enabled by default.
>>
>> rust-ts-mode, go-ts-mode, cmake-mode
>>
>> Should we move it to 4 for consistency with the rest?
>>
>> Previously, we talked about that and concluded that function calls are usually everywhere and are easy to notice without additional highlighting.
> Right, I think they should be level 4.

On a related note: 'property' is in level 3 in c-ts-mode.

Should it to go level 4?

Also in typescript-ts-mode.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-02 20:25   ` Dmitry Gutov
@ 2023-02-03  2:38     ` Yuan Fu
  2023-02-03  2:51       ` Dmitry Gutov
  2023-02-03  6:46     ` Eli Zaretskii
  1 sibling, 1 reply; 24+ messages in thread
From: Yuan Fu @ 2023-02-03  2:38 UTC (permalink / raw)
  To: Dmitry Gutov
  Cc: 61205, Theodor Thornhill, Jostein Kjønigsen, randy taylor



> On Feb 2, 2023, at 12:25 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 01/02/2023 07:18, Yuan Fu wrote:
>>> On Jan 31, 2023, at 6:08 PM, Dmitry Gutov<dgutov@yandex.ru>  wrote:
>>> 
>>> X-Debbugs-Cc: Yuan Fu<casouri@gmail.com>, Randy Taylor<dev@rjt.dev>
>>> 
>>> Some new built-in modes has 'function' feature highlighting thus enabled by default.
>>> 
>>> rust-ts-mode, go-ts-mode, cmake-mode
>>> 
>>> Should we move it to 4 for consistency with the rest?
>>> 
>>> Previously, we talked about that and concluded that function calls are usually everywhere and are easy to notice without additional highlighting.
>> Right, I think they should be level 4.
> 
> On a related note: 'property' is in level 3 in c-ts-mode.
> 
> Should it to go level 4?

I believe property is level 3. Quoting the (emacs) manual:

Level 1
     This level usually fontifies only comments and function names in
     function definitions.
Level 2
     This level adds fontification of keywords, strings, and data types.
Level 3
     This is the default level; it adds fontification of assignments,
     numbers, properties, etc.
Level 4
     This level adds everything else that can be fontified: operators,
     delimiters, brackets, other punctuation, function names in function
     calls, variables, etc.

Yuan




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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03  2:38     ` Yuan Fu
@ 2023-02-03  2:51       ` Dmitry Gutov
  2023-02-03  6:45         ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-03  2:51 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 61205, Theodor Thornhill, Jostein Kjønigsen, randy taylor

On 03/02/2023 04:38, Yuan Fu wrote:
> 
> 
>> On Feb 2, 2023, at 12:25 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
>>
>> On 01/02/2023 07:18, Yuan Fu wrote:
>>>> On Jan 31, 2023, at 6:08 PM, Dmitry Gutov<dgutov@yandex.ru>  wrote:
>>>>
>>>> X-Debbugs-Cc: Yuan Fu<casouri@gmail.com>, Randy Taylor<dev@rjt.dev>
>>>>
>>>> Some new built-in modes has 'function' feature highlighting thus enabled by default.
>>>>
>>>> rust-ts-mode, go-ts-mode, cmake-mode
>>>>
>>>> Should we move it to 4 for consistency with the rest?
>>>>
>>>> Previously, we talked about that and concluded that function calls are usually everywhere and are easy to notice without additional highlighting.
>>> Right, I think they should be level 4.
>>
>> On a related note: 'property' is in level 3 in c-ts-mode.
>>
>> Should it to go level 4?
> 
> I believe property is level 3. Quoting the (emacs) manual:
> 
> Level 1
>       This level usually fontifies only comments and function names in
>       function definitions.
> Level 2
>       This level adds fontification of keywords, strings, and data types.
> Level 3
>       This is the default level; it adds fontification of assignments,
>       numbers, properties, etc.
> Level 4
>       This level adds everything else that can be fontified: operators,
>       delimiters, brackets, other punctuation, function names in function
>       calls, variables, etc.

The manual could be updated.

Here's where we seem to have agreed that is should be level 4: 
https://lists.gnu.org/archive/html/emacs-devel/2022-12/msg01221.html

Even that discussion aside, property accesses and function calls seem to 
be similar enough as syntactic elements (in terms of position, usage, 
and frequency), so they should probably be on the same level. Don't you 
think?





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03  2:51       ` Dmitry Gutov
@ 2023-02-03  6:45         ` Eli Zaretskii
  0 siblings, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-03  6:45 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, casouri, theo, jostein, dev

> Cc: 61205@debbugs.gnu.org, Theodor Thornhill <theo@thornhill.no>,
>  Jostein Kjønigsen <jostein@secure.kjonigsen.net>,
>  randy taylor <dev@rjt.dev>
> Date: Fri, 3 Feb 2023 04:51:04 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> >> On a related note: 'property' is in level 3 in c-ts-mode.
> >>
> >> Should it to go level 4?
> > 
> > I believe property is level 3. Quoting the (emacs) manual:
> > 
> > Level 1
> >       This level usually fontifies only comments and function names in
> >       function definitions.
> > Level 2
> >       This level adds fontification of keywords, strings, and data types.
> > Level 3
> >       This is the default level; it adds fontification of assignments,
> >       numbers, properties, etc.
> > Level 4
> >       This level adds everything else that can be fontified: operators,
> >       delimiters, brackets, other punctuation, function names in function
> >       calls, variables, etc.
> 
> The manual could be updated.
> 
> Here's where we seem to have agreed that is should be level 4: 
> https://lists.gnu.org/archive/html/emacs-devel/2022-12/msg01221.html

Whatever you decide, please keep the doc string of
treesit-font-lock-level and the manual in sync.  I already needed to
fix those more than once because the code "disagreed" with the
documentation.  The pretest is too close for us to be able to sustain
such inaccuracies and resolve them in time.

So please present the patches to this effect for review and comments
before installing them.  TIA.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-02 20:25   ` Dmitry Gutov
  2023-02-03  2:38     ` Yuan Fu
@ 2023-02-03  6:46     ` Eli Zaretskii
  2023-02-03 11:42       ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-03  6:46 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, casouri, theo, dev

> Cc: 61205@debbugs.gnu.org, Theodor Thornhill <theo@thornhill.no>,
>  randy taylor <dev@rjt.dev>
> Date: Thu, 2 Feb 2023 22:25:47 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On a related note: 'property' is in level 3 in c-ts-mode.
> 
> Should it to go level 4?
> 
> Also in typescript-ts-mode.

What is 'property' in those modes?  In c-ts-mode, is 'property' the
name of a struct or enum member, as in foo.bar?  Or is it something
else?





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03  6:46     ` Eli Zaretskii
@ 2023-02-03 11:42       ` Dmitry Gutov
  2023-02-03 12:19         ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-03 11:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61205, casouri, theo, dev

On 03/02/2023 08:46, Eli Zaretskii wrote:
>> Cc:61205@debbugs.gnu.org, Theodor Thornhill<theo@thornhill.no>,
>>   randy taylor<dev@rjt.dev>
>> Date: Thu, 2 Feb 2023 22:25:47 +0200
>> From: Dmitry Gutov<dgutov@yandex.ru>
>>
>> On a related note: 'property' is in level 3 in c-ts-mode.
>>
>> Should it to go level 4?
>>
>> Also in typescript-ts-mode.
> What is 'property' in those modes?  In c-ts-mode, is 'property' the
> name of a struct or enum member, as in foo.bar?  Or is it something
> else?

It's the 'bar' in 'foo.bar', yes. Specifically in the cases where it's 
being looked up, rather than defined (in a type definition). Examples:

	  it2.lnum_pixel_width = it.lnum_pixel_width;

	  || (it.bidi_p && it.bidi_it.scan_dir == -1

'lnum_pixel_width', 'bidi_p', 'bidi_it' and 'scan_dir' are highlighted 
with font-lock-property-face. You can see it for yourself by trying 
c-ts-mode in any of our files.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03 11:42       ` Dmitry Gutov
@ 2023-02-03 12:19         ` Eli Zaretskii
  2023-02-03 15:15           ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-03 12:19 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, casouri, theo, dev

> Date: Fri, 3 Feb 2023 13:42:51 +0200
> Cc: 61205@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no, dev@rjt.dev
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 03/02/2023 08:46, Eli Zaretskii wrote:
> >> Cc:61205@debbugs.gnu.org, Theodor Thornhill<theo@thornhill.no>,
> >>   randy taylor<dev@rjt.dev>
> >> Date: Thu, 2 Feb 2023 22:25:47 +0200
> >> From: Dmitry Gutov<dgutov@yandex.ru>
> >>
> >> On a related note: 'property' is in level 3 in c-ts-mode.
> >>
> >> Should it to go level 4?
> >>
> >> Also in typescript-ts-mode.
> > What is 'property' in those modes?  In c-ts-mode, is 'property' the
> > name of a struct or enum member, as in foo.bar?  Or is it something
> > else?
> 
> It's the 'bar' in 'foo.bar', yes. Specifically in the cases where it's 
> being looked up, rather than defined (in a type definition). Examples:
> 
> 	  it2.lnum_pixel_width = it.lnum_pixel_width;
> 
> 	  || (it.bidi_p && it.bidi_it.scan_dir == -1
> 
> 'lnum_pixel_width', 'bidi_p', 'bidi_it' and 'scan_dir' are highlighted 
> with font-lock-property-face. You can see it for yourself by trying 
> c-ts-mode in any of our files.

Then as far as I'm concerned, this can go to level 4, but it must be
done consistently across all the *-ts modes.  So if some mode wants
'property' to be highlighted, and wants it badly, we should IMO keep
it in C as well.

In any case, please make this consistent across all the relevant
modes, and don't forget adjusting the documentation of
treesit-font-lock-level accordingly as needed.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03 12:19         ` Eli Zaretskii
@ 2023-02-03 15:15           ` Dmitry Gutov
  2023-02-03 15:54             ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-03 15:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61205, casouri, theo, dev

On 03/02/2023 14:19, Eli Zaretskii wrote:
>> Date: Fri, 3 Feb 2023 13:42:51 +0200
>> Cc: 61205@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no, dev@rjt.dev
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> On 03/02/2023 08:46, Eli Zaretskii wrote:
>>>> Cc:61205@debbugs.gnu.org, Theodor Thornhill<theo@thornhill.no>,
>>>>    randy taylor<dev@rjt.dev>
>>>> Date: Thu, 2 Feb 2023 22:25:47 +0200
>>>> From: Dmitry Gutov<dgutov@yandex.ru>
>>>>
>>>> On a related note: 'property' is in level 3 in c-ts-mode.
>>>>
>>>> Should it to go level 4?
>>>>
>>>> Also in typescript-ts-mode.
>>> What is 'property' in those modes?  In c-ts-mode, is 'property' the
>>> name of a struct or enum member, as in foo.bar?  Or is it something
>>> else?
>>
>> It's the 'bar' in 'foo.bar', yes. Specifically in the cases where it's
>> being looked up, rather than defined (in a type definition). Examples:
>>
>> 	  it2.lnum_pixel_width = it.lnum_pixel_width;
>>
>> 	  || (it.bidi_p && it.bidi_it.scan_dir == -1
>>
>> 'lnum_pixel_width', 'bidi_p', 'bidi_it' and 'scan_dir' are highlighted
>> with font-lock-property-face. You can see it for yourself by trying
>> c-ts-mode in any of our files.
> 
> Then as far as I'm concerned, this can go to level 4, but it must be
> done consistently across all the *-ts modes.  So if some mode wants
> 'property' to be highlighted, and wants it badly, we should IMO keep
> it in C as well.

Consistency is what I'm after here.

c-ts-mode, as well as go-ts-mode, rust-ts-mode and typescript-ts-mode, 
all previously mentioned in this report, currently put it at 3.

The rest put it as 4, or don't use it at all.

Maybe we'll move them to level 3 for the next release; I think to do 
that, we will have to define separate faces first (for function calls 
and maybe variable references, as soon as we manage to find them in a 
smart way), so that the users will be able to make sure definitions are 
distinct from usages.

> In any case, please make this consistent across all the relevant
> modes, and don't forget adjusting the documentation of
> treesit-font-lock-level accordingly as needed.

Okay.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03 15:15           ` Dmitry Gutov
@ 2023-02-03 15:54             ` Eli Zaretskii
  2023-02-03 17:10               ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-03 15:54 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, casouri, theo, dev

> Date: Fri, 3 Feb 2023 17:15:05 +0200
> Cc: 61205@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no, dev@rjt.dev
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> > Then as far as I'm concerned, this can go to level 4, but it must be
> > done consistently across all the *-ts modes.  So if some mode wants
> > 'property' to be highlighted, and wants it badly, we should IMO keep
> > it in C as well.
> 
> Consistency is what I'm after here.
> 
> c-ts-mode, as well as go-ts-mode, rust-ts-mode and typescript-ts-mode, 
> all previously mentioned in this report, currently put it at 3.
> 
> The rest put it as 4, or don't use it at all.

The question is: how important is this for go-ts-mode, rust-ts-mode,
and typescript-ts-mode?  I don't know the answer.  If the importance
is not high, then this should be moved to level 4.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03 15:54             ` Eli Zaretskii
@ 2023-02-03 17:10               ` Dmitry Gutov
  2023-02-04  3:36                 ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-03 17:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61205, casouri, theo, dev

On 03/02/2023 17:54, Eli Zaretskii wrote:
>> Date: Fri, 3 Feb 2023 17:15:05 +0200
>> Cc:61205@debbugs.gnu.org,casouri@gmail.com,theo@thornhill.no,dev@rjt.dev
>> From: Dmitry Gutov<dgutov@yandex.ru>
>>
>>> Then as far as I'm concerned, this can go to level 4, but it must be
>>> done consistently across all the *-ts modes.  So if some mode wants
>>> 'property' to be highlighted, and wants it badly, we should IMO keep
>>> it in C as well.
>> Consistency is what I'm after here.
>>
>> c-ts-mode, as well as go-ts-mode, rust-ts-mode and typescript-ts-mode,
>> all previously mentioned in this report, currently put it at 3.
>>
>> The rest put it as 4, or don't use it at all.
> The question is: how important is this for go-ts-mode, rust-ts-mode,
> and typescript-ts-mode?  I don't know the answer.  If the importance
> is not high, then this should be moved to level 4.

Right. They don't seem to be particularly more important there than in 
other modes. Or than 'function', for example.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-03 17:10               ` Dmitry Gutov
@ 2023-02-04  3:36                 ` Dmitry Gutov
  2023-02-04  6:53                   ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-04  3:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61205, casouri, theo, dev

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

On 03/02/2023 19:10, Dmitry Gutov wrote:
> On 03/02/2023 17:54, Eli Zaretskii wrote:
>>> Date: Fri, 3 Feb 2023 17:15:05 +0200
>>> Cc:61205@debbugs.gnu.org,casouri@gmail.com,theo@thornhill.no,dev@rjt.dev
>>> From: Dmitry Gutov<dgutov@yandex.ru>
>>>
>>>> Then as far as I'm concerned, this can go to level 4, but it must be
>>>> done consistently across all the *-ts modes.  So if some mode wants
>>>> 'property' to be highlighted, and wants it badly, we should IMO keep
>>>> it in C as well.
>>> Consistency is what I'm after here.
>>>
>>> c-ts-mode, as well as go-ts-mode, rust-ts-mode and typescript-ts-mode,
>>> all previously mentioned in this report, currently put it at 3.
>>>
>>> The rest put it as 4, or don't use it at all.
>> The question is: how important is this for go-ts-mode, rust-ts-mode,
>> and typescript-ts-mode?  I don't know the answer.  If the importance
>> is not high, then this should be moved to level 4.
> 
> Right. They don't seem to be particularly more important there than in 
> other modes. Or than 'function', for example.

Here's the updated patch in the meantime.

Not sure what to do with 'type' highlighting in rust-ts-mode yet. 
Additional scoping seems like will require a bunch of repetitions. 
Perhaps a :pred instruction to filter out children of a call_expression 
might work better.

[-- Attachment #2: ts-modes-refine-features.diff --]
[-- Type: text/x-patch, Size: 8039 bytes --]

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 97732b65e32..a86c12a0db7 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -1159,11 +1159,11 @@ Parser-based Font Lock
 This level adds fontification of keywords, strings, and data types.
 @item Level 3
 This is the default level; it adds fontification of assignments,
-numbers, properties, etc.
+numbers, etc.
 @item Level 4
 This level adds everything else that can be fontified: operators,
 delimiters, brackets, other punctuation, function names in function
-calls, variables, etc.
+calls, property look ups, variables, etc.
 @end table
 
 @vindex treesit-font-lock-feature-list
diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 390f67a8e8c..206b2e98fb3 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -774,8 +774,8 @@ c-ts-base-mode
   (setq-local treesit-font-lock-feature-list
               '(( comment definition)
                 ( keyword preprocessor string type)
-                ( assignment constant escape-sequence label literal property )
-                ( bracket delimiter error function operator variable))))
+                ( assignment constant escape-sequence label literal)
+                ( bracket delimiter error function operator property variable))))
 
 ;;;###autoload
 (define-derived-mode c-ts-mode c-ts-base-mode "C"
diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index 5f3e1ea3e68..a4b64808ca2 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -119,17 +119,27 @@ go-ts-mode--font-lock-settings
    :feature 'delimiter
    '((["," "." ";" ":"]) @font-lock-delimiter-face)
 
+   :language 'go
+   :feature 'definition
+   '((function_declaration
+      name: (identifier) @font-lock-function-name-face)
+     (method_declaration
+      name: (field_identifier) @font-lock-function-name-face)
+     (method_spec
+      name: (field_identifier) @font-lock-function-name-face))
+
+   :language 'go
+   :feature 'definition
+   '((field_declaration
+      name: (field_identifier) @font-lock-property-face))
+
    :language 'go
    :feature 'function
    '((call_expression
       function: (identifier) @font-lock-function-name-face)
      (call_expression
       function: (selector_expression
-                 field: (field_identifier) @font-lock-function-name-face))
-     (function_declaration
-      name: (identifier) @font-lock-function-name-face)
-     (method_declaration
-      name: (field_identifier) @font-lock-function-name-face))
+                 field: (field_identifier) @font-lock-function-name-face)))
 
    :language 'go
    :feature 'keyword
@@ -217,11 +227,10 @@ go-ts-mode
     ;; Font-lock.
     (setq-local treesit-font-lock-settings go-ts-mode--font-lock-settings)
     (setq-local treesit-font-lock-feature-list
-                '(( comment)
+                '(( comment definition)
                   ( keyword string type)
-                  ( constant escape-sequence function label number
-                    property variable)
-                  ( bracket delimiter error operator)))
+                  ( constant escape-sequence label number)
+                  ( bracket delimiter error function operator property variable)))
 
     (treesit-major-mode-setup)))
 
diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index e317793d211..662c286716b 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -155,6 +155,16 @@ rust-ts-mode--font-lock-settings
    :feature 'delimiter
    '((["," "." ";" ":" "::"]) @font-lock-delimiter-face)
 
+   :language 'rust
+   :feature 'definition
+   '((function_item name: (identifier) @font-lock-function-name-face)
+     (macro_definition "macro_rules!" @font-lock-constant-face)
+     (macro_definition (identifier) @font-lock-preprocessor-face))
+
+   :language 'rust
+   :feature 'definition
+   '((field_declaration name: (field_identifier) @font-lock-property-face))
+
    :language 'rust
    :feature 'function
    '((call_expression
@@ -164,15 +174,12 @@ rust-ts-mode--font-lock-settings
         field: (field_identifier) @font-lock-function-name-face)
        (scoped_identifier
         name: (identifier) @font-lock-function-name-face)])
-     (function_item (identifier) @font-lock-function-name-face)
      (generic_function
       function: [(identifier) @font-lock-function-name-face
                  (field_expression
                   field: (field_identifier) @font-lock-function-name-face)
                  (scoped_identifier
                   name: (identifier) @font-lock-function-name-face)])
-     (macro_definition "macro_rules!" @font-lock-constant-face)
-     (macro_definition (identifier) @font-lock-preprocessor-face)
      (macro_invocation macro: (identifier) @font-lock-preprocessor-face))
 
    :language 'rust
@@ -208,7 +215,6 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     (scoped_identifier name: (identifier) @font-lock-type-face)
      (scoped_identifier path: (identifier) @font-lock-constant-face)
      (scoped_identifier
       (scoped_identifier
@@ -317,11 +323,11 @@ rust-ts-mode
     ;; Font-lock.
     (setq-local treesit-font-lock-settings rust-ts-mode--font-lock-settings)
     (setq-local treesit-font-lock-feature-list
-                '(( comment)
+                '(( comment definition)
                   ( keyword string)
                   ( attribute builtin constant escape-sequence
-                    function number property type variable)
-                  ( bracket delimiter error operator)))
+                    number type)
+                  ( bracket delimiter error function operator property variable)))
 
     ;; Imenu.
     (setq-local treesit-simple-imenu-settings
diff --git a/test/manual/etags/rs-src/test.rs b/test/manual/etags/rs-src/test.rs
index 081d0d7d4df..06cde0e79e2 100644
--- a/test/manual/etags/rs-src/test.rs
+++ b/test/manual/etags/rs-src/test.rs
@@ -1,5 +1,10 @@
 mod test;
 
+use std::collections::hash_map::{self, HashMap};
+
+use std::path::{self, Path, PathBuf};  // good: std is a crate name
+use crate::foo::baz::foobaz;    // good: foo is at the root of the crate
+
 enum IpAddrKind {
     V4,
     V6,
@@ -12,3 +17,69 @@ fn test1() {
 fn main() {
    test::test1();
 }
+
+fn eat_box_i32(boxed_i32: Box<i32>) {
+    println!("Destroying box that contains {}", boxed_i32);
+}
+
+// This function borrows an i32
+fn borrow_i32(borrowed_i32: &i32) {
+    println!("This int is: {}", borrowed_i32);
+}
+
+struct Val {
+    val: f64,
+}
+
+struct GenVal<T> {
+    gen_val: T,
+}
+
+// impl of Val
+impl Val {
+    fn value(&self) -> &f64 {
+        &self.val
+    }
+}
+
+// impl of GenVal for a generic type `T`
+impl<T> GenVal<T> {
+    fn value(&self) -> &T {
+        &self.gen_val
+    }
+}
+
+fn main() {
+    let x = Val { val: 3.0 };
+    let y = GenVal { gen_val: 3i32 };
+
+    println!("{}, {}", x.value(), y.value());
+}
+
+fn main() {
+    // Create a boxed i32, and a stacked i32
+    let boxed_i32 = Box::new(5_i32);
+    let stacked_i32 = 6_i32;
+
+    // Borrow the contents of the box. Ownership is not taken,
+    // so the contents can be borrowed again.
+    borrow_i32(&boxed_i32);
+    borrow_i32(&stacked_i32);
+
+    {
+        // Take a reference to the data contained inside the box
+        let _ref_to_i32: &i32 = &boxed_i32;
+
+        // Error!
+        // Can't destroy `boxed_i32` while the inner value is borrowed later in scope.
+        eat_box_i32(boxed_i32);
+        // FIXME ^ Comment out this line
+
+        // Attempt to borrow `_ref_to_i32` after inner value is destroyed
+        borrow_i32(_ref_to_i32);
+        // `_ref_to_i32` goes out of scope and is no longer borrowed.
+    }
+
+    // `boxed_i32` can now give up ownership to `eat_box` and be destroyed
+    eat_box_i32(boxed_i32);
+}

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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-04  3:36                 ` Dmitry Gutov
@ 2023-02-04  6:53                   ` Eli Zaretskii
  2023-02-04 23:44                     ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-04  6:53 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, casouri, theo, dev

> Date: Sat, 4 Feb 2023 05:36:15 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: 61205@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no, dev@rjt.dev
> 
> Here's the updated patch in the meantime.

Thanks, but please also update the doc string of
treesit-font-lock-level.

> Not sure what to do with 'type' highlighting in rust-ts-mode yet. 

What is the problem with that?





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-04  6:53                   ` Eli Zaretskii
@ 2023-02-04 23:44                     ` Dmitry Gutov
  2023-02-05  6:05                       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-04 23:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61205, casouri, theo, dev

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

On 04/02/2023 08:53, Eli Zaretskii wrote:
>> Date: Sat, 4 Feb 2023 05:36:15 +0200
>> From: Dmitry Gutov<dgutov@yandex.ru>
>> Cc:61205@debbugs.gnu.org,casouri@gmail.com,theo@thornhill.no,dev@rjt.dev
>>
>> Here's the updated patch in the meantime.
> Thanks, but please also update the doc string of
> treesit-font-lock-level.

Done.

>> Not sure what to do with 'type' highlighting in rust-ts-mode yet.
> What is the problem with that?

The nodes structure of a 'use' instruction has a lot of nesting, and at 
least a couple of variations, which would lead to a combinatoric 
increase in the number of queries.

Taking another look at the declarations, though, I wasn't sure I could 
understand the specific logic for choosing between font-lock-type-face 
and font-lock-constant-face.

It seemed heavily inspired by 
https://github.com/tree-sitter/tree-sitter-rust/blob/master/queries/highlights.scm, 
though. So what I did is reverted to those rules (in that area): the 
path segments that start with an uppercase char get highlighted with 
font-lock-type-face. The rest don't get highlighted at all. That's how 
Rust code looks at Github, so a fair number of developers must be okay 
with it.

(Github also highlights function calls, though.)

I'm also adding function parameter highlighting to rust and go modes.

See the attached patch. I suggest we install it in emacs-29, but then 
people are free to tweak the rules further.

[-- Attachment #2: ts-modes-refine-features.diff --]
[-- Type: text/x-patch, Size: 7959 bytes --]

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 97732b65e32..a86c12a0db7 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -1159,11 +1159,11 @@ Parser-based Font Lock
 This level adds fontification of keywords, strings, and data types.
 @item Level 3
 This is the default level; it adds fontification of assignments,
-numbers, properties, etc.
+numbers, etc.
 @item Level 4
 This level adds everything else that can be fontified: operators,
 delimiters, brackets, other punctuation, function names in function
-calls, variables, etc.
+calls, property look ups, variables, etc.
 @end table
 
 @vindex treesit-font-lock-feature-list
diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 2a164af26ea..7300074e5c6 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -774,8 +774,8 @@ c-ts-base-mode
   (setq-local treesit-font-lock-feature-list
               '(( comment definition)
                 ( keyword preprocessor string type)
-                ( assignment constant escape-sequence label literal property )
-                ( bracket delimiter error function operator variable))))
+                ( assignment constant escape-sequence label literal)
+                ( bracket delimiter error function operator property variable))))
 
 ;;;###autoload
 (define-derived-mode c-ts-mode c-ts-base-mode "C"
diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index 95dcf653fc6..4b14e55281e 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -123,17 +123,26 @@ go-ts-mode--font-lock-settings
    :feature 'delimiter
    '((["," "." ";" ":"]) @font-lock-delimiter-face)
 
+   :language 'go
+   :feature 'definition
+   '((function_declaration
+      name: (identifier) @font-lock-function-name-face)
+     (method_declaration
+      name: (field_identifier) @font-lock-function-name-face)
+     (method_spec
+      name: (field_identifier) @font-lock-function-name-face)
+     (field_declaration
+      name: (field_identifier) @font-lock-property-face)
+     (parameter_declaration
+      name: (identifier) @font-lock-variable-name-face))
+
    :language 'go
    :feature 'function
    '((call_expression
       function: (identifier) @font-lock-function-name-face)
      (call_expression
       function: (selector_expression
-                 field: (field_identifier) @font-lock-function-name-face))
-     (function_declaration
-      name: (identifier) @font-lock-function-name-face)
-     (method_declaration
-      name: (field_identifier) @font-lock-function-name-face))
+                 field: (field_identifier) @font-lock-function-name-face)))
 
    :language 'go
    :feature 'keyword
@@ -221,11 +230,10 @@ go-ts-mode
     ;; Font-lock.
     (setq-local treesit-font-lock-settings go-ts-mode--font-lock-settings)
     (setq-local treesit-font-lock-feature-list
-                '(( comment)
+                '(( comment definition)
                   ( keyword string type)
-                  ( constant escape-sequence function label number
-                    property variable)
-                  ( bracket delimiter error operator)))
+                  ( constant escape-sequence label number)
+                  ( bracket delimiter error function operator property variable)))
 
     (treesit-major-mode-setup)))
 
diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index e317793d211..5722d037bba 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -155,6 +155,16 @@ rust-ts-mode--font-lock-settings
    :feature 'delimiter
    '((["," "." ";" ":" "::"]) @font-lock-delimiter-face)
 
+   :language 'rust
+   :feature 'definition
+   '((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)
+     (parameter pattern: (identifier) @font-lock-variable-name-face)
+     (parameter
+      pattern: (reference_pattern (identifier) @font-lock-variable-name-face)))
+
    :language 'rust
    :feature 'function
    '((call_expression
@@ -164,15 +174,12 @@ rust-ts-mode--font-lock-settings
         field: (field_identifier) @font-lock-function-name-face)
        (scoped_identifier
         name: (identifier) @font-lock-function-name-face)])
-     (function_item (identifier) @font-lock-function-name-face)
      (generic_function
       function: [(identifier) @font-lock-function-name-face
                  (field_expression
                   field: (field_identifier) @font-lock-function-name-face)
                  (scoped_identifier
                   name: (identifier) @font-lock-function-name-face)])
-     (macro_definition "macro_rules!" @font-lock-constant-face)
-     (macro_definition (identifier) @font-lock-preprocessor-face)
      (macro_invocation macro: (identifier) @font-lock-preprocessor-face))
 
    :language 'rust
@@ -208,20 +215,20 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     (scoped_identifier name: (identifier) @font-lock-type-face)
-     (scoped_identifier path: (identifier) @font-lock-constant-face)
-     (scoped_identifier
-      (scoped_identifier
-       path: (identifier) @font-lock-constant-face))
+     ((scoped_identifier name: (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     ((scoped_identifier path: (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     ((scoped_identifier
+        (scoped_identifier
+         path: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
      ((scoped_identifier
        path: [(identifier) @font-lock-type-face
               (scoped_identifier
                name: (identifier) @font-lock-type-face)])
       (:match "^[A-Z]" @font-lock-type-face))
      (scoped_type_identifier path: (identifier) @font-lock-constant-face)
-     (scoped_use_list
-      path: [(identifier) @font-lock-constant-face
-             (scoped_identifier (identifier) @font-lock-constant-face)])
      (type_identifier) @font-lock-type-face
      (use_as_clause alias: (identifier) @font-lock-type-face)
      (use_list (identifier) @font-lock-type-face))
@@ -317,11 +324,11 @@ rust-ts-mode
     ;; Font-lock.
     (setq-local treesit-font-lock-settings rust-ts-mode--font-lock-settings)
     (setq-local treesit-font-lock-feature-list
-                '(( comment)
+                '(( comment definition)
                   ( keyword string)
                   ( attribute builtin constant escape-sequence
-                    function number property type variable)
-                  ( bracket delimiter error operator)))
+                    number type)
+                  ( bracket delimiter error function operator property variable)))
 
     ;; Imenu.
     (setq-local treesit-simple-imenu-settings
diff --git a/lisp/treesit.el b/lisp/treesit.el
index 98f446a1456..2562e1286c7 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -585,9 +585,10 @@ treesit-font-lock-level
 Level 1 usually contains only comments and definitions.
 Level 2 usually adds keywords, strings, data types, etc.
 Level 3 usually represents full-blown fontifications, including
-assignments, constants, numbers and literals, properties, etc.
+assignments, constants, numbers and literals, etc.
 Level 4 adds everything else that can be fontified: delimiters,
-operators, brackets, punctuation, all functions and variables, etc.
+operators, brackets, punctuation, all functions, properties,
+variables, etc.
 
 In addition to the decoration level, individual features can be
 turned on/off by calling `treesit-font-lock-recompute-features'.

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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-04 23:44                     ` Dmitry Gutov
@ 2023-02-05  6:05                       ` Eli Zaretskii
  2023-02-05 13:52                         ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-02-05  6:05 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 61205, casouri, theo, dev

> Date: Sun, 5 Feb 2023 01:44:40 +0200
> Cc: 61205@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no, dev@rjt.dev
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 04/02/2023 08:53, Eli Zaretskii wrote:
> >> Date: Sat, 4 Feb 2023 05:36:15 +0200
> >> From: Dmitry Gutov<dgutov@yandex.ru>
> >> Cc:61205@debbugs.gnu.org,casouri@gmail.com,theo@thornhill.no,dev@rjt.dev
> >>
> >> Here's the updated patch in the meantime.
> > Thanks, but please also update the doc string of
> > treesit-font-lock-level.
> 
> Done.

Thanks, LGTM.

> >> Not sure what to do with 'type' highlighting in rust-ts-mode yet.
> > What is the problem with that?
> 
> The nodes structure of a 'use' instruction has a lot of nesting, and at 
> least a couple of variations, which would lead to a combinatoric 
> increase in the number of queries.

If this proves to be a problem in practice, maybe we'll need some
customization specific to Rust.

> Taking another look at the declarations, though, I wasn't sure I could 
> understand the specific logic for choosing between font-lock-type-face 
> and font-lock-constant-face.
> 
> It seemed heavily inspired by 
> https://github.com/tree-sitter/tree-sitter-rust/blob/master/queries/highlights.scm, 
> though. So what I did is reverted to those rules (in that area): the 
> path segments that start with an uppercase char get highlighted with 
> font-lock-type-face. The rest don't get highlighted at all. That's how 
> Rust code looks at Github, so a fair number of developers must be okay 
> with it.

And this solves the potential combinatoric explosion?

> See the attached patch. I suggest we install it in emacs-29, but then 
> people are free to tweak the rules further.

Yes, please install.





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

* bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list
  2023-02-05  6:05                       ` Eli Zaretskii
@ 2023-02-05 13:52                         ` Dmitry Gutov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2023-02-05 13:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dev, casouri, 61205-done, theo

On 05/02/2023 08:05, Eli Zaretskii wrote:
>> Date: Sun, 5 Feb 2023 01:44:40 +0200
>> Cc: 61205@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no, dev@rjt.dev
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> On 04/02/2023 08:53, Eli Zaretskii wrote:
>>>> Date: Sat, 4 Feb 2023 05:36:15 +0200
>>>> From: Dmitry Gutov<dgutov@yandex.ru>
>>>> Cc:61205@debbugs.gnu.org,casouri@gmail.com,theo@thornhill.no,dev@rjt.dev
>>>>
>>>> Here's the updated patch in the meantime.
>>> Thanks, but please also update the doc string of
>>> treesit-font-lock-level.
>>
>> Done.
> 
> Thanks, LGTM.
> 
>>>> Not sure what to do with 'type' highlighting in rust-ts-mode yet.
>>> What is the problem with that?
>>
>> The nodes structure of a 'use' instruction has a lot of nesting, and at
>> least a couple of variations, which would lead to a combinatoric
>> increase in the number of queries.
> 
> If this proves to be a problem in practice, maybe we'll need some
> customization specific to Rust.
> 
>> Taking another look at the declarations, though, I wasn't sure I could
>> understand the specific logic for choosing between font-lock-type-face
>> and font-lock-constant-face.
>>
>> It seemed heavily inspired by
>> https://github.com/tree-sitter/tree-sitter-rust/blob/master/queries/highlights.scm,
>> though. So what I did is reverted to those rules (in that area): the
>> path segments that start with an uppercase char get highlighted with
>> font-lock-type-face. The rest don't get highlighted at all. That's how
>> Rust code looks at Github, so a fair number of developers must be okay
>> with it.
> 
> And this solves the potential combinatoric explosion?

Yes, simply because function names don't start with a capital letter by 
convention. So we don't highlight them as types.

>> See the attached patch. I suggest we install it in emacs-29, but then
>> people are free to tweak the rules further.
> 
> Yes, please install.

Thank you, done, and closing.





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

end of thread, other threads:[~2023-02-05 13:52 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-01  2:08 bug#61205: 'function' in 3rd element of treesit-font-lock-feature-list Dmitry Gutov
2023-02-01  5:18 ` Yuan Fu
2023-02-02  2:34   ` Dmitry Gutov
2023-02-02  3:18     ` Randy Taylor
2023-02-02 11:03       ` Dmitry Gutov
2023-02-02 20:25   ` Dmitry Gutov
2023-02-03  2:38     ` Yuan Fu
2023-02-03  2:51       ` Dmitry Gutov
2023-02-03  6:45         ` Eli Zaretskii
2023-02-03  6:46     ` Eli Zaretskii
2023-02-03 11:42       ` Dmitry Gutov
2023-02-03 12:19         ` Eli Zaretskii
2023-02-03 15:15           ` Dmitry Gutov
2023-02-03 15:54             ` Eli Zaretskii
2023-02-03 17:10               ` Dmitry Gutov
2023-02-04  3:36                 ` Dmitry Gutov
2023-02-04  6:53                   ` Eli Zaretskii
2023-02-04 23:44                     ` Dmitry Gutov
2023-02-05  6:05                       ` Eli Zaretskii
2023-02-05 13:52                         ` Dmitry Gutov
2023-02-02  2:34 ` Randy Taylor
2023-02-02  2:44   ` Dmitry Gutov
2023-02-02  3:29     ` Randy Taylor
2023-02-02 11:11       ` Dmitry Gutov

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).