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

Allow variable and tag end characters to be quoted. #27

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

Conversation

dylanahsmith
Copy link
Contributor

@pushrax & @trishume for review

For Shopify/liquid#623

Problem

Shopify/liquid#170 tried to allow curly braces in variables, but was rejected for performance reasons. However, parsing is a lot faster now that we are using liquid-c, so I think I don't think performance would be as much of a concern now.

Solution

When encountering a quote character inside a variable or tag, the tokenizer will include everything up to the end quote character (if one is present) even if it includes the tag/variable end characters.

@pushrax
Copy link
Contributor

pushrax commented Jul 14, 2015

I don't really like the idea of coupling the definition of a quoted string literal to both the tokenizer and lexer, but if we have to then let's use the same function in both places (i.e. simple logic around scan_past).

@dylanahsmith
Copy link
Contributor Author

I don't really like the idea of coupling the definition of a quoted string literal to both the tokenizer and lexer

Well, a tokenizer is a lexer. The problem is that the tokenization was split into multiple passes originally. With strict parsing we could actually parse variables in the same pass, but lax parsing makes that more complicated.

if we have to then let's use the same function in both places (i.e. scan_past).

scan_past is a very thin wrapper around memchr which doesn't provide any benefit here. It is also awkward, since it makes the assumption that the cursor is on the quote character, and I found naming the return value more difficult (e.g. after_end_quote vs end_quote). I think using memchr is easier to read, since it is a standard API.

@pushrax
Copy link
Contributor

pushrax commented Jul 14, 2015

{% " %} asdf {% " %} will change behaviour right?

@dylanahsmith
Copy link
Contributor Author

Yes, unterminated quotes would be a behaviour change. Should we check existing templates to see how often this happens, and how much of an effect it would have on those templates?

@trishume
Copy link
Contributor

👍

But you should definitely test it for breakage of existing templates

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

3 participants