alamb commented on code in PR #1618:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1618#discussion_r1896991320
##########
src/parser/mod.rs:
##########
@@ -3641,22 +3670,31 @@ impl<'a> Parser<'a> {
#[must_use]
pub fn parse_keyword(&mut self, expected: Keyword) -> bool {
if self.peek_keyword(expected) {
- self.next_token_ref();
+ self.advance_token();
Review Comment:
I think this is nicer too as it makes clear the token is unused (it is just
being advanced)
##########
src/parser/mod.rs:
##########
@@ -1276,7 +1276,9 @@ impl<'a> Parser<'a> {
let dialect = self.dialect;
- let (next_token, next_token_index) = self.next_token_ref_with_index();
+ self.advance_token();
Review Comment:
This has two benefits:
1. I think it is more explicit
2. It doesn't not result in a `mut` borrow of `self`(this is the key
difference)
##########
src/parser/mod.rs:
##########
@@ -3557,45 +3564,67 @@ impl<'a> Parser<'a> {
///
/// See [`Self::next_token_ref`] to avoid the copy.
pub fn next_token(&mut self) -> TokenWithSpan {
- self.next_token_ref().clone()
+ self.advance_token();
+ self.get_current_token().clone()
}
- pub fn next_token_ref(&mut self) -> &TokenWithSpan {
- self.next_token_ref_with_index().0
+ /// Returns the index of the current token
+ ///
+ /// This can be used with APIs that expect an index, such as
+ /// [`Self::token_at`]
+ pub fn get_current_index(&self) -> usize {
+ self.index.saturating_sub(1)
}
- /// Return the first non-whitespace token that has not yet been processed
- /// and that tokens index and advances the tokens
+ /// Return the next unprocessed token, possibly whitespace.
+ pub fn next_token_no_skip(&mut self) -> Option<&TokenWithSpan> {
+ self.index += 1;
+ self.tokens.get(self.index - 1)
+ }
+
+ /// Advances the current token to the next non-whitespace token
///
- /// # Notes:
- /// OK to call repeatedly after reaching EOF.
- pub fn next_token_ref_with_index(&mut self) -> (&TokenWithSpan, usize) {
Review Comment:
Note thse APIs were added in
https://github.com/apache/datafusion-sqlparser-rs/pull/1587 (so not yet
released) so this isn't a breaking API change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
