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

Preserve Liquid tags contents #4484

Merged
merged 3 commits into from
May 15, 2018
Merged

Preserve Liquid tags contents #4484

merged 3 commits into from
May 15, 2018

Conversation

duailibe
Copy link
Member

Closes #3695

proto.inlineTokenizers.liquid = tokenizer;

function tokenizer(eat, value) {
const match = /^({%-?[\s\S]*?-?%}|{{-?[\s\S]*?-?}})/.exec(value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the -? part is unnecessary, {%- something -%} is already included in {% something %}, or are those leading/trailing spaces necessary?

And I think string.match(regex) should be better than regex.exec(string) since we do not use multiple exec calls, what do you think?

Copy link
Member Author

@duailibe duailibe May 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right about -?. I don't understand what you mean about the leading/trailing spaces.

I don't know if one is better. I was using exec specifically because I only care about the first match but that was before I was using ^. But sure, I'll change it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you mean about the leading/trailing spaces.

I meant this:

{% something %}
  ^         ^

Copy link
Member Author

@duailibe duailibe May 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not mandatory from what I could gather from parsers implementations


{{ foo
multiline
where does it end }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s add some examples from the issue:

{% include_relative _installations/tarball.md %}
{% cloudinary nice_prefix_-_for_the_filename.jpg %}

Copy link
Member

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just what @ikatyang said.

@duailibe duailibe merged commit 7bc5ec2 into prettier:master May 15, 2018
@duailibe duailibe deleted the liquid branch May 15, 2018 17:22
@lipis lipis added this to the 1.13 milestone May 16, 2018
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Aug 14, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants