More trivia changes - #220
Conversation
| } | ||
|
|
||
| #[must_use] | ||
| pub fn splice_trivia<R, I>(&self, side: TriviaSide, range: R, replace_with: I) -> GreenToken |
There was a problem hiding this comment.
I'm hesitant to offer such API because new rowan cannot have it; in it, the only way to create a new node or token is a from-scratch builder. So if you want this to aid with the migration I'll happily merge it, but it cannot be the final form.
| /// Returns a [`SyntaxNodePtr`] for the node. | ||
| pub fn new(node: &SyntaxNode<L>) -> Self { | ||
| Self { kind: node.kind(), range: node.text_range() } | ||
| Self { kind: node.kind(), range: node.text_range_without_outer_trivia() } |
There was a problem hiding this comment.
Why is SyntaxNodePtr, of all things, using this method? It could maybe be useful for other things but SyntaxNodePtr only purpose in life is locating nodes; the range doesn't matter as long as it's unique, so why choose the less natural range?
| self.raw.index() | ||
| } | ||
|
|
||
| pub fn child_index(&self) -> Option<usize> { |
There was a problem hiding this comment.
We should probably remove index() and call this index() instead. What index() does with trivia is highly non-natural and I wouldn't even want to commit to it.
| self.raw.last_token().map(SyntaxToken::from) | ||
| } | ||
|
|
||
| pub fn next_structural_token(&self) -> Option<SyntaxToken<L>> { |
There was a problem hiding this comment.
"Structural" isn't really descriptive. I suggest [next/prev]_non_trivia_token(). It's long but crystal clear.
|
|
||
| pub fn first_token(&self) -> Option<SyntaxToken> { | ||
| self.first_child_or_token()?.first_token() | ||
| self.children_with_tokens().find_map(|child| child.first_token()) |
There was a problem hiding this comment.
This doesn't include trivia, right? I missed this in the previous PR but we might want it to do.
There was a problem hiding this comment.
Nope, we have new set of APIs now.
| (None, Some(right)) => !right.is_trivia() && right.prev_structural_token().is_none(), | ||
| (None, None) => false, | ||
| }; | ||
| adjacent.then(|| { |
There was a problem hiding this comment.
Oh this only works if they are adjacent. So at least give it a name like trivia_between_adjacent(), and consider making it assert they're adjacent.
There was a problem hiding this comment.
This was horrible. I have new trivia_before and trivia after APIs exposed.
eddf3c3 to
a0b80bf
Compare
2 participants
No description provided.