Skip to content
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

replaced lexerStream with text/scanner.Scanner #71

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

replaced lexerStream with text/scanner.Scanner #71

wants to merge 3 commits into from

Conversation

generikvault
Copy link

Hi,

I've replaced the lexerStream type with the golang text/scanner.Scanner type. My commit causes syntax changes with string escaping. Now there is a real difference between single and double quotes.

'abc" is no longer a vailid string. In exchange "abc's", 'a"b"c' and '\u263a' are valid now.

JanErik Keller added 3 commits September 12, 2017 16:32
featurechanges:
Don't escape single quotes in double qiates: Use "abc's" instead of "abc\'s"
Added Support for new quote `abc` with unescapable content
Added Support for Unicode number codes '\u263a'
@Knetic Knetic self-assigned this Sep 13, 2017
@Knetic
Copy link
Owner

Knetic commented Sep 13, 2017

I'm still going over the change, but all the tests are passing and i think the change to quoting is fine - it always felt like a bug to allow something like 'abc" to be read as a valid string.
So it seems like this change is all good, but i'm still going over what the scanner parses that the hand-rolled lexerStream didn't, and looking for any possible conflicts.

@Knetic
Copy link
Owner

Knetic commented Oct 22, 2017

This change is good, but I'm going to wait to merge until a new major version is ready - this is a big enough API break that it'll probably affect downstreams.

@generikvault
Copy link
Author

ok, cool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants