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

Fix interpolated string rendering issues #483

Merged
merged 4 commits into from
Jun 24, 2024
Merged

Fix interpolated string rendering issues #483

merged 4 commits into from
Jun 24, 2024

Conversation

osyrisrblx
Copy link
Member

@osyrisrblx osyrisrblx commented Jun 22, 2024

Fixes #458 and #484

#458: Incorrect escaping for multi-line interpolated string literals

The issue was that the escape logic did not pick up Windows-style line endings properly. For this we need to also check for \r followed by an optional \n.

Input

warn(`look I am
a multiline string`)

Before

warn(`look I am
\
a multiline string`)

After

warn(`look I am\
a multiline string`)

#484: Incorrect escaping for unicode escape sequences in interpolated string literals

Input

const string_a = "\u{E000}";
const string_b = `\u{E000}`;

Before

local string_a = "\u{E000}"
local string_b = `\u\{E000\}`

After

local string_a = "\u{E000}"
local string_b = `\u{E000}`

@osyrisrblx osyrisrblx changed the title Fix #458 Fix interpolated string rendering issues Jun 22, 2024
// escape braces and newlines, but do not touch braces within unicode escape codes
return node.text
.replace(
/(\\u\{[0-9A-Fa-f]+\})|([{}])/g,
Copy link
Contributor

@Dionysusnu Dionysusnu Jun 22, 2024

Choose a reason for hiding this comment

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

This regex might be cleaner with a negative lookbehind instead of capturing the Unicode sequences to like-for-like replace them. Was it a conscious decision not to use one?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't able to find a way to do this that was readable. Open to suggestions.

@osyrisrblx osyrisrblx requested a review from Dionysusnu June 24, 2024 01:45
@osyrisrblx osyrisrblx merged commit 59387e3 into master Jun 24, 2024
3 checks passed
@osyrisrblx osyrisrblx deleted the fix-458 branch June 24, 2024 01:49
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.

Multiline interpolated strings produces invalid backslashes
2 participants