nushell / tree-sitter-nu

A tree-sitter grammar for nu-lang, the language of nushell
MIT License
121 stars 27 forks source link

Fix issues with parameters and flags #144

Closed blindFS closed 1 week ago

blindFS commented 1 week ago

This PR fixes those parameter/flag related issues in #142

fdncred commented 1 week ago

thanks

melMass commented 1 week ago

I'm not sure if it's related to this PR but I'm now spammed with these in neovim:

image

EDIT: It's unrelated to this, updating treesitter solved the issue

blindFS commented 1 week ago

@melMass Yes, :TSUpdate nu is required since PR #143 changed highlights.scm file to support the new spread operators.

melMass commented 1 week ago

Thanks, it was something else in the neovim ts plugin and updating the plugin fixed the issue! I use TSUpdate as my build command so it's ran whenever I update nvim-treesitter (very often)

fdncred commented 1 week ago

I also created a similar issue https://github.com/nushell/tree-sitter-nu/issues/145 that I ran upon trying to test out melmass's report.

blindFS commented 1 week ago

@melMass Sorry, but I'd like to figure out the root cause here since other users may face the same problem. In my understanding, neovim ts plugin hosts a stand-alone copy of queries scheme files for nu and other languages. There's delay of updating, so it can cause inconsistency between the grammar and queries files. I don't see why updating neovim ts can solve the issue (their highlights.scm for nu is still out of date).

Would you please share your nvim config about this?

blindFS commented 1 week ago

@fdncred I'm afraid #145 is related to the inconsistency that I mentioned above.

fdncred commented 1 week ago

@blindFS How so? The queries are the same in this repo and the repo that nvim pulls down. Neither queries work.

Update: LOL, they're the same because I copied the queries over from here to my nvim-data folder. But they still didn't work until I commented the two items out that I mentioned.

melMass commented 1 week ago

I'd like to figure out the root cause

I think it's completely unrelated to this and just an upstream "bug" in nvim-treesitter. I realized that after having the same error message in a lua file (new session, no hidden nu buffers) which prompted me to update the plugin that fixed it in all languages

Would you please share your nvim config about this?

Oh wait I actually went a commit ealier while trying to solve the issue...:

{
    'nvim-treesitter/nvim-treesitter',
    build = ':TSUpdate',

    dependencies = {
      { 'nushell/tree-sitter-nu', commit = '45f9e51e5ee296dc0965a80f3d00178d985dffbd' }
}

I'll remove it now, retry and report how it went

melMass commented 1 week ago

Oh right, the issue is there my bad


stack traceback:
    [C]: in function '_ts_parse_query'
    ...0.10.2_1/share/nvim/runtime/lua/vim/treesitter/query.lua:252: in function 'fn'
    ...im/0.10.2_1/share/nvim/runtime/lua/vim/func/_memoize.lua:58: in function 'fn'
    ...im/0.10.2_1/share/nvim/runtime/lua/vim/func/_memoize.lua:58: in function 'get'
    ..._1/share/nvim/runtime/lua/vim/treesitter/highlighter.lua:28: in function 'new'
    ..._1/share/nvim/runtime/lua/vim/treesitter/highlighter.lua:243: in function 'get_query'
    ..._1/share/nvim/runtime/lua/vim/treesitter/highlighter.lua:191: in function 'fn'
    ...1/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:491: in function 'for_each_tree'
    ..._1/share/nvim/runtime/lua/vim/treesitter/highlighter.lua:178: in function 'prepare_highlight_states'
    ..._1/share/nvim/runtime/lua
Error in decoration provider treesitter/highlighter.win:
Error executing lua: ...0.10.2_1/share/nvim/runtime/lua/vim/treesitter/query.lua:252: Query error at 193:12. Impossible pattern:
(long_flag ["="] @punctuation.special)
           ^

I can provide more info as needed

blindFS commented 1 week ago

I think when you do :TSUpdate nu, it installs the grammar in neovim-ts's repo, which is older version than this. However your highlights.scm file is updated with this repo, which is newer. This can explain both of your problems.

melMass commented 1 week ago

I think when you do :TSUpdate nu, it installs the grammar in neovim-ts's repo, which is older version than this. However your highlights.scm file is updated with this repo, which is newer. This can explain both of your problems.

I use Lazy to manage it so it's possible yep, but this confirms the issues is introduced by this merge

blindFS commented 1 week ago

@melMass would you please try to remove the dependency here:

{
    'nvim-treesitter/nvim-treesitter',
    build = ':TSUpdate',
}

And also clean-up the tree-sitter-nu plugin, then try again.

melMass commented 1 week ago

@melMass would you please try to remove the dependency here: And also clean-up the tree-sitter-nu plugin, then try again.

Ohh I see what you meant, so it's now merged upstream and doesn't require the explicit dependency, this seems to work but I'm not sure how to know which "version" TS install on it's own.

I get why they do that but it would be so much simpler to point to the maintained repos instead of PRing updates "manually"

blindFS commented 1 week ago

@melMass I think it is specified in the lockfile https://github.com/nvim-treesitter/nvim-treesitter/blob/master/lockfile.json

melMass commented 1 week ago

Hmmm seems like it uses some old ones:

$ cd .local/share/nvim/lazy/nvim-treesitter/queries
$ ls nu/
╭───┬───────────────────┬──────┬────────┬─────────────╮
│ # │       name        │ type │  size  │  modified   │
├───┼───────────────────┼──────┼────────┼─────────────┤
│ 0 │ nu/indents.scm    │ file │  376 B │ 2 weeks ago │
│ 1 │ nu/highlights.scm │ file │ 5.6 KB │ 2 weeks ago │
│ 2 │ nu/injections.scm │ file │  219 B │ 2 weeks ago │
╰───┴───────────────────┴──────┴────────┴─────────────╯

Ah but it's expected.

Tbh I'm regularly confused by Treesitter even though I went quite deep into it at some point...

Shouldn't we still be able to use this repo to get the latest updates?

melMass commented 1 week ago

@melMass I think it is specified in the lockfile https://github.com/nvim-treesitter/nvim-treesitter/blob/master/lockfile.json

But that's "just" the parser, what I don't understand is why this PR doesn't work? A feature not yet in the treesitter version used in the plugin?

blindFS commented 1 week ago

@melMass Sure, but when you do :TSUpdate nu, you only install the locked version. If you want to use the latest grammar, I'm afraid that you have to use the method of my config as in #145

blindFS commented 1 week ago

This PR's update of highlights.scm only works with its corresponding grammar. If your grammar is some older version, the query of the "=" token in long_flag is simply invalid ("=" is in long_flag_equals_value which is removed in this PR).

melMass commented 1 week ago

I think I get it now, so the saner approach for me seems to accept the slight delay upstream and just use that! Thanks for the help debugging this, I'll know how to switch from upstream to latest in case I need to

blindFS commented 1 week ago

@melMass Yes, we will keep minimum updates to queries files so this kind of trouble won't happen often.