-
Notifications
You must be signed in to change notification settings - Fork 17
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 double-escaping of newlines #68
base: master
Are you sure you want to change the base?
Fix double-escaping of newlines #68
Conversation
Hi @bryghtlabs-richard it works for me, could you merge the PR? Thank you! |
@bryghtlabs-richard hold on, I'm testing with apostrophe escaping ( slash + ' ), will be able to tell if it works properly in a day or so |
Hi all, I'm not a maintainer of lv_i18n - we'll need their review and help, and with anything it's worth taking the time to get it right. Also, I'm not a JS developer, please take this patch with a grain of salt, and I'm not sure this is the right way to handle this - the other option will be to ensure the keys/values get de-escaped on parsing, then re-escaped on output. |
cc @puzrin So we assume that the characters are correctly escaped in the input strings already, so we can use them as they are, right? |
It seems to work that way, YAML and C seem to share a lot of escape sequences, I tried " and it compiled properly with this patch, but we should check any differences in the language escapes. There are some tests for C unescaping, I'll try to get those running when I look at this again. @rlidwka , any thoughts? |
Hi @bryghtlabs-richard, I rebased your commit on master. It seems to work for me but I also am not a JS developer and cannot comment on any possible corner cases. 0001-fix-double-escaping-of-newlines.patch Edit: the provided patch contained errors, now it should work |
I came across this exact bug, why isn't this PR merged? |
I haven't had time to finish testing each escape sequence. Feel free to pick this up. |
I'm not sure this is the right approach. It looks like lib/parser.js has a mechanism to un-escape input files, and lib/compiler_template.js has a mechanism to escape output files, and that seemed to make sense, so I was expecting both to run, but it seems
esc()
was running on the raw text, which was then double-escaped.This change removes
esc()
.Fixes #66