Skip to content
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

Inline _identifier and _quoted_identifier #112

Merged

Conversation

DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented May 19, 2024

Requires DavisVaughan/r-tree-sitter#7 for its updated print method used in snapshot tests

tree-sitter/tree-sitter#3332 prevents us from being able to use ts_node_is_missing() in many places because our identifier node has internal "hidden" rules. While we wait for this to be fixed, we can workaround this by inlining those hidden rules, being careful about the fact that we still need a "terminal" rule that can be used for word.

I've changed from _identifier to identifier for use in word, but I don't think included quoted identifiers is going to make much of a difference for what word is used for (distinguishing keywords like if and function from identifiers)

@@ -122,7 +122,7 @@
"]" [(1, 4), (1, 5)]
)
)
close: "]]" [(1, 5), (1, 5)]
close: "]]" MISSING [(1, 5), (1, 5)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now show MISSING for anonymous and named nodes that have been interpolated by tree-sitter's error recovery (like they do in their cli output). This requires DavisVaughan/r-tree-sitter#7

Comment on lines +29 to +33
rhs: (identifier MISSING [(3, 0), (3, 0)])
)

Text
1 +
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the other places in this PR where MISSING now shows up would have worked even without the changes to the grammar in this PR, because every other case has to do with missing anonymous nodes.

This is a missing named node, and tree-sitter pretty much always seems to inject identifier here as the implied node type, thus causing the issue where prior to the grammar changes here, this would not have shown up as MISSING.

word: $ => $._identifier,
word: $ => $.identifier,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note _identifier -> identifier, which just includes quoted identifiers in the word check now, but I don't think that is going to matter much after reading this again https://tree-sitter.github.io/tree-sitter/creating-parsers#keyword-extraction

Comment on lines +522 to +532
identifier: $ => {
const _identifier = /[\p{XID_Start}._][\p{XID_Continue}.]*/;
const _quoted_identifier = /`((?:\\(.|\n))|[^`\\])*`/;

return token(
choice(
_identifier,
_quoted_identifier
)
)
},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavisVaughan DavisVaughan force-pushed the fix/workaround-missing-node-issue branch from 5065d2e to a607767 Compare June 10, 2024 17:25
@DavisVaughan DavisVaughan merged commit 2c46921 into r-lib:next Jun 10, 2024
7 checks passed
@DavisVaughan DavisVaughan deleted the fix/workaround-missing-node-issue branch June 10, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant