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

Copy values bound to tweak-strings directly into buffers #2042

Merged
merged 1 commit into from
Sep 18, 2020

Conversation

evgenykochetkov
Copy link
Contributor

It fixes a problem with tweak-string-*s introduced in v0.35.0 and generally optimises memory usage.

@evgenykochetkov evgenykochetkov added the hotfix Issue/PR for a patch release label Sep 17, 2020
@evgenykochetkov evgenykochetkov self-assigned this Sep 17, 2020
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

👍

@@ -97,6 +97,8 @@ const cppType = def(
)
);

const escapeCppString = str => R.replace(/"/g, '\\"', unquote(str));
Copy link
Contributor

Choose a reason for hiding this comment

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

It's time to make escaping safer.
Here is the case that fails in compile-time, but the same tweak value then works fine in live session: "mwa\\ha"ha\" (first and last quotes is a part of literal)

Copy link
Contributor

@brusherru brusherru left a comment

Choose a reason for hiding this comment

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

Okay. Escaping chars we can fix further in separate PR :)
LGTM

@evgenykochetkov evgenykochetkov merged commit 333006b into 0.35.x Sep 18, 2020
This was referenced Sep 18, 2020
@evgenykochetkov evgenykochetkov deleted the fix-tweak-strings branch January 12, 2021 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix Issue/PR for a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants