Update to global strategy (draft)

[Danielkonge]

I am generally happy with the global strategy, but the issue mentioned in https://github.com/neovim/neovim/issues/27296 sometimes annoys me, so I have made this somewhat hacky solution to the problem.

I don't really like this solution, but I can't see a really good way to do this correctly with the current treesitter implementation. Do you have any ideas for how to do this better?

Here are some comparison videos:

New code: https://github.com/HiPhish/rainbow-delimiters.nvim/assets/7075380/78dc2dac-5058-438e-ba02-725db7c4d620

Old code: https://github.com/HiPhish/rainbow-delimiters.nvim/assets/7075380/9df1530c-72e0-4611-93b4-e13ea582c2cf

Note: For now this builds on top of my other pull request, so only the last commit is really part of this pull request.

[HiPhish]

Can you please rebase on the current master? I see the three commits from your previous PR included in this PR. I have only been on 0.10 for a couple of days now in which I have not done any real programming, so I haven't noticed anything wrong. This is definitely a problem that I will have to look into.

[Danielkonge]

I have rebased on master now.

This is not a problem I notice that much, but in certain cases it will show up. The problem is that we get containers without their delimiters and sentinels, because of the range bounds in iter_captures (where we used to get everything). The problem with e.g.

local tmp = {
    [1] = { 1 },
    [2] = {
        a = print()
    }
}

is that we get the container from [2] without anything letting us know that we finish it. (Note that we don't get the [1] line, which is good, but we still run into problems because of the [2] container.)

What this PR does (in a somewhat hacky way) is add back in information about the sentinels of the containers that we care about (given the range).

[HiPhish]

I have added a test so we can track that the bug remains fixed as we tinker with the code. You can run make e2e-test to run all tests. Testing requires busted to be installed and all submodules to be checked out. Please let me know if you have trouble running the tests, I have only run them locally on my machine so far.

[HiPhish]

@Danielkonge Sorry for the long wait, but I have not had time to properly look into this PR until now.

If I recall correctly we rewrote the algorithm in #43 to use Query:iter_captures instead of Query:iter_matches because iter_matches would only return the last node of each capture. So if we had multiple @delimiter captures we would only get the last one when iterating over the nodes of a captures. If I understand correctly this should be fixed in 0.10 thanks to all = true, which will become the default in subsequent Neovim releases.

Would it then be possible to rewrite the algorithm to use iter_matches again? As it stands now you are iterating through the tree twice, once with iter_matches starting at the root, and then later with iter_queries. I wonder if we would even need the @sentinel nodes anymore when using iter_matches.

[Danielkonge]

If I recall correctly we rewrote the algorithm in #43 to use Query:iter_captures instead of Query:iter_matches because iter_matches would only return the last node of each capture. So if we had multiple @delimiter captures we would only get the last one when iterating over the nodes of a captures. If I understand correctly this should be fixed in 0.10 thanks to all = true, which will become the default in subsequent Neovim releases.

Yes, the previous problem with iter_matches should be fixed now.

Would it then be possible to rewrite the algorithm to use iter_matches again? As it stands now you are iterating through the tree twice, once with iter_matches starting at the root, and then later with iter_queries. I wonder if we would even need the @sentinel nodes anymore when using iter_matches.

We should be able to go back to iter_matches, and I think we can do without the @sentinel nodes too (though we might want to keep them in since they allow for some control that you might not have without them). I can probably make a draft of a rewrite using iter_matches during this week.

[HiPhish]

I think we would do well to have automated tests for highlighting this time to ensure that our highlighting is the same as before. I'll try to come up with a solution, although I will probably commit it to master because it is not related to this PR.

We already have test files, so I was thinking of placing a JSON file next to each test file which contains a specification of what highlighting we expect at which location. There would be only one actual busted test file which will loop through all directories, read the JSON specification, load the test file into a buffer and then compare the specification to the actual highlighting. That way we do not have to write a separate test for each language.

Yes, the previous problem with iter_matches should be fixed now.

In that case we can keep the old implementation of the global strategy as is for Neovim versions under 0.10. We can have a runtime version check that will load either the new or the old implementation.

-- The file 'rainbow-delimiters/strategy/global.lua' contains just the check
if has('nvim-0.10') then
    return require 'rainbow-delimiters/strategy/global-new'
else
    return require 'rainbow-delimiters/strategy/global-legacy'
end

We should be able to go back to iter_matches, and I think we can do without the @sentinel nodes too (though we might want to keep them in since they allow for some control that you might not have without them).

There is no harm in keeping them, so let's leave them in for now.