Skip to content

Correctly escape '${ #2083

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

Merged
merged 3 commits into from
Nov 1, 2020
Merged

Correctly escape '${ #2083

merged 3 commits into from
Nov 1, 2020

Conversation

Gabriella439
Copy link
Collaborator

Fixes #2078

Previously the pretty-printer would escape '${ as '''${ inside of
a multi-line string, but that escape sequence is incorrect because
''' is itself an escape sequence for ''. The solution is to
instead escape '${ as ${"'"}${

Fixes #2078

Previously the pretty-printer would escape `'${` as `'''${` inside of
a multi-line string, but that escape sequence is incorrect because
`'''` is itself an escape sequence for `''`.  The solution is to
instead escape `'${` as `${"'"}${`
@Gabriella439 Gabriella439 mentioned this pull request Oct 29, 2020
@Gabriella439 Gabriella439 merged commit b99f028 into master Nov 1, 2020
@Gabriella439 Gabriella439 deleted the gabriel/2078 branch November 1, 2020 02:53
@sjakobi
Copy link
Collaborator

sjakobi commented Nov 1, 2020

Shouldn't we also escape the ${ part of '${? If so, it should be ${"'"}''${, right?

Indeed. I think that's just a little mistake in the description though. The test output looks correct, I think.

@basile-henry
Copy link
Collaborator

Shouldn't we also escape the ${ part of '${? If so, it should be ${"'"}''${, right?

@basile-henry
Copy link
Collaborator

Oh yes you're right! I did look at the test, but I guess I didn't parse it correctly. Sorry about that 😅

@sjakobi
Copy link
Collaborator

sjakobi commented Nov 1, 2020

No worries, @basile-henry. I was initially surprised too! :)

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.

Interpolation and files imported as Text
4 participants