Skip to content
This repository
Browse code

Fix sorting of helpers from different paths

When more than one directory for helpers is provided to a controller, it
should preserver the order of directories. Given 2 paths:

    MyController.helpers_paths = ["dir1/helpers", "dir2/helpers"]

helpers from dir1 should be loaded first. Before this commit, all
helpers were mixed and then sorted alphabetically, which essentially
would require to rename helpers to get desired order.

This is a problem especially for engines, where you would like to be
able to predict accurately which engine helpers will load first.
  • Loading branch information...
commit 26ddf2fae91a930aeb1ab6d90d039e50e5d4a009 1 parent 1f2ee5d
Piotr Sarnacki authored May 27, 2012
4  actionpack/lib/action_controller/metal/helpers.rb
@@ -96,9 +96,9 @@ def all_helpers_from_path(path)
96 96
         helpers = []
97 97
         Array.wrap(path).each do |_path|
98 98
           extract  = /^#{Regexp.quote(_path.to_s)}\/?(.*)_helper.rb$/
99  
-          helpers += Dir["#{_path}/**/*_helper.rb"].map { |file| file.sub(extract, '\1') }
  99
+          names = Dir["#{_path}/**/*_helper.rb"].map { |file| file.sub(extract, '\1') }
  100
+          helpers += names.sort
100 101
         end
101  
-        helpers.sort!
102 102
         helpers.uniq!
103 103
         helpers
104 104
       end
30  actionpack/test/controller/helper_test.rb
@@ -50,12 +50,42 @@ def lib
50 50
 class MeTooController < JustMeController
51 51
 end
52 52
 
  53
+class HelpersPathsController < ActionController::Base
  54
+  paths = ["helpers1_pack", "helpers2_pack"].map do |path|
  55
+    File.join(File.expand_path('../../fixtures', __FILE__), path)
  56
+  end
  57
+  $:.unshift(*paths)
  58
+
  59
+  self.helpers_path = paths
  60
+  helper :all
  61
+
  62
+  def index
  63
+    render :inline => "<%= conflicting_helper %>"
  64
+  end
  65
+end
  66
+
53 67
 module LocalAbcHelper
54 68
   def a() end
55 69
   def b() end
56 70
   def c() end
57 71
 end
58 72
 
  73
+class HelperPathsTest < ActiveSupport::TestCase
  74
+  def setup
  75
+    @request    = ActionController::TestRequest.new
  76
+    @response   = ActionController::TestResponse.new
  77
+  end
  78
+
  79
+  def test_helpers_paths_priority
  80
+    request  = ActionController::TestRequest.new
  81
+    responses = HelpersPathsController.action(:index).call(request.env)
  82
+
  83
+    # helpers2_pack was given as a second path, so pack2_helper should be
  84
+    # included as the second one
  85
+    assert_equal "pack2", responses.last.body
  86
+  end
  87
+end
  88
+
59 89
 class HelperTest < ActiveSupport::TestCase
60 90
   class TestController < ActionController::Base
61 91
     attr_accessor :delegate_attr
5  actionpack/test/fixtures/helpers1_pack/pack1_helper.rb
... ...
@@ -0,0 +1,5 @@
  1
+module Pack1Helper
  2
+  def conflicting_helper
  3
+    "pack1"
  4
+  end
  5
+end
5  actionpack/test/fixtures/helpers2_pack/pack2_helper.rb
... ...
@@ -0,0 +1,5 @@
  1
+module Pack2Helper
  2
+  def conflicting_helper
  3
+    "pack2"
  4
+  end
  5
+end

0 notes on commit 26ddf2f

Please sign in to comment.
Something went wrong with that request. Please try again.