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 improper string escaping #72

Merged
merged 3 commits into from
Feb 15, 2022
Merged

Fix improper string escaping #72

merged 3 commits into from
Feb 15, 2022

Conversation

cpiber
Copy link
Contributor

@cpiber cpiber commented Feb 14, 2022

Before, input strings like '\\' or "\\" were improperly escaped. For double quotes, backslashes were not escaped at all and for single quotes, they were basically ignored (due to /\\?'/).

The above examples now produce '\\'/"\\" instead of '\'/"\" and are thus valid strings again.

Before escaped the backslashes from the escaping of `\n` and `\r` a second time...
@sindresorhus
Copy link
Owner

This will need some tests.

@cpiber
Copy link
Contributor Author

cpiber commented Feb 14, 2022

So the change is desirable? Because it breaks existing tests that obviously relied on a "wrong" escaping... In that case I'll update that and write new tests :)

index.js Show resolved Hide resolved
@sindresorhus
Copy link
Owner

So the change is desirable?

Yes

@sindresorhus sindresorhus merged commit 699cef7 into sindresorhus:main Feb 15, 2022
@sindresorhus
Copy link
Owner

Thanks :)

@cpiber cpiber deleted the patch-string-escape branch February 15, 2022 18:09
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.

2 participants