search

tree-sitter / tree-sitter-typescript

TypeScript grammar for tree-sitter
MIT License
360 stars 109 forks source link

Fix empty range for ternary question mark #215

Closed tom95 closed 2 years ago

tom95 commented 2 years ago

skip() was used after the qmark token has been read, which resets the start position of the token: https://github.com/tree-sitter/tree-sitter/blob/9df064c9fe31d35dc48244a6675588aee2d61a41/lib/src/lexer.c#L184

I also moved the mark_end earlier such that the qmark token does not include trailing whitespace in its range.

One could argue that the behavior of advance(skip=true) in tree-sitter should be changed instead but I'm not sure about the implications for other external parsers. (E.g., make it so that the start_position is only moved up during advance(skip=true) if no mark_end call has occurred yet).

Fixes https://github.com/tree-sitter/tree-sitter-typescript/issues/200 As pointed out in the issue and also to my knowledge, there is currently no simple way to provide automated tests for this change, or is there?

Checklist:

carlosala commented 2 years ago

@maxbrunsfeld this solves #200 for me, in all cases I tested, and has no issues. Could we merge it? Thanks!

maxbrunsfeld commented 2 years ago

Yes, sorry for the delay. Approved this to run on CI.

maxbrunsfeld commented 2 years ago

As pointed out in the issue and also to my knowledge, there is currently no simple way to provide automated tests for this change, or is there?

There is no simple way, you're right. It would be good to allow for optional range assertions in the corpus tests, but I'm not sure when I'll get around to that.

Thank you for the fix.

carlosala commented 2 years ago

thanks Max!

unphased commented 1 year ago

Thank you for fixing this. I'm using tree-sitter to build some powerful code manipulation tools and there is no other project with as many languages supported. I also enjoy tree-sitter intensely inside neovim. Great job on the project!

I see this particular fix is a year old. The version v0.20.1 which is published to NPM is over a year old and predates this fix. I lost some time in the rabbit hole of trying to fix the ternary question mark operator positioning due to this bug. I should have had faith and checked for this upstream fix in the beginning. Anyway I am delighted to confirm that using "tree-sitter-typescript": "https://github.com/tree-sitter/tree-sitter-typescript.git#v0.20.2" in the package.json fixes the problem.

It would appear that my neovim setup automatically got the latest typescript grammars. maybe it is just an oversight that the latest has not been published on npm.

Is there anything we can do to get the new version published to npm? Even if that does not happen I am off to the races with the github link in my package.json.