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

Finish the \a escape sequence #6221

Merged
merged 3 commits into from Jun 22, 2018
Merged

Conversation

wooster0
Copy link
Contributor

No description provided.

@@ -231,8 +231,8 @@ describe "Lexer" do
it_lexes_number :i8, ["0i8", "0"]

it_lexes_char "'a'", 'a'
it_lexes_char "'\\a'", 7.chr
it_lexes_char "'\\b'", 8.chr
it_lexes_char "'\\a'", '\u{7}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not '\a'?

Copy link
Contributor Author

@wooster0 wooster0 Jun 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because p 7.chr gives '\u{7}'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@r00ster91 can you change the \u{7} to \a and also use \b in the following spec to keep it consistent with the rest of the suite.

@@ -231,8 +231,8 @@ describe "Lexer" do
it_lexes_number :i8, ["0i8", "0"]

it_lexes_char "'a'", 'a'
it_lexes_char "'\\a'", 7.chr
it_lexes_char "'\\b'", 8.chr
it_lexes_char "'\\a'", '\u{7}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@r00ster91 can you change the \u{7} to \a and also use \b in the following spec to keep it consistent with the rest of the suite.

@bcardiff
Copy link
Member

For reference, this is a follow up of #5864

@wooster0
Copy link
Contributor Author

Okay changed it.

@straight-shoota
Copy link
Member

You should also add \a and \b to Char#dump_or_inspect.

@wooster0
Copy link
Contributor Author

That Travis CI failed is just a runtime error I think

@bcardiff bcardiff added this to the 0.25.1 milestone Jun 22, 2018
@bcardiff bcardiff merged commit e806c95 into crystal-lang:master Jun 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants