experimentation/internal_benches/partials.rb experimentation/internal_benches/partials_views/_partial.erb spec/public/abstract_controller/controllers/views/partial/partial_with_different_sets_of_locals/_ab.erb spec/public/abstract_controller/controllers/views/partial/partial_with_different_sets_of_locals/index.erb spec/public/abstract_controller/controllers/views/partial/partial_with_locals_overriding_helpers/_variable.erb spec/public/abstract_controller/controllers/views/partial/partial_with_locals_overriding_helpers/index.erb @@ -714,7 +714,9 @@ class Merb::BootLoader::Templates < Merb::BootLoader # template roots. eg app/views/shared/* template_paths << Dir["#{Merb.dir_for(:view)}/**/*.#{extension_glob}"] if Merb.dir_for(:view) - template_paths.flatten.compact.uniq + # This ignores templates for partials, which need to be compiled at use time to generate + # a preamble that assigns local variables + template_paths.flatten.compact.uniq.grep(%r{^.*/[^_][^/]*$}) end end end lib/merb-core/bootloader.rb @@ -606,15 +606,4 @@ class Merb::AbstractController opts[:exclude] = Array(opts[:exclude]).map {|x| x.to_s} if opts[:exclude] return opts end - - # Attempts to return the partial local variable corresponding to sym. - # - # ==== Paramteres - # sym<Symbol>:: Method name. - # *arg:: Arguments to pass to the method. - # &blk:: A block to pass to the method. - def method_missing(sym, *args, &blk) - return @_merb_partial_locals[sym] if @_merb_partial_locals && @_merb_partial_locals.key?(sym) - super - end end lib/merb-core/controller/abstract_controller.rb @@ -268,17 +268,6 @@ module Merb::RenderMixin # # In this case, "one" will be available in the partial through the local # variable named +number+. - # - # ==== Notes - # It is important to note that the object being passed to the partial - # as well as any extra local variables cannot use names of helper methods - # since any helper method of the same name will take precedence over the - # passed variable. Example: - # - # partial :bar, :with => "one", :as => :partial - # - # In this case, "one" will not be available in the partial because "partial" - # is already a helper method. def partial(template, opts={}) # partial :foo becomes "#{controller_name}/_foo" @@ -290,32 +279,36 @@ module Merb::RenderMixin kontroller = (m = template.match(/.*(?=\/)/)) ? m[0] : controller_name template = "_#{File.basename(template)}" end - template_method, template_location = - _template_for(template, opts.delete(:format) || content_type, kontroller, template_path) - (@_old_partial_locals ||= []).push @_merb_partial_locals - # This handles no :with as well with = [opts.delete(:with)].flatten - as = opts.delete(:as) || template_location.match(%r[.*/_([^\.]*)])[1] - - @_merb_partial_locals = opts.merge(:collection_index => -1, :collection_size => with.size) + as = (opts.delete(:as) || template.match(%r[(?:.*/)?_([^\./]*)])[1]).to_sym + + # Ensure that as is in the locals hash even if it isn't passed in here + # so that it's included in the preamble. + locals = opts.merge(:collection_index => -1, :collection_size => with.size, as => opts[as]) + template_method, template_location = _template_for( + template, + opts.delete(:format) || content_type, + kontroller, + template_path, + locals.keys) # this handles an edge-case where the name of the partial is _foo.* and your opts # have :foo as a key. - named_local = @_merb_partial_locals.key?(as.to_sym) + named_local = opts.key?(as) sent_template = with.map do |temp| - @_merb_partial_locals[as.to_sym] = temp unless named_local + locals[as] = temp unless named_local + if template_method && self.respond_to?(template_method) - @_merb_partial_locals[:collection_index] += 1 - send(template_method) + locals[:collection_index] += 1 + send(template_method, locals) else raise TemplateNotFound, "Could not find template at #{template_location}.*" end end.join - @_merb_partial_locals = @_old_partial_locals.pop sent_template end @@ -382,6 +375,7 @@ module Merb::RenderMixin # context<Object>:: The controller action or template (basename or absolute path). # content_type<~to_s>:: The content type (like html or json). # controller<~to_s>:: The name of the controller. Defaults to nil. + # locals<Array[Symbol]>:: A list of locals to assign from the args passed into the compiled template. # # ==== Options (opts) # :template<String>:: @@ -391,13 +385,13 @@ module Merb::RenderMixin # ==== Returns # Array[Symbol, String]:: # A pair consisting of the template method and location. - def _template_for(context, content_type, controller=nil, template=nil) + def _template_for(context, content_type, controller=nil, template=nil, locals=[]) template_method, template_location = nil, nil # absolute path to a template (:template => "/foo/bar") if template.is_a?(String) && template =~ %r{^/} template_location = self._absolute_template_location(template, content_type) - return [_template_method_for(template_location), template_location] + return [_template_method_for(template_location, locals), template_location] end self.class._template_roots.reverse_each do |root, template_meth| @@ -409,7 +403,7 @@ module Merb::RenderMixin template_location = root / self.send(template_meth, context, content_type, controller) end - break if template_method = _template_method_for(template_location.to_s) + break if template_method = _template_method_for(template_location.to_s, locals) end # template_location is a Pathname @@ -421,11 +415,12 @@ module Merb::RenderMixin # # ==== Parameters # template_location<String>:: The phyical path of the template + # locals<Array[Symbol]>:: A list of locals to assign from the args passed into the compiled template. # # ==== Returns # String:: The method, if it exists. Otherwise return nil. - def _template_method_for(template_location) - meth = Merb::Template.template_for(template_location) + def _template_method_for(template_location, locals) + meth = Merb::Template.template_for(template_location, [], locals) meth && self.respond_to?(meth) ? meth : nil end lib/merb-core/controller/mixins/render.rb @@ -3,9 +3,10 @@ end module Merb::Template - EXTENSIONS = {} unless defined?(EXTENSIONS) - METHOD_LIST = {} unless defined?(METHOD_LIST) - MTIMES = {} unless defined?(MTIMES) + EXTENSIONS = {} unless defined?(EXTENSIONS) + METHOD_LIST = {} unless defined?(METHOD_LIST) + SUPPORTED_LOCALS_LIST = Hash.new([].freeze) unless defined?(SUPPORTED_LOCALS_LIST) + MTIMES = {} unless defined?(MTIMES) class << self # Get the template's method name from a full path. This replaces @@ -55,26 +56,37 @@ module Merb::Template # ==== Parameters # path<String>:: A full path to find a template method for. # template_stack<Array>:: The template stack. Not used. + # locals<Array[Symbol]>:: The names of local variables # # ==== Returns # <String>:: name of the method that inlines the template. #--- # @semipublic - def template_for(path, template_stack = []) + def template_for(path, template_stack = [], locals=[]) path = File.expand_path(path) - ret = - if Merb::Config[:reload_templates] + if needs_compilation?(path, locals) file = Dir["#{path}.{#{template_extensions.join(',')}}"].first - METHOD_LIST[path] = file ? inline_template(load_template_io(file)) : nil - else - METHOD_LIST[path] ||= begin - file = Dir["#{path}.{#{template_extensions.join(',')}}"].first - file ? inline_template(load_template_io(file)) : nil - end + METHOD_LIST[path] = file ? inline_template(load_template_io(file), locals) : nil end - ret + METHOD_LIST[path] + end + + # Decide if a template needs to be re/compiled. + # + # ==== Parameters + # path<String>:: The full path of the template to check support for. + # locals<Array[Symbol]>:: The list of locals that need to be supported + # + # ==== Returns + # Boolean:: Whether or not the template for the provided path needs to be recompiled + #--- + def needs_compilation?(path, locals) + return true if Merb::Config[:reload_templates] || !METHOD_LIST[path] + + current_locals = SUPPORTED_LOCALS_LIST[path] + locals.any?{|local| !current_locals.include?(local)} end # Get all known template extensions @@ -93,6 +105,9 @@ module Merb::Template # ==== Parameters # io<#path>:: # An IO that responds to #path (File or VirtualFile) + # locals<Array[Symbol]>:: + # A list of local names that should be assigned in the template method + # from the arguments hash. Defaults to []. # mod<Module>:: # The module to put the compiled method into. Defaults to # Merb::InlineTemplates @@ -102,10 +117,14 @@ module Merb::Template # must be available to instances of AbstractController that will use it. #--- # @public - def inline_template(io, mod = Merb::InlineTemplates) - path = File.expand_path(io.path) - ret = METHOD_LIST[path.gsub(/\.[^\.]*$/, "")] = - engine_for(path).compile_template(io, template_name(path), mod) + def inline_template(io, locals=[], mod = Merb::InlineTemplates) + full_file_path = File.expand_path(io.path) + engine_neutral_path = full_file_path.gsub(/\.[^\.]*$/, "") + + SUPPORTED_LOCALS_LIST[engine_neutral_path] |= locals unless locals.empty? + ret = METHOD_LIST[engine_neutral_path] = + engine_for(full_file_path).compile_template(io, template_name(full_file_path), locals, mod) + io.close ret end @@ -156,12 +175,17 @@ module Merb::Template # ==== Parameters # io<#path>:: An IO containing the full path of the template. # name<String>:: The name of the method that will be created. + # locals<Array[Symbol]>:: A list of locals to assign from the args passed into the compiled template. # mod<Module>:: The module that the compiled method will be placed into. - def self.compile_template(io, name, mod) + def self.compile_template(io, name, locals, mod) template = ::Erubis::BlockAwareEruby.new(io.read) - _old_verbose, $VERBOSE = $VERBOSE, nil - template.def_method(mod, name, File.expand_path(io.path)) + assigns = locals.inject([]) do |assigns, local| + assigns << "#{local} = _locals[#{local.inspect}]" + end.join(";") + + code = "def #{name}(_locals={}); #{assigns}; #{template.src}; end" + mod.module_eval code, File.expand_path(io.path) $VERBOSE = _old_verbose name lib/merb-core/controller/template.rb @@ -81,6 +81,25 @@ module Merb::Test::Fixtures end end + class PartialWithLocalsOverridingHelpers < RenderIt + + def index + @foo = %w{ local } + render + end + + def value + "helper" + end + end + + class PartialWithDifferentSetsOfLocals < RenderIt + + def index + render + end + end + class PartialWithBoth < RenderIt def index spec/public/abstract_controller/controllers/partial.rb @@ -31,6 +31,14 @@ describe Merb::AbstractController, " Partials" do dispatch_should_make_body("PartialWithLocals", "Partial with local variables") end + it "should work with key/value pairs of locals that override helpers" do + dispatch_should_make_body("PartialWithLocalsOverridingHelpers", "Partial with local") + end + + it "should work with different calls to the same partial having different sets of locals" do + dispatch_should_make_body("PartialWithDifferentSetsOfLocals", "ab") + end + it "should work with both collections and locals" do dispatch_should_make_body("PartialWithBoth", "Partial with c-o-l-l-e-c-t-i-o-n-") end spec/public/abstract_controller/partial_spec.rb @@ -23,7 +23,7 @@ module Merb::Test::Fixtures class MyTemplateEngine - def self.compile_template(io, name, mod) + def self.compile_template(io, name, locals, mod) text = io.read table = { "\r"=>"\\r", "\n"=>"\\n", "\t"=>"\\t", '"'=>'\\"', "\\"=>"\\\\" } text = (text.split("\n").map {|x| '"' + (x.gsub(/[\r\n\t"\\]/) { |m| table[m] }) + '"'}).join(" +\n") @@ -58,7 +58,7 @@ describe Merb::Template do # @semipublic def rendering_template(template_path) - Merb::Template.inline_template(File.open(template_path), Merb::Test::Fixtures::MyHelpers) + Merb::Template.inline_template(File.open(template_path), [], Merb::Test::Fixtures::MyHelpers) Merb::Test::Fixtures::Environment.new. send(Merb::Template.template_name(template_path)) end @@ -77,6 +77,7 @@ describe Merb::Template do it "should compile and inline templates that comes through via VirtualFile" do Merb::Template.inline_template(VirtualFile.new("Hello", File.dirname(__FILE__) / "templates" / "template.html.erb"), + [], Merb::Test::Fixtures::MyHelpers) res = Merb::Test::Fixtures::Environment.new. @@ -97,7 +98,7 @@ describe Merb::Template do it "should find the full template name for a path via #template_for" do template_path = File.dirname(__FILE__) / "templates" / "template.html.erb" - name = Merb::Template.inline_template(File.open(template_path), Merb::Test::Fixtures::MyHelpers) + name = Merb::Template.inline_template(File.open(template_path), [], Merb::Test::Fixtures::MyHelpers) Merb::Test::Fixtures::Environment.new.should respond_to(name) end spec/public/template/template_spec.rb 3ef9115ad5c332abdbc0f880e4d455259b649854 Drew Colthorp colthorp@atomicobject.com http://github.com/wycats/merb-core/commit/59c0379aac67576e470e06e8c0cece67d25cb491 59c0379aac67576e470e06e8c0cece67d25cb491 2008-10-05T19:23:20-07:00 2008-10-05T10:03:08-07:00 compile templates with preambles to assign locals, support recompiling partial templates if previously unseen locals are provided Signed-off-by: Ezra Zygmuntowicz <ez@engineyard.com> 91b044abb37fc095dcf5c65ca2dc931619b4efe0 Ezra Zygmuntowicz ez@engineyard.com