GNU bug report logs - #61302
29.0.60; rust-ts-mode does not show function-invocation on field-properties

Previous Next

Full log

View this message in rfc822 format

From: Randy Taylor <dev <at> rjt.dev>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: eliz <at> gnu.org, Jostein Kjønigsen <jostein <at> kjonigsen.net>, 61302 <at> debbugs.gnu.org
Subject: bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties
Date: Sun, 12 Feb 2023 02:48:59 +0000

On Friday, February 10th, 2023 at 17:50, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>On 10/02/2023 05:44, Randy Taylor wrote:
>> I believe so (with the exception of use declarations as you note).
>> Not familiar enough with Rust to say for sure :).
>
>So this is a discussion between two people who don't write Rust.
>
>Hmmm. :-)

You couldn't find anyone more qualified if you tried ;).

>
>>>>> So 'usize' in the above is definitely a "type", not a "module"?
>>>>
>>>> I think so. You can see on usize's documentation page (https://doc.rust-lang.org/std/primitive.usize.html)
>>>> that it provides that function, amongst many others.
>>>
>>> I was thinking that it might also be referring to (apparently
>>> deprecated) https://doc.rust-lang.org/std/usize/index.html.
>>
>> That module only provides the constants listed on that page.
>> The usize type itself provides a bunch of constants and functions (same for the rest of the primitives).
>>
>> I'm curious how other editors/IDEs highlight this stuff, but I don't have any on hand ATM.
>
>Here's some overview.

Thank you! It's quite comprehensive.

>
>I mentioned previously
>https://github.com/tree-sitter/tree-sitter-rust/blob/master/queries/highlights.scm,
>which apparently corresponds to how Github highlights Rust syntax. E.g.
>here:
>https://github.com/tree-sitter/tree-sitter-rust/blob/master/examples/ast.rs
>
>The capitalized identifiers are highlighted as "types", apparently, and
>the lowercase segments are not highlighted at all. For some reason the
>types are also not highlighted in variable and parameter declarations.
>That seems like a problem, FWIW.

Agreed. I guess they dumb it down for the web.

>
>If I press "edit in dev", navigating to
>https://github.dev/tree-sitter/tree-sitter-rust/blob/master/examples/ast.rs,
>that seems to open some webby version of VS Code, except with a
>different color theme.
>
>Some observations:
>
>- A lot more highlights.
>- The "modules" and the "Types" have the same color.

I like how neovim does it (which is basically how we're doing it, with separate highlights).

>- The identifiers at the end of a scope chain are not highlighted if
>they are a) lowercase and, b) not from a known set, apparently.

Right. We can do better here IMO, and highlight them regardless because we know what they should be (which is what my patch does).
The exception being modules which require a little more care.

>- So "use std::usize;" is highlighted and "use std::uuusizeee;" is not.
>- "use std::foo::usize;" is highlighted.
>
>This also matches with VS Code's behavior installed locally. Neither the
>"cloud" VS Code nor the local one use tree-sitter, IIUC.
>
>IntelliJ Rust doesn't highlight "modules" or types at all, AFAICT:
>https://intellij-rust.github.io/assets/intro_screen_editor.png
>And those guys usually write their own parsers, so the highlights are
>most likely parse tree based, just not with tree-sitter.
>
>>> Sorry, I'm not really familiar with Rust. E.g. in Ruby every class can
>>> also serve as a "module" in the scoping sense. As a result we highlight
>>> both classes and modules with font-lock-type-face. This could also be an
>>> option here, if everything else fails.
>>>
>>> But we could also highlight based on a "role" (constant if it's used as
>>> a scope, and type if it's used as a type).
>>>
>>> Although one could say that 'Path' in
>>>
>>>    Some(Path::new("./foo"))
>>>
>>> is being used as a type as well, and 'Some' too. So it might not be the
>>> best fit.
>>>
>>> Speaking of 'usize' again, what if some lib or the app defines an
>>> 'usize' module for its custom functions acting on it? E.g.
>>> 'my::app::usize'. A simple regexp matcher will probably highlight it as
>>> a type as well.
>>
>> I don't think we should worry about those cases IMO.
>
>Okay.
>
>>> Some problems from my testing:
>>>
>>> 1. If I keep treesit-font-lock-level at its default value (3), some
>>> stuff gets misfontified:
>>
>> Sorry, I have only been testing with level 4.
>> This is also why I want type and module combined into one so we don't have to deal with this headache.
>
>I'm not quite sure what's the best choice here (keeping in mind the
>problem with overreaching variable highlights on level 4), but
>logically, I think 'module' belongs with 'function' and 'property'
>because all three denote some basic syntactic elements which are easy to
>understand even without colors, and are harder to make a mistake in.

I am proposing to get rid of the module feature entirely and bring those queries into the type feature because:
- Of how much overlap there is between them
- It will make maintaining the queries much easier
  - It's already a headache dealing with them separately, and I'm not sure the best way to deal with the issues of them being separate (and the different levels of highlighting to worry about). It will probably be quite the hack to deal with it, and no matter what I tried stuff was always sneaking through.
- It also won't introduce that much more highlighting

>
>That is in contrast with highlighting of variable declarations, for
>example, which in Rust can use some non-trivial syntax, more prone to
>user error.
>
>>>    use std::collections::hash_map::{self, HashMap};
>>>
>>> 'hash_map' is highlighted as a type. 'HashMap' is not highlighted at all.
>>>
>>>    use std::{fmt, fs, usize};
>>>
>>> Only 'use' is highlighted here.
>>
>> This is because of how things are broken out into the module feature.
>> That some highlighting for those occurs is by overlap of queries in the type feature.
>> Which again is why I think module should be part of type.
>>
>>>
>>>    test::test1();
>>>
>>> 'test1' is highlighted as a type (we discussed this problem with
>>> highlighting types by default -- it becomes necessary to filter out
>>> function calls, either with more complex queries, or with custom
>>> highlighter functions).
>>
>> Right, I added a query to filter that out now.
>
>Thanks, that works now.
>
>>>
>>> 2. If I switch to treesit-font-lock-level 4:
>>>
>>>    let boxed_i32 = Box::new(5_i32);
>>>
>>> 'Box' is highlighted with font-lock-constant-face. I think it's a type,
>>> though.
>>
>> Oops, I accidentally removed the rule for that. Added it back.
>
>That too.
>
>>> Also here's a pre-existing problem mentioned above:
>>>
>>>    use std::{fmt, fs, usize};
>>>
>>> 'fmt' and 'fs' are not types. But they are highlighted with
>>> font-lock-type-face.
>>
>> This is really weird, I can reproduce it with emacs -Q but not with my normal config...
>> Also with emacs -Q this:
>> let date = DateTime::<chrono::hey::there::Utc>::from_utc(date, chrono::cool::this::Utc);
>>
>> highlights incorrectly, where "there" is font-lock-variable-name-face. But with my normal config everything is fine.
>>
>> I'll look into it tomorrow. Not really sure what in my config could cause this...
>
>Thank you.

I did a clean build and wasn't able to reproduce it anymore. Guess it was stale bytecode or something?
At level 4 everything highlights correctly I believe, and with level 3 the only issues are the module highlighting ones, and to deal with that I think the module feature should be merged into the type one as I mentioned above. Then we can call it a day :). I'll post a new patch with those changes if you agree.

Previous Next

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