Merge pull request #1514 from maul-esel/tag-errors

Consistent error handling in Liquid tags

lib/jekyll/tags/gist.rb

@@ -12,7 +12,15 @@ def render(context)
         gist_id, filename = tag_contents[0], tag_contents[1]
         gist_script_tag(gist_id, filename)
       else
-        "Error parsing gist id"
+        raise ArgumentError.new <<-eos
+Syntax error in tag 'gist' while parsing the following markup:
+
+  #{@markup}
+
+Valid syntax:
+  for public gists:  {% gist 1234567 %}
+  for private gists: {% gist user/1234567 %}
+eos
       end
     end
 

lib/jekyll/tags/include.rb

@@ -77,14 +77,10 @@ def file_read_opts(context)
 
       def render(context)
         dir = File.join(context.registers[:site].source, INCLUDES_DIR)
-        if error = validate_dir(dir, context.registers[:site].safe)
-          return error
-        end
+        validate_dir(dir, context.registers[:site].safe)
 
         file = File.join(dir, @file)
-        if error = validate_file(dir, context.registers[:site].safe)
-          return error
-        end
+        validate_file(file, context.registers[:site].safe)
 
         partial = Liquid::Template.parse(source(file, context))
 
@@ -96,15 +92,15 @@ def render(context)
 
       def validate_dir(dir, safe)
         if File.symlink?(dir) && safe
-          "Includes directory '#{dir}' cannot be a symlink"
+          raise IOError.new "Includes directory '#{dir}' cannot be a symlink"
         end
       end
 
       def validate_file(file, safe)
         if !File.exists?(file)
-          "Included file '#{@file}' not found in '#{INCLUDES_DIR}' directory"
+          raise IOError.new "Included file '#{@file}' not found in '#{INCLUDES_DIR}' directory"
         elsif File.symlink?(file) && safe
-          "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink"
+          raise IOError.new "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink"
         end
       end
 

lib/jekyll/tags/post_url.rb

@@ -50,9 +50,11 @@ def render(context)
           end
         end
 
-        puts "ERROR: post_url: \"#{@orig_post}\" could not be found"
+        raise ArgumentError.new <<-eos
+Could not find post "#{@orig_post}" in tag 'post_url'.
 
-        return "#"
+Make sure the post exists and the name is correct.
+eos
       end
     end
   end

test/test_tags.rb

@@ -19,7 +19,7 @@ def create_post(content, override = {}, converter_class = Jekyll::Converters::Ma
     payload = { "pygments_prefix" => @converter.pygments_prefix,
                 "pygments_suffix" => @converter.pygments_suffix }
 
-    @result = Liquid::Template.parse(content).render(payload, info)
+    @result = Liquid::Template.parse(content).render!(payload, info)
     @result = @converter.convert(@result)
   end
 
@@ -273,22 +273,19 @@ def fill_post(code, override = {})
         end
       end
 
-      context "when invalid" do
-        setup do
-          @gist = "mattr-24081a1d93d2898ecf0f"
-          @filename = "myfile.ext"
-          content = <<CONTENT
+      should "raise ArgumentError when invalid" do
+        @gist = "mattr-24081a1d93d2898ecf0f"
+        @filename = "myfile.ext"
+        content = <<CONTENT
   ---
   title: My Cool Gist
   ---
 
   {% gist #{@gist} #{@filename} %}
 CONTENT
-          create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true})
-        end
 
-        should "write script tag with specific file in gist" do
-          assert_match "Error parsing gist id", @result
+        assert_raise ArgumentError do
+          create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true})
         end
       end
     end
@@ -313,24 +310,23 @@ def fill_post(code, override = {})
     end
 
     context "with blank gist id" do
-      setup do
+      should "raise ArgumentError" do
         content = <<CONTENT
 ---
 title: My Cool Gist
 ---
 
 {% gist %}
 CONTENT
-        create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true})
-      end
 
-      should "output error message" do
-        assert_match "Error parsing gist id", @result
+        assert_raise ArgumentError do
+          create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true})
+        end
       end
     end
 
     context "with invalid gist id" do
-      setup do
+      should "raise ArgumentError" do
         invalid_gist = 'invalid'
         content = <<CONTENT
 ---
@@ -339,11 +335,10 @@ def fill_post(code, override = {})
 
 {% gist #{invalid_gist} %}
 CONTENT
-        create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true})
-      end
 
-      should "output error message" do
-        assert_match "Error parsing gist id", @result
+        assert_raise ArgumentError do
+          create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true})
+        end
       end
     end
   end