Conversation
|
@gkorland I believe we can optimize even more but maybe it will be good to check if see that it makes a difference ... |
| } | ||
|
|
||
| match ch { | ||
| CH_DOLLA => self.dolla(pos, ch), |
There was a problem hiding this comment.
I was wondering about this DOLLA / dolla spelling... do you know if this is a typo or on purpose? maybe we should fix it?
There was a problem hiding this comment.
Not sure, this is how it is on the main repository..
| pub fn next_token(&mut self) -> Result<Token, TokenError> { | ||
| let (pos, ch) = self.input.next_char().map_err(to_token_error)?; | ||
| pub fn next_token(&mut self) -> Result<Token<'a>, TokenError> { | ||
| let (pos, ch) = self.input.peek_char().map_err(to_token_error)?; |
There was a problem hiding this comment.
I'm not sure about the logic, but are we changing behavior here? What are the situations in which we do peek instead of next as compared to the original?
There was a problem hiding this comment.
The old code was creating a new string. So it used next, saw the $, took the string after, and added it to char $. Now we do not want to compose a new string but return a slice that points to the original string, so we peek the next char, advance if needed (if its not $), and if it is $ we take the entire slice (including the $).
| pub fn peek_char(&self) -> Result<(usize, char), ReaderError> { | ||
| let ch = self.input.chars().next().ok_or(ReaderError::Eof)?; | ||
| Ok((self.pos + ch.len_utf8(), ch)) | ||
| Ok((self.pos, ch)) |
There was a problem hiding this comment.
This is a bug fix regardless of using slices, right?
Avoiding advancing the position when peeking? (should be advanced on next_char and take_while)
Co-authored-by: Omer Shadmi <[email protected]>
No description provided.