Skip to content
This repository

Fixed the application_controller require_dependency path generated by the app generator #6643

Merged
merged 1 commit into from almost 2 years ago

3 participants

Fred Wu Andrew White José Valim
Fred Wu
fredwu commented

application_controller.rb is always in the same location, this patch fixes an issue where the path generated in namespaced controllers isn't correct.

e.g. require_dependency "test_app/application_controller" instead of require_dependency "test_app/some/module/application_controller".

Andrew White pixeltrix commented on the diff
railties/lib/rails/generators/named_base.rb
@@ -84,10 +84,11 @@ def namespaced_file_path
84 84
         end
85 85
 
86 86
         def namespaced_class_path
87  
-          @namespaced_class_path ||= begin
88  
-            namespace_path = namespace.name.split("::").map {|m| m.underscore }
89  
-            namespace_path + @class_path
90  
-          end
  87
+          @namespaced_class_path ||= [namespace_path] + @class_path
  88
+        end
  89
+
  90
+        def namespace_path
7
Andrew White Owner
pixeltrix added a note

I'm wondering whether this should be call namespaced_path or namespaced_root_path - @josevalim, @drogus wdyt?

José Valim Owner
josevalim added a note

I found namespace_path good enough but namespaced_path is also reasonable.

Andrew White Owner
pixeltrix added a note

namespaced_path is consistent with namespaced_class_path and namespaced_file_path.

José Valim Owner
josevalim added a note
Andrew White Owner
pixeltrix added a note

Already changed: 45427e2

Fred Wu
fredwu added a note

I know this is trivial, but I think namespace_path is actually more accurate. The 'namespace' in namespaced_class_path and namespaced_file_path is more of a prefix to describe the namedspaced nature of the paths. But, in the case of namespace_path - the path is not namespaced per se - it is the namespace itself. What do you think? :) @josevalim @pixeltrix

Andrew White Owner
pixeltrix added a note

@fredwu it may be true that it's more accurate when talking about in terms of English grammar but I'd go with consistency since not everyone's grammar is up to scratch. I'd include some people who's first language is English in that set - no slight on those whose native language isn't English.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
José Valim josevalim merged commit 7682f70 into from
José Valim josevalim closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Jun 06, 2012
Fred Wu Fixed the application_controller require_dependency path generated by…
… the app generator
686966a
This page is out of date. Refresh to see the latest.
9  railties/lib/rails/generators/named_base.rb
@@ -84,10 +84,11 @@ def namespaced_file_path
84 84
         end
85 85
 
86 86
         def namespaced_class_path
87  
-          @namespaced_class_path ||= begin
88  
-            namespace_path = namespace.name.split("::").map {|m| m.underscore }
89  
-            namespace_path + @class_path
90  
-          end
  87
+          @namespaced_class_path ||= [namespace_path] + @class_path
  88
+        end
  89
+
  90
+        def namespace_path
  91
+          @namespace_path ||= namespace.name.split("::").map {|m| m.underscore }[0]
91 92
         end
92 93
 
93 94
         def class_name
2  railties/lib/rails/generators/rails/controller/templates/controller.rb
... ...
@@ -1,5 +1,5 @@
1 1
 <% if namespaced? -%>
2  
-require_dependency "<%= namespaced_file_path %>/application_controller"
  2
+require_dependency "<%= namespace_path %>/application_controller"
3 3
 
4 4
 <% end -%>
5 5
 <% module_namespacing do -%>
4  railties/test/generators/namespaced_generators_test.rb
@@ -38,7 +38,9 @@ def test_skipping_namespace
38 38
 
39 39
   def test_namespaced_controller_with_additional_namespace
40 40
     run_generator ["admin/account"]
41  
-    assert_file "app/controllers/test_app/admin/account_controller.rb", /module TestApp/, /  class Admin::AccountController < ApplicationController/
  41
+    assert_file "app/controllers/test_app/admin/account_controller.rb", /module TestApp/, /  class Admin::AccountController < ApplicationController/ do |contents|
  42
+      assert_match %r(require_dependency "test_app/application_controller"), contents
  43
+    end
42 44
   end
43 45
 
44 46
   def test_helpr_is_also_namespaced
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.