czarek
2009-Mar-19 18:01 UTC
upgrade to 2.3.2: render started matching vim-patchmode *.orig files
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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Cezary BagiĆski
2009-Mar-20 13:00 UTC
Re: upgrade to 2.3.2: render started matching vim-patchmode *.orig files
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. czarek wrote:> 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 > > > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---