Use instance_eval to evaluate expressions

[stadelmanma]

This would replace the manual evaluation that is currently done which has the limitation that methods cannot be called on hash objects. Executing things this way would add the possibility for NoMethodErrors which aren't possible in the current LookupOrMethod#evaluate method (shown below). Instead we would catch the NoMethodError and just return nil in that instance.

This would be a huge step forward in allowing expressions of arbitrary complexity to be evaluated like in regular ERB templates although we would still be limited to one-liners.

Handling Backwards Compatibility Issues:

For full backwards compatibility with .expression used on a Hash we would need to monkey-patch the Hash class's method_missing method to do a hash lookup. However, that is not something we want floating around in our general namespace.

This would only be an option if the patch can be applied inside the limited scope of instance_eval and no where else. method_missing and refine are available in ruby 2.0 and I think they might do the trick. (Links below). However things accessed using send fail for refinements. This might kill the idea because overriding method_missing doesn't appear possible in my test cases.

While dropping support for ruby less than 2.0 is undesirable support for 1.9.3 ended in 2015 and support for 2.0 ended in 2016. Updating the gemspec and tagging a release (0.0.22) before this change and then (0.1.0) afterwards should be sufficient as people using old rubies can still use an older version.

Excerpt from operations.rb

    class LookupOrMethodCall < Struct.new(:receiver_expr, :expression)
      def evaluate(context)
        if receiver = receiver_expr.evaluate(context)
          expression.split(".").inject(receiver) do |local, m|
            case local
            when Hash
              local[m]
            else
              local.public_send m if local.respond_to?(m)
            end
          end
        end
      end

....

Non-functional test case (added incase i figure out a fix)

module LookupHash
  refine Hash do
    def method_missing(name, *args, &block)
      fetch(name, nil)
    end
  end
end

InstanceObj = Struct.new(:data)
instance = InstanceObj.new({ key1: 123 })

using LookupHash
begin
  instance.instance_eval("data.key1")
rescue NoMethodError => e
  puts e
end

http://ruby-doc.org/core-2.0.0/BasicObject.html#method-i-method_missing
http://ruby-doc.org/core-2.0.0/doc/syntax/refinements_rdoc.html
https://ruby-doc.org/core-1.9.3/BasicObject.html#method-i-instance_eval

[senny]

@stadelmanma if we would break backwards compatibility we will need a very good reason to do so. It would mean that upgrading potentially means that you need to update all your templates.

To provide some context, would you mind sharing some expressions that this enables that are not possible now? Ideally this would be real-world use-cases and not constructed scenarios.

One more thing, in the beginning I deliberately kept the expression complexity low. Editing templates is not fun, you should be editing code. So instead of defining complex expressions in the template Sablons take would be to define a wrapping object with a method that does what you need. You can then easily change that method without needing to update any templates. I think in terms of maintenance this is a huge benefit.

[stadelmanma]

@senny don't worry I'm not sold on this idea either (hence the proposed 😉 ). It came to mind as a way to make our context manipulations more powerful so I decided to jot it down, and invite any feedback.

Permitting more complex expressions would be part of a second PR that refactors our conditional and loop statements into the natural ruby syntax. i.e. paragraph:if becomes if pragraph. We would also have the benefit of leveraging the entire ruby environment implicitly without needing to manually code equivalent "sablon expressions" such each and if. Maintaining backwards compatibility in the second part is easy by using the same regular expression patterns we already use to transform the "old" syntax into the new.

Originally I thought this could be implemented without breaking backwards compatibility. However, after some testing and research it doesn't seem like that is possible and I don't think there are enough benefits to warrant the change. Especially given the tedium involved in updating those word templates.

[senny]

Thanks for explaining this further. Having a more "natural" syntax would be nice for sure. However, my other point stands, you don't want these expressions in the template to get complex. You want the complexity in your code. Ping me if you happen to do a spike to implement this. I'd be interested in seeing the internals. I vaguely remember that there were some issues with parsing to know if it's a block or not, I can imagine that parsing real Ruby would be tricky.

[stadelmanma]

Closing this as it isn't something that I think fits well within the original intent of sablon, which was to keep templates simple because they are a real pain to debug.

Something I want to do eventually is refactor the logic used to handle merge fields and their corresponding operations to allow easier extension by the end user (a sort of plugin type of API maybe). Perhaps depending on the flexibility added by those changes this could be implemented separately as an extension gem instead of being part of the core components.