Fixed missing template paths on exception [#1082 state:resolved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>

4 files changed, 37 insertions, 25 deletions

diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb
index 64597b3d39..12808581a3 100644
--- a/actionpack/lib/action_view/template.rb
+++ b/actionpack/lib/action_view/template.rb
@@ -52,15 +52,20 @@ module ActionView #:nodoc:
     end
     memoize :path_without_format_and_extension
 
+    def relative_path
+      path = File.expand_path(filename)
+      path.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '') if defined?(RAILS_ROOT)
+      path
+    end
+    memoize :relative_path
+
     def source
       File.read(filename)
     end
     memoize :source
 
     def method_segment
-      segment = File.expand_path(filename)
-      segment.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}/, '') if defined?(RAILS_ROOT)
-      segment.gsub!(/([^a-zA-Z0-9_])/) { $1.ord }
+      relative_path.to_s.gsub(/([^a-zA-Z0-9_])/) { $1.ord }
     end
     memoize :method_segment
 
@@ -69,7 +74,7 @@ module ActionView #:nodoc:
     rescue Exception => e
       raise e unless filename
       if TemplateError === e
-        e.sub_template_of(filename)
+        e.sub_template_of(self)
         raise e
       else
         raise TemplateError.new(self, view.assigns, e)
diff --git a/actionpack/lib/action_view/template_error.rb b/actionpack/lib/action_view/template_error.rb
index 2368662f31..bcce331773 100644
--- a/actionpack/lib/action_view/template_error.rb
+++ b/actionpack/lib/action_view/template_error.rb
@@ -7,12 +7,14 @@ module ActionView
     attr_reader :original_exception
 
     def initialize(template, assigns, original_exception)
-      @base_path = template.base_path.to_s
-      @assigns, @source, @original_exception = assigns.dup, template.source, original_exception
-      @file_path = template.filename
+      @template, @assigns, @original_exception = template, assigns.dup, original_exception
       @backtrace = compute_backtrace
     end
 
+    def file_name
+      @template.relative_path
+    end
+
     def message
       ActiveSupport::Deprecation.silence { original_exception.message }
     end
@@ -24,7 +26,7 @@ module ActionView
     def sub_template_message
       if @sub_templates
         "Trace of template inclusion: " +
-        @sub_templates.collect { |template| strip_base_path(template) }.join(", ")
+        @sub_templates.collect { |template| template.relative_path }.join(", ")
       else
         ""
       end
@@ -34,18 +36,18 @@ module ActionView
       return unless num = line_number
       num = num.to_i
 
-      source_code = IO.readlines(@file_path)
+      source_code = @template.source.split("\n")
 
       start_on_line = [ num - SOURCE_CODE_RADIUS - 1, 0 ].max
       end_on_line   = [ num + SOURCE_CODE_RADIUS - 1, source_code.length].min
 
       indent = ' ' * indentation
       line_counter = start_on_line
-      return unless source_code = source_code[start_on_line..end_on_line] 
-      
+      return unless source_code = source_code[start_on_line..end_on_line]
+
       source_code.sum do |line|
         line_counter += 1
-        "#{indent}#{line_counter}: #{line}"
+        "#{indent}#{line_counter}: #{line}\n"
       end
     end
 
@@ -63,12 +65,6 @@ module ActionView
         end
     end
 
-    def file_name
-      stripped = strip_base_path(@file_path)
-      stripped.slice!(0,1) if stripped[0] == ?/
-      stripped
-    end
-
     def to_s
       "\n\n#{self.class} (#{message}) #{source_location}:\n" +
         "#{source_extract}\n    #{clean_backtrace.join("\n    ")}\n\n"
@@ -88,12 +84,6 @@ module ActionView
         ]
       end
 
-      def strip_base_path(path)
-        stripped_path = File.expand_path(path).gsub(@base_path, "")
-        stripped_path.gsub!(/^#{Regexp.escape File.expand_path(RAILS_ROOT)}/, '') if defined?(RAILS_ROOT)
-        stripped_path
-      end
-
       def source_location
         if line_number
           "on line ##{line_number} of "
diff --git a/actionpack/test/fixtures/test/sub_template_raise.html.erb b/actionpack/test/fixtures/test/sub_template_raise.html.erb
new file mode 100644
index 0000000000..f38c0bda90
--- /dev/null
+++ b/actionpack/test/fixtures/test/sub_template_raise.html.erb
@@ -0,0 +1 @@
+<%= render :partial => "test/raise" %>
\ No newline at end of file
diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
index f3c8dbcae9..a4ea22ddcb 100644
--- a/actionpack/test/template/render_test.rb
+++ b/actionpack/test/template/render_test.rb
@@ -70,7 +70,23 @@ class ViewRenderTest < Test::Unit::TestCase
   end
 
   def test_render_partial_with_errors
-    assert_raise(ActionView::TemplateError) { @view.render(:partial => "test/raise") }
+    @view.render(:partial => "test/raise")
+    flunk "Render did not raise TemplateError"
+  rescue ActionView::TemplateError => e
+    assert_match "undefined local variable or method `doesnt_exist'", e.message
+    assert_equal "", e.sub_template_message
+    assert_equal "1", e.line_number
+    assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name
+  end
+
+  def test_render_sub_template_with_errors
+    @view.render(:file => "test/sub_template_raise")
+    flunk "Render did not raise TemplateError"
+  rescue ActionView::TemplateError => e
+    assert_match "undefined local variable or method `doesnt_exist'", e.message
+    assert_equal "Trace of template inclusion: #{File.expand_path("#{FIXTURE_LOAD_PATH}/test/sub_template_raise.html.erb")}", e.sub_template_message
+    assert_equal "1", e.line_number
+    assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name
   end
 
   def test_render_partial_collection