Ticket Navigation

Ticket #130 (closed defect: invalid)

Opened 1 year ago

Last modified 1 year ago

PATCH for null templating engine

Reported by:	pmisiowi..@mac.com	Assigned to:
Priority:	major	Milestone:	0.4
Component:	Merb	Keywords:
Cc:

Description

This patch fixes two issues in render.rb.

After checking out the latest Merb trunk on 8/15/07 and testing the Invoice Tracker app found at http://svn.depixelate.com/applications/invoice_tracker/, I found an issue in which the controller's call to render was bombing but only on the first request.

can't convert nil into String - (TypeError)
/usr/local/lib/ruby/gems/1.8/gems/merb-0.4.0/lib/merb/template/erubis.rb:36:in `read'
/usr/local/lib/ruby/gems/1.8/gems/merb-0.4.0/lib/merb/template/erubis.rb:36:in `new_eruby_obj'
/usr/local/lib/ruby/gems/1.8/gems/merb-0.4.0/lib/merb/template/erubis.rb:25:in `transform'
/usr/local/lib/ruby/gems/1.8/gems/merb-0.4.0/lib/merb/mixins/render.rb:151:in `render'
/usr/local/httpd/invoice_tracker/app/controllers/clients.rb:7:in `new'

Per the Rdoc, calling just plain "render" should look for a file matching "views/controllername/actionname.*"

However, after tracing the code, the render function was attempting (via find_template) to look for:

/usr/local/httpd/invoice_tracker/app/views/clients/index.{}

...because at this point, the template engine has not yet been registered and so @@template_extensions is empty.

The first part of the patch changes the above to

/usr/local/httpd/invoice_tracker/app/views/clients/index.{*}

After resolving the above, a new error popped up:

undefined method `transform' for nil:NilClass - (NoMethodError)
/usr/local/lib/ruby/gems/1.8/gems/merb-0.4.0/lib/merb/mixins/render.rb:153:in `render'

Similar to the above issue, since the templating engine was not yet registered, the engine_for function was returning a null object. With this patch, the engine_for function defaults to Erubis to be consistent with the exception trap.

In case this is helpful, I'm running ruby 1.8.6 (2007-03-13 patchlevel 0) [universal-darwin8.0] on MacOSX 10.4.10.

Attachments

render_nil_engine_patch.diff (0.8 kB) - added by pmisiowi..@mac.com on 08/16/07 13:51:01.

Change History

08/18/07 06:31:38 changed by b.candl..@pobox.com

Oops, I raised this in duplicate as #133.

However I don't think you solution is right. If you always tell find_template to match :ext => '*' then the code

extensions = .@@template_extensions.keys].flatten.uniq

is never used, so arguably should be removed.

But more importantly, since only the first matching template is found, it could lead to mismatches, especially since your patch treats all unrecognised extensions as erb. For example:

suppose you have foo.rxml and then make a copy as foo.bak. In this case, foo.bak will be loaded in preference to foo.rxml; it would also be treated as erb.
if you had foo.js.bak and foo.rxml, then foo.js.bak would match foo.* and again be treated as erb
if you have foo.rxml but have not loaded the builder gem, then foo.rxml will be processed as if it were erb, leading to strange output which would be hard to diagnose

So I think the author's intention is to match only known extensions, and this should be maintained. The solution which I proposed in #133 is to modify the skeleton merb_init.rb so it loads at least the Erubis handler, and indicates what needs to be done to load others: e.g.

## Uncomment the template handers which your application needs to load
Merb::Template::Erubis       # .herb .jerb .erb .rhtml .html.erb .js.erb
#Merb::Template::Haml        # .haml
#Merb::Template::Markaby     # .mab
#Merb::Template::XMLBuilder  # .rxml .xerb .builder

(and also mention the need to do this for existing applications in History.txt)

Download in other formats:

merb