Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(deps): bump tree-sitter and treesitter-c to 0.20.1 #16402

Merged
merged 3 commits into from Nov 23, 2021
Merged

build(deps): bump tree-sitter and treesitter-c to 0.20.1 #16402

merged 3 commits into from Nov 23, 2021

Conversation

clason
Copy link
Member

@clason clason commented Nov 22, 2021

This bumps tree-sitter from a fixed pre-release commit to v0.20.1, and similarly for treesitter-c (the bundled C parser).

Two tests had to be updated to the latest query API.

@clason clason requested a review from vigoux November 22, 2021 09:27
@github-actions github-actions bot added build building and installing Neovim using the provided scripts dependencies build dependencies (LuaJIT, LibUV, etc.) and removed dependencies build dependencies (LuaJIT, LibUV, etc.) labels Nov 22, 2021
@clason clason added dependencies build dependencies (LuaJIT, LibUV, etc.) treesitter labels Nov 22, 2021
@clason
Copy link
Member Author

clason commented Nov 22, 2021

Two test failures:

  • test/functional/treesitter/parser_spec.lua @ 230: treesitter parser API can match special regex characters like \ * + ( with vim-match?
  • test/functional/treesitter/parser_spec.lua @ 320: treesitter parser API allow loading query with escaped quotes and capture them with lua-match? and vim-match?

@clason clason changed the title build(deps): bump tree-sitter to 0.20.1 build(deps): bump tree-sitter and treesitter-c to 0.20.1 Nov 22, 2021
@theHamsta
Copy link
Member

theHamsta commented Nov 22, 2021

The tests fail because the pattern index changed (treesitter/parser_spec.lua)

      for pattern, match in cquery:iter_matches(tree:root(), 0) do
        -- can't transmit node over RPC. just check the name and range
        local mrepr = {}
        for cid,node in pairs(match) do
          table.insert(mrepr, {cquery.captures[cid], node:type(), node:range()})
        end
        table.insert(res, {pattern, mrepr})

Probably the pattern index should be removed in the last line of both tests. Sorry, for adding it in the first place!

@clason
Copy link
Member Author

clason commented Nov 22, 2021

Thanks for looking at this!

Probably the pattern index should be removed in the last line of both tests. Sorry, for adding it in the first place!

I don't know anything about these tests, so a very explicit change suggestion would be helpful. Do you mean just removing the line table.insert(res, {pattern, mrepr}), or something else?

@theHamsta
Copy link
Member

theHamsta commented Nov 22, 2021

Sorry, there seems to be a real problem here

Passed in:
  (table: 0x7fb6c0b62aa8) { }

Thought at first sight that only the indices were switched. Can look into this tonight.

@clason
Copy link
Member Author

clason commented Nov 22, 2021

Yep, looks like some nodes are just missing (the second failing test makes that clearer): quote, paren, plus, times

But string and escape are there and correct.

@theHamsta
Copy link
Member

theHamsta commented Nov 22, 2021

Pretty strange. I did a clean build and the tests are failing the same way as on CI. But tree-sitter itself seems to be working normally and also the exact queries from the test still seem to be working.

Apparently, we're loosing all anonymous in the tests (does (_) really capture anonymous nodes or do we have to use something else @stsewd you know more about wild cards, right?). Related?: tree-sitter/tree-sitter#1496

@theHamsta
Copy link
Member

This continues to work

❯ tree-sitter query foo.scm bar.c
Warning: You have not configured any parser directories!
Please run `tree-sitter init-config` and edit the resulting
configuration file to indicate where we should look for
language grammars.

bar.c
  pattern: 0
    capture: quote, start: (0, 9), text: "\""
  pattern: 0
    capture: quote, start: (0, 15), text: "\""

tree-sitter-c on  master [?] is 📦 v0.20.1 via  v17.0.1 via 🦀 v1.58.0-nightly 
❯ cat foo.scm
((_) @quote (match? @quote "^\"$"))

tree-sitter-c on  master [?] is 📦 v0.20.1 via  v17.0.1 via 🦀 v1.58.0-nightly 
❯ cat bar.c
#include "bar.h"

@stsewd
Copy link
Contributor

stsewd commented Nov 23, 2021

Apparently, we're loosing all anonymous in the tests (does (_) really capture anonymous nodes or do we have to use something else @stsewd you know more about wild cards, right?). Related?: tree-sitter/tree-sitter#1496

(_) matches all named nodes, and _ matches named and anonymous nodes.

@theHamsta
Copy link
Member

theHamsta commented Nov 23, 2021

But using _ will result in a syntax error. I tried that before. Maybe one would have to put the #match statements in a different place then. Can you fix the tests?

@stsewd
Copy link
Contributor

stsewd commented Nov 23, 2021

So, these queries

cquery = vim.treesitter.parse_query("c", '((_) @plus (vim-match? @plus "^\\\\+$"))'..
should be written as

([_] @plus (#vim-match? @plus "^\\\\+$"))

That way we are able to match any anonymous nodes, otherwise (_ @plus) is invalid or (_) will only match named nodes. Also, we should use the new syntax for predicates (prefixing with #), the other syntax is deprecated.

@clason
Copy link
Member Author

clason commented Nov 23, 2021

Looks like changing all (_) to [_] for these tests did the trick, thanks @stsewd !

(I can still change to the new syntax if somebody tells me exactly what to write ;))

@stsewd
Copy link
Contributor

stsewd commented Nov 23, 2021

(I can still change to the new syntax if somebody tells me exactly what to write ;))

the other syntax update would be to change vim-match? and friends to #vim-match? (pre-fixing #), isn't a problem now, but that syntax could be removed in the future.

Change query to include anonymous nodes (`(_)` -> `[_]`) and
use new syntax (`{vim,lua}.match?`->`#{vim,lua}.match?`)
@clason
Copy link
Member Author

clason commented Nov 23, 2021

Like so?

@stsewd
Copy link
Contributor

stsewd commented Nov 23, 2021

@clason yes, that's it!

@clason
Copy link
Member Author

clason commented Nov 23, 2021

@stsewd Will this bump have ramifications for nvim-treesitter? Will those queries need to be updated, too?

@clason
Copy link
Member Author

clason commented Nov 23, 2021

All checks pass -- even Windows! Now we have to merge ;)

@stsewd
Copy link
Contributor

stsewd commented Nov 23, 2021

@stsewd Will this bump have ramifications for nvim-treesitter? Will those queries need to be updated, too?

AFAIK we don't have queries like those, but if we did, it should be easy to fix. (I run the nightly version of tree-sitter locally, and haven't seen any errors)

@clason clason merged commit dd8a4e2 into neovim:master Nov 23, 2021
@clason clason deleted the treesitter-bump branch November 23, 2021 19:13
@dundargoc dundargoc removed the request for review from vigoux October 29, 2022 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build building and installing Neovim using the provided scripts dependencies build dependencies (LuaJIT, LibUV, etc.) treesitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants