HiPhish / rainbow-delimiters.nvim

Rainbow delimiters for Neovim with Tree-sitter
https://gitlab.com/HiPhish/rainbow-delimiters.nvim
Apache License 2.0
470 stars 35 forks source link

Add `rainbow-blocks` to `query` and allow query setup via a function #70

Closed Danielkonge closed 7 months ago

Danielkonge commented 7 months ago

Note

This PR doesn't depend on recent neovim updates, but it will be much more useful with the following:

Explanation

I looked at #68 and thought something like that might be nice for InspectTree, so I added the following:

Note: There is one small problem that doesn't have anything to do with this update, see neovim/neovim/issues/26325 .

Here is an image showcasing rainbow-delimiters for the query file and rainbow-blocks for InspectTree. [ Note: this uses neovim/neovim/pull/26304 ]

Screenshot 2023-11-29 at 21 30 19
HiPhish commented 7 months ago

Merged, thank you. I changed the example in the manual to use vim.bo.buftype == 'nofile' instead of the undocumented variable.

The rainbow-blocks query looks really nice in :InspectTree. I would not want to actually write queries like that, but it really does make the tree more readable when inspecting it. Pretty cool idea by @binhtran432k.

Danielkonge commented 7 months ago

Merged, thank you. I changed the example in the manual to use vim.bo.buftype == 'nofile' instead of the undocumented variable.

The rainbow-blocks query looks really nice in :InspectTree. I would not want to actually write queries like that, but it really does make the tree more readable when inspecting it. Pretty cool idea by @binhtran432k.

I am not sure if vim.bo.buftype == 'nofile is a good idea since that is also used in the query editor in InspectTree (press o after going into InspectTree), whereas vim.b.dev_base should only catch the InspectTree buffer itself. This is not that important though.

It should be fine for the docs with the way you wrote it.

HiPhish commented 7 months ago

You are right, but it's just an example, so I am not too concerned about it. It doesn't really matter though, both approaches have a major problem. Open an editable file and execute :InspectTree, the tree buffer will open with the correct highlighting. Now switch back to the editable buffer and make any change. The tree window will revert back to rainbow-delimiters (or whatever the query for the editable buffer is). That is because both b: variables and vim.bo options will be evaluated relative to the current buffer, not the buffer that is being updated.

I should be able to fix this easily with nvim_buf_call though.

HiPhish commented 7 months ago

It should work now. I also fixed up strategies and queries as well. But now that I think about it, maybe this is too magic? Perhaps it would have been better if the function was not a thunk, but had an explicit bufnr argument, and leave it up to the user whether he wants to use that argument or not.

Danielkonge commented 7 months ago

It should work now. I also fixed up strategies and queries as well. But now that I think about it, maybe this is too magic? Perhaps it would have been better if the function was not a thunk, but had an explicit bufnr argument, and leave it up to the user whether he wants to use that argument or not.

I think the current solution is good enough, but we could definitely also add an optional bufnr to the thunk function instead. Adding the bufnr option in the function might give slightly better control over the exact behavior, but I do think that the current solution should be enough for almost everyone and it gives the behavior one would usually expect, so I think it works fine this way too.

HiPhish commented 7 months ago

I'll give it a few days of thought, but right now I am leaning towards less magic and explicit function arguments. Either way, existing user configuration will not be affected because missing arguments in Lua just become nil and unused arguments are ignored.

Danielkonge commented 7 months ago

I'll give it a few days of thought, but right now I am leaning towards less magic and explicit function arguments. Either way, existing user configuration will not be affected because missing arguments in Lua just become nil and unused arguments are ignored.

Yeah, I don't think there will be any problems with this. Just make sure to note that the function takes bufnr: integer? instead of bufnr: integer as input in the type annotations. Otherwise LuaLS will start complaining on existing user configs. (Also, if you don't need the bufnr you probably would just want to write a function with no input, so I think this type annotation makes sense either way.)