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

Escape charactors in string literals #45

Open
2 tasks done
tuchida opened this issue Jun 16, 2021 · 2 comments
Open
2 tasks done

Escape charactors in string literals #45

tuchida opened this issue Jun 16, 2021 · 2 comments

Comments

@tuchida
Copy link

tuchida commented Jun 16, 2021

Checklist

  • I have searched the issue list
  • I have tested my example against Shopify Liquid. (This isn't necessary if the actual behavior is a panic, or an error for which IsTemplateError returns false.)

Expected Behavior

{{ "a\"b\\c" }}

output

a"b\c

Actual Behavior

Syntax Error

Detailed Description

ref. #5

The Shopify specification does not allow escaping of characters in string literals.
https://shopify.github.io/liquid/basics/types/#string

Liquid does not convert escape sequences into special characters.

But this is inconvenient for me.

Possible Solution

The proposal makes it escapable when ``.
Shopify/liquid#1174

This my branch uses strconv.Unquote to escape only double-quoted.
master...tuchida:escape-string

In this implementation, it seems that only \' and \" can be escaped.
https://github.com/acstech/liquid/blob/5bcb5c9b2d87dbd09e8a08889477167721e09edf/core/parser.go#L135-L136

If you want to support escaping, you can decide on the specification and I will create a Pull Request.

@osteele
Copy link
Owner

osteele commented Jun 17, 2021

I'm wary of introducing a difference from the Shopify specification. Aside from this, this seems like an improvement, that would be useful for users who are not concerned about compatibility with the specification and with other implementations.

How about if this is controlled by a flag or option, that defaults to false?

This could be a (new) configuration parameter on NewEngine, or ParseString / ParseTemplate / ParseTemplateLocation.

@tuchida
Copy link
Author

tuchida commented Jun 17, 2021

I'm not familiar with Ragel, but it looks like it would probably be difficult to branch here.

dqchar = [^"\\] | ( '\\' any );
string = '"' . dqchar* . '"' | "'" (any - "'")* "'" ;

I can implement it if it can enclose it in backtick and allow backtick here only when the flag is valid.

if lex.data[lex.ts] == '\'' {

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

No branches or pull requests

2 participants