merb

Comments and changes to this ticket

• 

  Yehuda Katz (wycats) February 27th, 2008 @ 12:31 AM

  • → Milestone changed from “” to “0.9.4”
  • → State changed from “new” to “open”

  Unfortunately, partial locals work via method_missing, which means that they only get called if no other method exists in the context (which is not controller). We either need to think of a solution for this problem or document it very well.

• 

  Daniel Parker March 6th, 2008 @ 10:38 PM

  I had fixed this a while back for myself by switching the order that method_missing looks for a method or a view local variable. It had previously been set to look for a method first, then a local, and I just switched them. Looking at 0.9.1 code, it looks good to me:

  def method_missing(sym, *args, &blk)
    if @_merb_partial_locals.key?(sym)
      @_merb_partial_locals[sym]
    else
      @web_controller.send(sym, *args, &blk)
    end
  end
  

  Has this already been fixed in 0.9.1?

• 

  Yehuda Katz (wycats) March 6th, 2008 @ 10:47 PM

  The problem now is that templates are actually methods on controllers, so they'll call methods themselves, and not method_missing.

• 

  Ezra Zygmuntowicz March 8th, 2008 @ 04:09 AM

  I don't like the current way we do method missing for locals.

  How would people feel about making locals just a hash, so in your views you would have access to locals[:foo] or whatever?

  This would simplify the implementation as we could just pass the hash into and we would not need method missing hackery. also there would be no more name conflicts.

  Thoughts?

• 

  Daniel Parker March 9th, 2008 @ 07:00 AM

  Sounds like a hack. There's gotta be a better way. Is there something bad

  about method_missing? What are the available options? What options are more

  desirable but unavailable, and why?

• 

  Daniel Parker March 21st, 2008 @ 07:42 PM

  What about using instance variables for all variables passed into rendering? That would also have the positive side-effect of prettying up the partials a little - never have problems mixing up variables and methods.

• 

  Ezra Zygmuntowicz March 21st, 2008 @ 10:40 PM

  I'm going to change this to work with a locals hash passed into the compiled methods of partials. The methodmissing hack is way more of a hack then just using a hash.

  there are no changes needed if you want to just set instance variables instead of locals. But for passing in locals I am going to just go with a locals hash.

• 

  Daniel Parker March 21st, 2008 @ 11:14 PM

  And locals will persist only in the partial they were intended for, and lower? And when locals are created, they won't actually overwrite a local by the same name but defined in a parent partial, only for the current and lower partials? Is that possible?

  Actually, when it comes to locals as a hash, my StackableHash might work perfectly for the task:

  @locals = StackableHash.new
  => {}
  @locals['lev1'] = 'hello'
  => 'hello'
  @locals.add_stack!
  => {'lev1'=>'hello'}
  @locals['lev2'] = 'bye'
  => 'bye'
  @locals
  => {'lev1'=>'hello', 'lev2'=>'bye'}
  @locals.pop_stack!
  => {'lev2'=>'bye'}
  @locals
  => {'lev1'=>'hello'}
  

  It could be configured to different types of 'key inheritance' if need be.

• 

  Ben Chiu May 27th, 2008 @ 11:03 PM

  i must be missing something here. wouldn't it be more intuitive to have:

  partial 'apart', :foo => 'stay foo'

  inside partial:

  defined? foo --> true

  defined? bar --> false

  foo ||= 'be foo' --> 'stay foo'

  bar ||= 'be bar' --> 'be bar'

  all my partials are breaking because foo ||= 'foo' always reassigns foo.

  btw, any chance to get partials with layouts? it's useful when say index view has partial 'home' wrapped in a div, and the div can be updated via ajax using same partial (sans layout).

• 

  Ezra Zygmuntowicz May 28th, 2008 @ 08:19 PM

  yes we are going to change the wa locals work so it is just a hash that gets passed into the compiled templat methods. This means you will pass in your locals and then access them in a hash called 'locals' in the views. this will circumvent all this crazy bullshit that happens with locals now.

• 

  Ben Chiu May 29th, 2008 @ 12:12 AM

  good stuff. thanks. i do have a question tho, is there a reason why a local foo isn't automatically created from the locals[:foo]? logic tells me that if i pass foo to my partial, chances are that's the foo i want. this implementation seems cleaner. introducing a locals hash out of the blue seems a little mysterious (magical?).

• 

  Ezra Zygmuntowicz May 29th, 2008 @ 10:38 AM

  a locals hash is much less magical then using nasty eval to create true locals.

• 

  Michael Klishin (antares) August 2nd, 2008 @ 06:47 PM

  • → Tag changed from “” to “medium merb”
  • → Milestone changed from “0.9.4” to “0.9.7”
• 

  Yehuda Katz August 2nd, 2008 @ 07:24 PM

  • → Assigned user changed from “” to “Yehuda Katz (wycats)”

  The way Rails handles locals now is actually quite interesting and probably worth doing. I had previously contemplated something similar but worried about speed (benchmarking proved my worried unfounded):

  1) When you render a partial with locals, compile the template to take the locals as a Hash, but also add a preamble in the function to create locals out of the hash.

  2) Every subsequent time, check to see whether the locals list is a subset of the existing compiled locals. If it is, continue. If not, recompile the template with the larger set of possible locals.

  What's interesting about (2) is that in real life the recompilation will only happen a few times. Checking the existing set doesn't turn out to be that slow (although I will obviously benchmark the final implementation to check if it's changing our results).

• 

  Carl Lerche August 4th, 2008 @ 09:24 AM

  There is only one problem that I can foresee about this method. If one time, the partial is called setting the local variable foo, the foo method will become unavailable for every call to the partial where the developer might only want foo to be overridden that one time.

  Another option would be to compile a version of the partial for each argument signature.

• 

  Michael Klishin (antares) September 14th, 2008 @ 02:00 AM

  • → Milestone changed from “0.9.7” to “0.9.9”
• 

  Drew Colthorp October 6th, 2008 @ 03:30 AM

  Here's a patch that implements Yehuda's recommendation above. A few things to discuss.

  • I had to change the signature of a few methods. compile_template now takes a list of local names to be generated in the preamble. The new signature is compile_template(io, name, locals, mod).
  • Merb::Template.template_for also takes a list of locals. The new signature is template_for(path, template_stack = [], locals=[]). However, the docs say that template_stack is unused. Can this be removed?
  • inline_template also takes a new argument, I chose the signature inline_template(io, locals=[], mod = Merb::InlineTemplates).
  • With this change in place, it doesn't really make sense to precompile partials, since we'll just need to recompile them when they're used. I filtered out partial templates from the list of files to compile in Merb::BootLoader::Templates.

  • merb-core-locals-take-precedence-over-helpers.patch 19.5 KB
• 

  Drew Colthorp October 6th, 2008 @ 03:32 AM

  This patch updates merb-haml and merb-builder to support the changes in my previous post. Let me know if I should start a new ticket for this patch.

  • merb-more-support-locals-in-haml-and-builder.patch 5.7 KB
• 

  Drew Colthorp October 6th, 2008 @ 04:04 AM

  • → Tag changed from “medium merb” to “documentation medium merb "merb-more" patch spec”

  This merb-core patch is better – fixed a failing spec and removed an unnecessary change to the Erubis template.

  • merb-core-locals-take-precedence-over-helpers.patch 19.3 KB
• 

  Ezra Zygmuntowicz October 6th, 2008 @ 05:29 AM

  I've applied and played with this locally and I think it's a good solution.

  Can I get some +1's on applying it?

• 

  Ezra Zygmuntowicz October 6th, 2008 @ 06:13 AM

  OK I've applied these patches. This is definitely the right direction to go and I;d liek to get this in so any issues can get settled this week. I'll leave the ticket open but I;ve applied the patch.

  Nice work Drew, thanks.

• 

  Michael Klishin (antares) October 7th, 2008 @ 11:53 AM

  • → State changed from “open” to “resolved”

  The only note we should add is that - in local name is no longer can be used. This is Ruby grammar thing though. I tried it with both Erb and Haml apps, seems to work fine.

  Thank you very much, Drew.