-
Notifications
You must be signed in to change notification settings - Fork 45
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
fix(parser): fix some escape sequences in strings and indentation in block strings #633
Conversation
e89d64d
to
61bc870
Compare
…, fix \r in normal strings
61bc870
to
c90520d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
let line = &self.input[..index]; | ||
let rest = match self.input.get(index..=index + 1) { | ||
Some("\r\n") => &self.input[index + 2..], | ||
_ => &self.input[index + 1..], | ||
}; | ||
self.input = rest; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above is fine. It can be marginally nicer with split_at
and strip_prefix
:
let line = &self.input[..index]; | |
let rest = match self.input.get(index..=index + 1) { | |
Some("\r\n") => &self.input[index + 2..], | |
_ => &self.input[index + 1..], | |
}; | |
self.input = rest; | |
let (line, rest) = self.input.split_at(index); | |
// `rest` starts with \r or \n | |
self.input = rest.strip_prefix("\r\n").unwrap_or_else(|| &rest[1..]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my personal feeling is that split_at
looks nicer but the manual match
is more obviously correct despite being a bit noisy... though as i look back and forth i don't feel strongly anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\o/
Follow-up to #638, fixing conversion of StringValue nodes to Rust strings.
Fixes #609
Fixes #611
This handles
"
Incorrect parsing of string with trailing \" #611This does not handle these unicode escape issues: