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 issues 63+101 #130

Merged
merged 5 commits into from Jan 24, 2019

Conversation

Projects
None yet
2 participants
@ntwrick
Copy link
Contributor

ntwrick commented Jan 24, 2019

Hi Alex, for your consideration...

In issues #63 and #101, the root problem with strings that contain escaped control characters lies in the way the lexer parses the strings into tokens. In Go and Python, CR, LF, and TAB are replaced with their ASCII control codes only when they are created from literal strings such as "\n". If we read in the characters '\' and 'n' as a sequence of bytes from IO and just convert them into a string they are not automatically expanded into an ASCII control code. Therefore, we have to explicitly do the conversion when the string token is created in lexer.readString(). This also affects how split() and join() work when matching the delimiter.

The second problem is that when lexer.readString() is handling double escaped quotes it also creates a side effect on any other double escaped sequence where it removes the double escape. Thus both '\' '\' 'n' and '\' 'n' always comes out as "\n".

So, based on our earlier discussion this PR implements escaped control character expansion into ASCII control codes in lexer.readString() only when the strings are double quoted while single quoted strings preserve the literal escape sequence. Also, double quoted strings may contain a mix of expanded and double escaped control characters. This also allows a = split("a\nb\nc", "\n"); join(a, "\n") to match the expanded LF control code while a = split('a\nb\nc', '\n');join(a, '\n') will match the literal string. Note that in the absence of control characters, double and single quoted strings are otherwise interchangeable.

You can see a use case for this model in the tests/test-strings.abs file where the test echo's the fully escaped string before it executes it as an unescaped string.

Also, I have fixed issues with the docs for error line location from the previous PR.

Rick

@odino

This comment has been minimized.

Copy link
Collaborator

odino commented Jan 24, 2019

This is amazing.

@odino

odino approved these changes Jan 24, 2019

Copy link
Collaborator

odino left a comment

@ntwrick thank you so much for this!

@@ -176,6 +176,11 @@ func getFns() map[string]*object.Builtin {
"echo": &object.Builtin{
Types: []string{},
Fn: func(args ...object.Object) object.Object {
if len(args) == 0 {
// allow echo() without crashing

This comment has been minimized.

@odino

odino Jan 24, 2019

Collaborator

👍

@odino odino changed the base branch from master to 1.0 Jan 24, 2019

@odino

This comment has been minimized.

Copy link
Collaborator

odino commented Jan 24, 2019

@ntwrick I'm going to add Go tests for this functionality just to make sure those are covered. Again, thanks!

BTW I'm now using version branches, so I changed the base branch of this PR to 1.0.

@odino odino merged commit aedc7f3 into abs-lang:1.0 Jan 24, 2019

@ntwrick

This comment has been minimized.

Copy link
Contributor Author

ntwrick commented Jan 24, 2019

@odino, you're welcome. Fun project...

@ntwrick ntwrick deleted the ntwrick:fix-issues-63+101 branch Jan 24, 2019

@odino

This comment has been minimized.

Copy link
Collaborator

odino commented Jan 24, 2019

@ntwrick ntwrick restored the ntwrick:fix-issues-63+101 branch Jan 24, 2019

@ntwrick ntwrick deleted the ntwrick:fix-issues-63+101 branch Jan 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment