-
Notifications
You must be signed in to change notification settings - Fork 602
Fix dollar quoted string tokenizer #1193
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
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.
Thanks for this contribution @ZacJW
Can you please add some tests illustrating the behavior of this change (and so we don't accidentally break it in a future refactor).
Please mark it as ready for review when it is ready for another look
Thanks again and I am sorry for the delay
Pull Request Test Coverage Report for Build 8623596368Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
I've added four unit tests:
In writing the last one I noticed that the error message is different. Before my change there were four different error messages about the string being unterminated. Was this deliberate so an error could be tracked back to the site that created it? |
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.
Thank you again for this contribution @ZacJW -- the test cases really help and the code looks 👌
Currently the tokenizer is written in a way that assumes that once it's seen the starting delimiter (the first
$...$
) the next $ it sees must be the start of the ending delimiter, but this needn't be the case. This change checks to see if what might be the end delimiter actually matches, and if it doesn't considers it part of the string and keeps searching for the end.