-
Notifications
You must be signed in to change notification settings - Fork 62
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
Incorrect result of ts_node_first_child_for_pos in a heredoc_body #139
Comments
Thanks for the detailed report @casouri. |
Hello, I believe I have found a similar issue in tree-sitter-clojure: sogaiu/tree-sitter-clojure#32 I have based the reproduction recipe off of the one @casouri submitted. I'm not sure what we can do about it in the grammar itself. Now that it is seen here and in the clojure grammar I am wondering if it is a bug in tree-sitter itself. |
@casouri I continued investigating @dannyfreeman's sogaiu/tree-sitter-clojure#32 case and have turned up some bits that might be relevant to this issue. I've summarized a bit here. Update: On second thought, it might be better to disregard the above for now -- I'm not seeing a difference in debug graph output for this issue. Sorry for the false alarm. |
@casouri @maxbrunsfeld @dannyfreeman The current implementation of
IIUC, this implementation doesn't appear to handle this issue's case or that of sogaiu/tree-sitter-clojure#32 because there is some "climbing back up" that needs to happen (which doesn't). Here's the tail end of debug graph output I get for the source text in the sample code: In the figure above, there comes a time when the lower I tried replacing the current implementation of
AFAICT, with this alternative implementation, the sample code seems to behave better (though it may not be efficient if I take it that calling Here's the |
The current heredoc implementation ate the words |
it's because it's _heredoc_body_middle which is hidden, do we want to expose that as |
Yes we want, for all visible things there should be nodes in the tree. I think the Also the Between /cc: @amaanq |
In a following source:
The return value of
ts_node_first_child_for_byte(heredoc_body, 22)
, whereheredoc_body
is the heredoc_body node, and 22 is at the beginning of "text2", is expected to be the command_substitution node representing$(echo cmd)
. However, the function returns a null node.But if I evaluate
ts_node_first_child_for_byte(heredoc_body, 12)
, where 12 is at the beginning of "text1", the return value is expected: the simple_expansion representing$var
. So I don't know what could be the cause.Here is the program I used to test this:
Compiled using
If I run it, I get
I'm using the latest master branch of tree-sitter-bash and tree-sitter 0.20.7. TIA!
The text was updated successfully, but these errors were encountered: