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 escape_strings issue #2268

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheFalloutOf76
Copy link

Description

firstly, the '\n' is not correctly escaped (should be '\n'), which causes a syntax error in the beautified code, because an actual new line is inserted instead of \n.

additionally, even if it were correctly escaped, it would still cause problems as while the characters look similar, they are treated as not equal by javascript.

let object = {};
object['\u2028'] = "this is '\\u2028'";
object['\n'] = "this is '\\n'";
console.log(object['\u2028']) // this is '\u2028'

if it were replaced by '\n', it would become

let object = {};
object['\n'] = "this is '\\u2028'";
object['\n'] = "this is '\\n'";
console.log(object['\n']) // this is '\n'

which changes the behavior of the code

because of these issues, its best to leave certain escaped characters the way they are.

Fixes Issue: #2267

Before Merge Checklist

These items can be completed after PR is created.

(Check any items that are not applicable (NA) for this PR)

  • JavaScript implementation
  • Python implementation (NA if HTML beautifier)
  • Added Tests to data file(s)
  • Added command-line option(s) (NA if
  • README.md documents new feature/option(s)

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.

None yet

1 participant