-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Ruby: Add heredoc literals #2885
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.
Thank you for the PR @sj26!
It looks mostly good as is. I left you a few comments and please commit the minified files as well (run npm ci && npm run build
to generate those).
Thanks @RunDevelopment, I've removed the test suite change and expanded the heredoc support to more forms. It's not perfect, but it works well enough. I wasn't able to run
Maybe because I'm on an M1 mac? |
Yes but don't worry. According to nodejs/node#37061, this issue has been fixed in v15.9.0. You just need to update your NodeJS version. |
I have no idea why the tests fail. Everything looks fine to me. I'll investigate this tomorrow. |
Fixed the test problem. Now they fail, as they should, and give a proper error message. Let's look at the error message:
As we can see, an attacker could exploit this pattern's polynomial backtracking behavior. I'll give suggestions to fix this problem. |
Thanks @RunDevelopment, I've folded those changes in 👍 |
Please rebuild @sj26 and then we can merge this. |
Adds support for heredoc-style string literals in the Ruby syntax. https://ruby-doc.org/core-2.7.0/doc/syntax/literals_rdoc.html#label-Here+Documents+-28heredocs-29 This isn't perfect, but it's a good start.
Sorry I forgot to rebuild, but done now 👍 |
Thank you for contributing @sj26! |
This is a first pass at adding heredoc string literals for Ruby:
https://ruby-doc.org/core-2.7.0/doc/syntax/literals_rdoc.html#label-Here+Documents+-28heredocs-29
This only adds the basic form:
There are plenty of other styles with slightly different forms not yet covered by this PR: