- Notifications
You must be signed in to change notification settings - Fork689
Add escape sequence support for string literals (\t, \n, \r, etc.)#2331
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:develop
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
mathiasrw left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
please remove \b \f \v support
src/alasqlparser.jison Outdated
| {$$=newyy.StringValue({value:$1.substr(1,$1.length-2).replace(/(\\\')/g,"'").replace(/(\'\')/g,"'")});} | ||
| { | ||
| var str=$1.substr(1,$1.length-2).replace(/(\'\')/g,"'"); | ||
| str=str.replace(/\\(.)/g,function(match,char) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You dont know if the first \ is escaped beforehand. You will need to find pairs like \ (regex as \\) or not pair (what you got now \(.))
Please see if you can place regex outside of the executing function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed the regex to be more specific:/\\(n|t|r|\\|'|")/g instead of/\\(.)/g. This only matches the supported escape sequences and properly handles\\ without needing to worry about pairs. (57cfff3)
Uh oh!
There was an error while loading.Please reload this page.
| case'b':return'\b'; | ||
| case'f':return'\f'; | ||
| case'v':return'\v'; | ||
| case'\\':return'\\'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please check if this is being solved via#2335
Uh oh!
There was an error while loading.Please reload this page.
- Removed support for \b, \f, \v escape sequences as requested- Fixed regex to only match specific escape sequences: \n \t \r \\ \' \"- Removed default case that was accepting any character after backslash- Updated test134-B to remove tests for \b, \f, \v- Updated test125 escape sequence assertions to work with new behaviorCo-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Removed \b, \f, and \v support as requested. Now only supporting \n, \t, \r, \, ', and " escape sequences. (57cfff3) |
| functionJSONtoJS(obj,context,tableid,defcols){ | ||
| vars=''; | ||
| if(typeofobj=='string')s='"'+obj+'"'; | ||
| if(typeofobj=='string')s='"'+escapeq(obj)+'"'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What is we have a string of data that contains \n, will it then get converted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, this is the correct behavior. When data contains actual newline characters (after SQL parsing converts\n to a newline),escapeq() converts them back to\n when generating JavaScript code. This ensures the generated JS code is syntactically valid.
Flow:
- SQL
'hello\nworld'→ Parser converts to actual newline → Stored in memory ashello<LF>world - When generating JS code →
escapeq()converts back to\n→ Generated code:"hello\nworld" - JS runtime executes → Results in newline character again
Withoutescapeq(), strings with newlines/quotes would produce invalid JavaScript syntax.
Uh oh!
There was an error while loading.Please reload this page.
Description
AlaSQL now processes standard escape sequences in string literals. Previously,
\t,\n,\r, and similar sequences were treated as literal characters rather than their control character equivalents.This implementation focuses on essential escape sequences while maintaining backward compatibility with existing quote escaping mechanisms.
Changes
src/alasqlparser.jison): ModifiedStringValuegrammar rule to process escape sequences:\n,\t,\r,\\,\',\"/\\(n|t|r|\\|'|")/gto only match supported sequences\b,\f,\vto keep implementation conservative\xremains as\x)src/58json.js): FixedJSONtoJSto escape strings properly when generating JavaScript code, preventing syntax errors with embedded quotestest/test134-B.jswith 8 test cases covering all supported escape sequences; updatedtest/test125.jsto reflect new escape sequence behaviorExample
Supported Escape Sequences
\n- newline\t- tab\r- carriage return\\- backslash\'- single quote\"- double quoteAll other sequences (like
\b,\f,\v,\x, etc.) are left as literal backslash + character.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our2 minute survey.