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

Duck typed maths filters #731

Merged
merged 2 commits into from Mar 30, 2016
Merged

Conversation

ismasan
Copy link
Contributor

@ismasan ismasan commented Mar 30, 2016

What

Allow maths filters to work with objects that respond to #to_number as well as things that can be coerced into numbers.

Why

So I can design my own complex types or Drops that can work with maths filters.

How

If an object responds to #to_number, it will be used by Utils.to_number when coercing values.

An example:

class PriceDrop < Liquid::Drop
  def initialize(amount, currency_symbol)
    @amount, @currency_symbol = product_price, currency_symbol
  end

  def formatted
    "#{@currency_symbol}#{@amount}"
  end

  # .. etc, other methods here

  # this makes it work with Liquid's maths filters
  def to_number
    @amount
  end
end

Now it can be used with maths filters

price = PriceDrop.new(1000.0, '$')
template = Liquid::Template.parse("net total: {{ price.formatted }}. Tax: {{ price | times: 0.19 }}")
template.render('price' => price) # => "net total: $1000.0. Tax: 190"

Notes / questions

  • Standard text filters already use duck typing (by calling #to_s on the object), but Utils.to_number, used to coerce values in maths filters, does not. I am not sure whether or not this limitation is by design / security reasons, so I'm putting this here up for discussion.
  • I went with #to_number instead of a more conventional #to_i or #to_f to avoid accidental coercion (as many types in Ruby-land implement those) and also to mirror Utils.to_number. Please advice if this is not a good idea.

@fw42
Copy link
Contributor

fw42 commented Mar 30, 2016

I'm not against this... Seems useful to me. @pushrax, @dylanahsmith, thoughts?

@dylanahsmith
Copy link
Contributor

Sounds good to me

@pushrax pushrax merged commit bfee507 into Shopify:master Mar 30, 2016
@pushrax
Copy link
Contributor

pushrax commented Mar 30, 2016

Thanks for the contribution!

@ismasan
Copy link
Contributor Author

ismasan commented Mar 30, 2016

Thank you guys for Liquid! Do we have an idea re. when the next release is going to be?

pushrax added a commit that referenced this pull request Mar 30, 2016
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

4 participants