Skip to content

Only use MethodLiteral in condition expressions#1300

Merged
dylanahsmith merged 2 commits into
masterfrom
condition-specific-method-literal
Sep 25, 2020
Merged

Only use MethodLiteral in condition expressions#1300
dylanahsmith merged 2 commits into
masterfrom
condition-specific-method-literal

Conversation

@dylanahsmith
Copy link
Copy Markdown
Contributor

@dylanahsmith dylanahsmith commented Sep 24, 2020

Problem

The MethodLiteral literal value was added in #592 which described it as a hacky solution:

Not sure if this is a good fix, feels kinda hacky, but seems to work.

One way in which it is hacky is that it is only relevant for boolean comparison equality expressions and isn't relevant for all expressions.

In liquid-c (Shopify/liquid-c#60), I would like Liquid::Expression.parse to return a Liquid::C::Expression object that can be rendered more efficiently and I would like to avoid coupling this work to these MethodLiteral objects.

Solution

Add a Liquid::Condition.parse_expression method that wraps Liquid::Expression.parse so that it will return a MethodLiteral object for empty or blank literals and will otherwise delegate to Liquid::Expression.parse. This way the values for these literals that are returned by Liquid::Expression.parse can just be the empty strings that MethodLiteral#to_liquid returns.

Comment thread lib/liquid/condition.rb Outdated
Copy link
Copy Markdown
Member

@tjoyal tjoyal left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants