-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
[WIP] Fix global strategy highlighting #132
base: master
Are you sure you want to change the base?
Conversation
The new global strategy looks good to me, I played around with something similar and it generally worked fine, but I wanted to play around with making a blocks query for python and couldn't do that without something more complicated. I have a few comments for problems I have noted:
-- we get id's in the order they show up in the query file, which
-- will usually be
local delimiter_id, sentinel_id, container_id = 1, 2, 3
-- this will fix any mistaken order
for id, name in ipairs(query.captures) do
-- captures should only contain at most these three
if name == 'delimiter' then
delimiter_id = id
elseif name == 'sentinel' then
sentinel_id = id
elseif name == 'container' then
container_id = id
end
end And then compare e.g. |
5def45d
to
2b96c6e
Compare
I think the problem with Python is that the body of a statement is not sandwiched between two parent delimiters. Let's take the following piece of Python: if True:
print([{'a': {'b', 'c'}}]) The full tree is
I would like to highlight the
There is no matching delimiter to sandwich The problem is definitely lack of a closing delimiter. If I make both the This is definitely something I would like to get working. I want rainbow blocks in Python as well, but until now that was impossible because of the lack of a sentinel node. But now that we no longer need sentinels it should be doable. |
2b96c6e
to
30b85a4
Compare
I now know why sandwiching is required: the algorithm to determine nesting assumes that matches are returned in a certain order which is outlined in the HACKING file (when exiting a node according to a depth-first visitor pattern). This assumption only holds true if child matches are sandwiched between delimiters. Otherwise the match will be returned upon entering the node. This means I need to modify the nesting algorithm to account for the reverse case. I have updated the HACKING file with the current state of knowledge so far. |
30b85a4
to
bb53e86
Compare
OK, I give up on Python for the time being. Trying to get rainbow-blocks working for Python has given me some deeper insight though, so I do think that it is doable in the future, but the priority is getting this fix out of the door. A my findings are documented in the HACKING file. The problem is that right now I am assuming that for two matches if (some_var) {
# ...
} Right now we highlight the parentheses and braces the same, which makes sense because visually they are all siblings. However, in the syntax tree the block delimited by the braces is a child of the whole if-statement, and so it would get highlighted differently from the parentheses. I could define a
Now only what is inside the parentheses is considered a child, but the block with braces is a sibling (or more generally a cousin). The same for Python:
Here we have two bodies: whatever is between for (k, v) in {'a': 1, 'b': 2}:
print(k, v) Here |
Can you please provide some examples so I can replicate it? |
I have found one broken edge case: when pasting multiple lines with delimiters only the last line will have the proper highlighting, the other lines will be off by one per how many lines they are above the last line. Take the following example: print {
{a = 1, b = 2},
} Now type I have added a test case. I anyone finds more edge cases please post them here. |
baac83c
to
efda387
Compare
Pasting multiple lines should be fixed now. |
The test currently fails.
Previously I was abusing stacks as sets, but this was hard to read because it was unclear whether the stacks were actually used for their stack properties or just as a makeshift set. In addition to that stacks do not have the idempotence of sets, which as not an issue in practice, but it could have caused bugs.
There is nothing I can do until this issue is resolved: elm-tooling/tree-sitter-elm#159 The problem is that because the parenthesized pattern does not have a container of its own the nesting cannot be determined properly.
There are now two global strategies and the correct one gets picked at runtime based on the version of Neovim.
efda387
to
bd1ae1e
Compare
bd1ae1e
to
a026d4a
Compare
Sorry for the late reply, I have been on vacation and then after that I was quite busy for some time. I am not sure exactly what you have looked at since your last messages. Is there any of the problems I mentioned I should still try to find a way to reproduce? Also just a note: To get around the issue with not having sentinels (e.g. in Python), I used the end (line,column) of the node for queries with no sentinel provided, and that worked fine. I could generally get good highlighting in Python (and other languages) with that, I just used a really hacky solution to get around the problems you mentioned in Python, so that should definitely be considered a bit more thoroughly at some point, if we would want something like that to work. |
The best thing to do would be just to use the current state as it is and report if you find some issue. I have already fixed one issue (outlined above), but I have noticed a different one. Take this Lua code: print {
{
}
{
}
} The execute
I don't want to spend time on it until after this PR. I don't think we can make it work without an explicit The first priority is finishing this PR. The global strategy is in a good enough shape, the next step is porting over the local strategy and that should be it then. |
I am opening this PR for the sake of transparency. This PR is my attempt at fixing the broken highlighting caused by Neovim 0.10, it involves a rewrite of the core highlighting algorithm to use
iter_matches
rather thaniter_captures
now that the bug that makeiter_matches
unworkable has been fixed in Neovim.As of the time of writing this all tests pass, but that does not mean that I don't have any blind spots in my tests. Now is the time to actually try it out. Rewriting the highlighting has revealed a number of bugs, so of which are not even visible, in the old code, so there have been quite a few commits to
master
as well. Some patterns had to be dropped unfortunately because they never worked correctly to begin with (usually because nested parentheses would be on the same level in the tree).I have also found the previous highlighting code to be lacking in structure and hard to understand. I was making new data structure types up as I went along and I was abusing stacks as ad-hoc sets. Thus I used this opportunity to introduce a proper set type and properly define what a match and what a match tree are. These definitions will probably change before I merge this branch into
master
though.@Danielkonge Thank you for the time you have invested and the PRs you have opened, but I think this one obsoletes them.