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

Don't use heredoc inside interpolation #5648

Conversation

makenowjust
Copy link
Contributor

A heredoc inside interpolation is buggy. For example, this code cannot be compiled even though it was valid Ruby code:

p <<-HERE1
  here1-1
  #{<<-HERE2}
    here2
  HERE2
  here1-2
HERE1

And this code is compiled even though it was invalid Ruby code:

p "#{<<-HERE}
"
hello
HERE
# => "hello\n"

I tried to fix these, however I did't complete it and I found it is a bit hard to fix.
A heredoc inside interpolation is very tricky, so I suggest forbidding this.

Thank you.

@makenowjust makenowjust force-pushed the fix/crystal/dont-use-heredoc-inside-interpolation branch from fd21faf to 697043e Compare January 29, 2018 13:58
@makenowjust
Copy link
Contributor Author

ping @asterite. Could you review this?

@asterite
Copy link
Member

@makenowjust no, sorry, I won't have time to review PRs related to the compiler. I'll just discuss issues and approve trivial PRs

@bew
Copy link
Contributor

bew commented Mar 12, 2018

Maybe rebase? to make sure it still works who knows...

@makenowjust makenowjust force-pushed the fix/crystal/dont-use-heredoc-inside-interpolation branch from 697043e to adacab6 Compare March 13, 2018 02:34
heredoc inside interpolation is buggy and it is unuseful.
It is a bit hard to fix this, so I'd like to forbid.
@makenowjust makenowjust force-pushed the fix/crystal/dont-use-heredoc-inside-interpolation branch from adacab6 to b1dd55e Compare March 13, 2018 02:45
@makenowjust
Copy link
Contributor Author

@RX14 I think this failed is irrelevant. Could you re-run CircleCI job?

@makenowjust
Copy link
Contributor Author

Thank you.

@ysbaddaden ysbaddaden merged commit 52fa3b2 into crystal-lang:master Mar 14, 2018
@RX14 RX14 added this to the Next milestone Mar 14, 2018
@RX14
Copy link
Contributor

RX14 commented Mar 14, 2018

@makenowjust just to clarify, is this a breaking change or not? (i.e. did this work before or is this just improving the error message)

@makenowjust
Copy link
Contributor Author

@RX14 Just a bug fix (or bug hiding). There is no source code broken by this.

@makenowjust makenowjust deleted the fix/crystal/dont-use-heredoc-inside-interpolation branch March 14, 2018 13:13
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

6 participants