upgrade to 2.3.2: render started matching vim-patchmode *.orig files

[czarek]

As of 2.3.2 render started loading vim patch mode files
( '*.erb.orig' instead of '*.erb' ):

How to reproduce: create *.erb.orig files in the appropriate view
directory:

rails 2.3.2 loads:

app/views/articles/new.html.erb.orig

instead of:

app/views/articles/new.html.erb

This is probably because of the way
ActiionView::ReloadableTemplate::ReloadablePath is implemented. The
keys it contains come from the relative path without the extension, so
it gets:

@paths[ 'sessions/logout.html.erb' ] => template # from
login.html.erb

and the same hash value gets overwritten:

@paths[ 'sessions/logout.html.erb' ] => template # from
login.html.erb.orig

The quick hack I made just reverses the file listing
(Dir.glob.sort.reverse), so the *.erb files come after the *.erb.orig
files, and replace those created from .orig files (patch below).


NOTE: The problem appeared in RSpec, which may load templates
differently than production/non rspec/unit test/console - that is why
I changed all 3 Dir.glob instances.


--- reloadable_template.rb.orig 2009-03-18 12:27:45.000000000 +0100
+++ reloadable_template.rb 2009-03-19 18:48:31.109227986 +0100
@@ -69,7 +69,7 @@

# get all the template filenames from the dir
def template_files_from_dir(dir)
- Dir.glob(File.join(dir, '*'))
+ Dir.glob(File.join(dir, '*')).sort.reverse
end
end

--- template.rb.orig 2009-03-18 12:27:45.000000000 +0100
+++ template.rb 2009-03-19 18:51:29.281239443 +0100
@@ -78,7 +78,7 @@

private
def templates_in_path
- (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/
**")).each do |file|
+ (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/
**")).sort.reverse.each do |file|
yield create_template(file) unless File.directory?(file)
end
end

[Cezary Bagiński]

Update to my own post:

There are two paths depending on whether you have
config.action_view.cache_template_loading or not.

(Either EagerPath.load! or ReloadablePath.register_template)

EXAMPLE: Given the files:
views/main/index.html.erb
views/main/index.html.erb.backup

RESULT: get '/main' renders 'views/main/index.html.erb.backup'


PROBLEM:

In both cases (Eager, ReloadablePath) "@path[path]" is assigned twice,
setting "index.html.erb.backup" as the final template.

This is not relevant for production mode, since you don't usually have
backups of editor files there.

Backup files such as: "index.html.erb~" are handled correctly, BTW -
added to the list, but never used because of unique extension ("erb~").

QUESTION: Why does ActionView consider the *.backup files anyway?

ANSWER: They pass the ActionView::Template.split test:

elsif valid_extension?(m[1]) # format and extension

The thing is, index.html.erb.orig has 3 extensions m = ['html', 'erb',
'orig'] and m[2] is ignored. Idea: join m[1] and m[2] and return as
extension?


QUESTION: shouldn't both implementations(Eager/Reloadable) be identical,
except for handling _when_ to load the templates?

The differences may cause some problems to come up only in one case
(w/wo caching). E.g. my guess is that broken symbolic links will be
handled differently - ReloadablePath uses File.file?, which returns
false for the broken symlinks that the Dir.glob returns. EagerPath
doesn't seem to check that.

SOLUTION; Only assign a template once, don't clobber the previous value.
This works, because Dir.glob returns a sorted list, where
'index.html.erb' is at the top.

PATCH: The patch becomes trivial now:

--- template.rb.orig 2009-03-18 12:27:45.000000000 +0100

+++ template.rb 2009-03-20 12:15:14.698483807 +0100
@@ -64,7 +64,7 @@
templates_in_path do |template|
template.load!
template.accessible_paths.each do |path|
- @paths[path] = template
+ @paths[path] ||= template #don't clobber previous entry
with *.erb.backup files
end
end
@paths.freeze

--- reloadable_template.rb.orig 2009-03-18 12:27:45.000000000 +0100

+++ reloadable_template.rb 2009-03-20 13:13:43.866482672 +0100
@@ -41,7 +41,7 @@

def register_template(template)
template.accessible_paths.each do |path|
- @paths[path] = template
+ @paths[path] ||= template #don't clobber previous entry
with *.erb.backup files
end
end

Seems to work fine now. But does this solve/create other problems with
templates and caching?

Hope this helps and possibly prevents some headaches in the future.