unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Randy Taylor <dev@rjt.dev>
To: jostein@kjonigsen.net
Cc: eliz@gnu.org, 61302@debbugs.gnu.org, Dmitry Gutov <dgutov@yandex.ru>
Subject: bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties
Date: Mon, 13 Feb 2023 18:19:02 +0000	[thread overview]
Message-ID: <qlg23kr-yHWoiqRfk6u8D7MD6j1snsSp1XCL6Onj8e_ou9qU7Vhg0ijOy2mwT_E5EkrdM7-NI1hJ07eEh0Lvyj8jBlhGa1VryBmpkXa1vdA=@rjt.dev> (raw)
In-Reply-To: <7392c27d-0d5b-e3e9-ccd0-3d4bf329986c@secure.kjonigsen.net>

On Monday, February 13th, 2023 at 10:04, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
>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.

Fair enough. Personally, I wouldn't mind a simpler/less complete bug report if it's clearly stated that one didn't have enough time to fully detail it. That way I can at least try to fix it and if not, no harm, we just wait until the reporter has time to give more information.

>>
>> I have been comparing to rust-mode, both now and when I was making
>> rust-ts-mode.
>Ok. Sorry for missing it. My bad :)

No worries, you didn't miss anything. The exchange between Dmitry and I was simply that I was curious about how other editors and IDEs (especially ones with tree-sitter support like neovim) are highlighting these things, hence why rust-mode was not brought up.

>>
>> 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!

Thanks :).

>>
>> >    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?

Sorry about that, I forgot to reply to that. Hopefully I didn't miss anything else in this reply!
Yes, font-lock-preprocessor-face is more appropriate - I will include that change when I post my next patch.

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

Yup, that seems to be the case. I did see your other report that was also macro-related, I just didn't have anything to add to it.
Although whether or not the grammar is capable of anything more here I have no idea.
Like C/C++, whenever macros are involved, trouble arrives it seems.

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

I don't see why we can't be consistent about it, but it's just that the variable and function features themselves are, to my understanding, meant to highlight all of that stuff fully. Then, the assignment, definition, and whatever smaller features can be used for folks who want more specific and "quieter" highlights. I never added those smaller ones into any of my modes because I want my code looking like a wild disco.

>
>>
>> >    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 did notice that macro-related stuff had issues, but that's because of parser limitations.
Aside from macros, I think everything else is fontified fine and much better than rust-mode (pending my patch fixing up imports).

>
>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? :)

I just don't see why any of those issues should stop it from being included in emacs-29 - none of them are showstoppers, and nothing is forcing people to use rust-ts-mode.

I expect lots of bug reports for all the ts modes once emacs-29 comes out :D.

>
>--
>Jostein





  reply	other threads:[~2023-02-13 18:19 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-05 20:15 bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties Jostein Kjønigsen
2023-02-05 21:30 ` Randy Taylor
2023-02-05 21:52   ` Jostein Kjønigsen
2023-02-05 21:59     ` Jostein Kjønigsen
2023-02-06  1:50       ` Randy Taylor
2023-02-06  2:45         ` Dmitry Gutov
2023-02-06  2:57           ` Randy Taylor
2023-02-07 14:26           ` Randy Taylor
2023-02-07 18:16             ` Dmitry Gutov
2023-02-07 18:25               ` Dmitry Gutov
2023-02-08  3:38                 ` Randy Taylor
2023-02-08 15:44                   ` Dmitry Gutov
2023-02-09  3:38                     ` Randy Taylor
2023-02-09 21:19                       ` Dmitry Gutov
2023-02-10  3:44                         ` Randy Taylor
     [not found]                           ` <33cec9a6-7e69-2eb3-a8a6-58ce23a5c185@yandex.ru>
2023-02-12  2:48                             ` Randy Taylor
2023-02-13  3:37                               ` Dmitry Gutov
2023-02-14  3:25                                 ` Randy Taylor
2023-02-14 11:42                                   ` Jostein Kjønigsen
2023-02-14 12:39                                     ` Randy Taylor
2023-02-14 14:28                                       ` Jostein Kjønigsen
2023-02-14 22:14                                       ` Dmitry Gutov
2023-02-15  2:07                                         ` Randy Taylor
2023-02-16  1:53                                           ` Dmitry Gutov
2023-02-18  3:27                                             ` Dmitry Gutov
2023-02-18 20:42                                               ` Randy Taylor
2023-02-18 21:45                                                 ` Dmitry Gutov
2023-02-18 23:31                                                   ` Randy Taylor
2023-02-19  0:13                                                     ` Dmitry Gutov
2023-02-19  0:50                                                       ` Randy Taylor
2023-02-19 17:23                                                         ` Dmitry Gutov
2023-02-18 20:59                                               ` Dmitry Gutov
2023-02-13 10:17                           ` Jostein Kjønigsen
2023-02-13 14:39                             ` Randy Taylor
2023-02-13 15:04                               ` Jostein Kjønigsen
2023-02-13 18:19                                 ` Randy Taylor [this message]
2023-02-13 19:57                               ` Dmitry Gutov
2023-02-13 20:49                                 ` Dmitry Gutov
2023-02-13 19:59                             ` Dmitry Gutov
2023-02-05 21:56   ` Dmitry Gutov
2023-02-06  2:06     ` Randy Taylor
2023-02-06  2:16       ` Dmitry Gutov
2023-02-05 21:44 ` Dmitry Gutov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='qlg23kr-yHWoiqRfk6u8D7MD6j1snsSp1XCL6Onj8e_ou9qU7Vhg0ijOy2mwT_E5EkrdM7-NI1hJ07eEh0Lvyj8jBlhGa1VryBmpkXa1vdA=@rjt.dev' \
    --to=dev@rjt.dev \
    --cc=61302@debbugs.gnu.org \
    --cc=dgutov@yandex.ru \
    --cc=eliz@gnu.org \
    --cc=jostein@kjonigsen.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).