-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix behaviour of matching when tree-sitter grammar is available #7238
Conversation
If the current character is a valid bracket, resort to plaintext matching. Otherwise, use regular tree-sitter node traversal.
doc: &Rope, | ||
cursor_pos: usize, | ||
) -> Option<usize> { | ||
pub fn find_matching_bracket_plaintext(doc: &Rope, cursor_pos: usize) -> Option<usize> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😓 even after having so much looks into it, and fixing the docs, I missed that the function name was left saying it worked on "current_line". So I'm addressing it here, removing mention of only working on the current line.
… we are not on top of the boundary of the node *and* we have a valid pair in the node start/ending bytes.
The original proposed solution is quite simple, just look at the first commit: d0eae33 It could be argued that this simple solution is wasteful in case that we are on top of a valid bracket, and we are right on the boundary of the node, and this node boundaries are valid bracket pairs. Under such case, we would be resorting to plaintext matching, when we could achieve the matching much faster by looking at the node positions. So... I have pushed a commit that would address those concerns: 1fb75bb Either option works, and I have no strong feelings reverting the second commit, if you think the optimisation is an overkill. |
This is actually the opposite direction of what I was looking to do. TS is generally preferable to text matching because it handles edge cases much better. I want to move miX and maX to TS too, see #6893 (review). The TS matching just needed some improvements. I had these in mind but never got around to it but I nave now implemented them in #7242. That PR also contains an (intentionally) very limited plaintext fallgack to handle... Mostly horrible plaintext macros #3357 (comment)... I might undo that tough we could handle those cases by injecting C into C preprocessor tokens the way nvim does |
@pascalkuthe oh, and you have it implemented already. Let me close this one, yours is looking much better and keeping in on TS. |
If the current character is a valid bracket, resort to plaintext matching. Otherwise, use regular tree-sitter node traversal.
This is a pragmatic solution that I propose, as addressing the tree-sitter grammars seems way more complex and tedious.
Why is believe this is acceptable? If the user is not on top of a valid bracket, regular operation will take place. It simply deals with the edge cases where the user is on top of a bracket, which brings the unexpected behaviour that users complain about (see #3357, #5177)