On 2/13/23 15:39, Randy Taylor wrote:
> In the future, it would've been nice to have bug reports filed for 
> these regardless.
> Especially since I had time last week to sink in to fixing these 
> problems...
I'll try to keep that in my for the future. But registering "high 
quality" bug-reports takes some effort too, and I feel like just rushing 
them because -I- lack time may come off as impolite.
>
> I have been comparing to rust-mode, both now and when I was making 
> rust-ts-mode.
Ok. Sorry for missing it. My bad :)
>
> Thanks. I wish you also would've included the code as text so I didn't 
> have to type it all out :).
Indeed. Where -are- my manners? Attached is the original source file!
>
> >    rust-mode: consistently fontify annotations (notice they are missing in rust-ts-mode, line 12 and 14). Also rust-mode use 
> font-lock-preprocessor-face, which I think as a more appropriate face 
> for this kind of syntax, than font-lock-constant-face (used in 
> rust-ts-mode).

Since this wasn't really mentioned in the reply... Any chance we can 
give font-lock-preprocessor-face some love too? Or should I make that a 
bug of its own?

> >    some code does not seem to get fontified at all (types, keywords, 
> etc). Line 14-17.
>
> Did you look at that with treesit-explore-mode?
> It's inside a macro invocation which mostly consists of token_trees.
> Not much helpful stuff for us to go on to highlight.

Like I said. Might need to be fixed upstream in the tree-sitter rust 
grammars.

I guess it seems like the rust-grammar in general could use some 
improvements...

>
> >    it seems to fontify all variables using font-lock-variable-name-face all over, regardless of it is a 
> declaration or not. I realize this is not 100% consistent throughout 
> the Emacs-verse, but I know other -ts-modes have aimed for declaration 
> only, and so does rust-mode from MELPA too (although with some 
> consistency-issues) which this would be replacing.
>
> Because that's what the variable feature is supposed to do, same as 
> the function feature.
> Perhaps rust-ts-mode's definition feature can be augmented to support 
> that (and also note it's missing an assignment feature that some other 
> modes have).

Right. Like I said, the Emacs-verse is not really 100% consistent about 
that, and I doubt it ever will be.

Personally I was asked to remove such fontification when submitting 
changes/improvements to typescrip-ts-mode and csharp-ts-mode... And in 
the end I think I like the results more that way, and just assumed this 
was supposed to be the standard.

Oh well.

>
> >    it does not seem to handle ::* imports properly? See line 9. The way I understand it, things preceeding the 
> ::* should be considered a namespace too?
>
> Correct, I will fix that as part of my next patch I post.
> Before, we weren't distinguishing imports and general scope 
> identifiers which caused a lot of inconsistencies. Now we do, so it's 
> just a matter of rounding up all the import-related queries.
Sounds great!
>
> >    I know imports are difficult to be 100% accurate about, as seen in this thread. Are we importing a module or 
> a class? Impossible to tell without looking at the referenced code! 
> Aiming for visual consistency may be a better goal than 100% 
> correctness, if the AST we're getting don't provide good enough 
> information? (This has been done in other modes too)
>
> That is what we're trying to do. I believe the patch I posted earlier 
> in the thread covers most of these cases, minus the wildcard one you 
> mentioned (which I will post a new patch addressing sometime later 
> today). If you notice any others, please shout.
Sure thing.
>
> I don't understand this. So there may be a few things missing or a few 
> inconsistencies - so what? People can make bug reports for them and 
> they can be fixed. rust-mode itself has inconsistencies and 
> correctness issues as well, and other ts modes do too (like 
> c/c++-ts-modes WRT macros).

I don't know. My WASM code may not have been the most typical rust code 
and as such may not serve as a great baseline for fontification?

I just assumed if it would be much work to fix up, it might be hard to 
make the required fixes in time.

If you (as one of the main implementers) disagree, who am I to argue? :)

--
Jostein