Merge branch 'master' of git://github.com/bjhess/paperclip into bjhess

Conflicts:

	lib/paperclip/attachment.rb
	lib/paperclip/thumbnail.rb

lib/paperclip.rb

@@ -106,6 +106,11 @@ module Paperclip
     # * +whiny_thumbnails+: Will raise an error if Paperclip cannot process thumbnails of an
     #   uploaded image. This will ovrride the global setting for this attachment. 
     #   Defaults to true. 
+    # * +thumbnail_convert_options+: When creating thumbnails, use this free-form options
+    #   field to pass in various convert command options.  Typical options are "-strip" to
+    #   remove all Exif data from the image (save space for thumbnails and avatars) or
+    #   "-depth 8" to specify the bit depth of the resulting conversion.  See ImageMagick
+    #   convert documentation for more options: (http://www.imagemagick.org/script/convert.php)
     # * +storage+: Chooses the storage backend where the files will be stored. The current
     #   choices are :filesystem and :s3. The default is :filesystem. Make sure you read the
     #   documentation for Paperclip::Storage::Filesystem and Paperclip::Storage::S3

lib/paperclip/attachment.rb

@@ -239,6 +239,10 @@ module Paperclip
       self.extend(@storage_module)
     end
 
+    def extra_options_for(style) #:nodoc:
+      [ convert_options[style], convert_options[:all] ].compact.join(" ")
+    end
+
     def post_process #:nodoc:
       return if @queued_for_write[:original].nil?
       logger.info("[paperclip] Post-processing #{name}")
@@ -249,7 +253,7 @@ module Paperclip
           @queued_for_write[name] = Thumbnail.make(@queued_for_write[:original], 
                                                    dimensions,
                                                    format, 
-                                                   convert_options[name],
+                                                   extra_options_for(name),
                                                    @whiny_thumnails)
         rescue PaperclipError => e
           @errors << e.message if @whiny_thumbnails

lib/paperclip/thumbnail.rb

@@ -8,7 +8,8 @@ module Paperclip
     # will attempt to transform the image into one defined by +target_geometry+
     # which is a "WxH"-style string. +format+ will be inferred from the +file+
     # unless specified. Thumbnail creation will raise no errors unless
-    # +whiny_thumbnails+ is true (which it is, by default.
+    # +whiny_thumbnails+ is true (which it is, by default. If +convert_options+ is
+    # set, the options will be appended to the convert command upon image conversion 
     def initialize file, target_geometry, format = nil, convert_options = nil, whiny_thumbnails = true
       @file             = file
       @crop             = target_geometry[-1,1] == '#'
@@ -33,6 +34,11 @@ module Paperclip
     def crop?
       @crop
     end
+    
+    # Returns true if the image is meant to make use of additional convert options.
+    def convert_options?
+      not @convert_options.blank?
+    end
 
     # Performs the conversion of the +file+ into a thumbnail. Returns the Tempfile
     # that contains the new image.
@@ -49,7 +55,7 @@ module Paperclip
       end_command
       success = system(command.gsub(/\s+/, " "))
 
-      if success && $?.exitstatus != 0 && @whiny_thumbnails
+      if !success && $?.exitstatus != 0 && @whiny_thumbnails
         raise PaperclipError, "There was an error processing this thumbnail"
       end
 
@@ -62,7 +68,7 @@ module Paperclip
       scale, crop = @current_geometry.transformation_to(@target_geometry, crop?)
       trans = "-scale \"#{scale}\""
       trans << " -crop \"#{crop}\" +repage" if crop
-      trans << " #{convert_options}" if convert_options
+      trans << " #{convert_options}" if convert_options?
       trans
     end
   end

test/attachment_test.rb

@@ -106,6 +106,47 @@ class AttachmentTest < Test::Unit::TestCase
     end
   end
 
+  context "An attachment with :convert_options" do
+    setup do
+      rebuild_model :styles => {
+                      :thumb => "100x100",
+                      :large => "400x400"
+                    },
+                    :convert_options => {
+                      :all => "-do_stuff",
+                      :thumb => "-thumbnailize"
+                    }
+      @dummy = Dummy.new
+    end
+
+    should "report the correct options when sent #extra_options_for(:thumb)" do
+      assert_equal "-thumbnailize -do_stuff", @dummy.avatar.send(:extra_options_for, :thumb), @dummy.avatar.convert_options.inspect
+    end
+
+    should "report the correct options when sent #extra_options_for(:large)" do
+      assert_equal "-do_stuff", @dummy.avatar.send(:extra_options_for, :large)
+    end
+
+    context "when given a file" do
+      setup do
+        @file = File.new(File.join(File.dirname(__FILE__),
+                                   "fixtures",
+                                   "5k.png"))
+        Paperclip::Thumbnail.stubs(:make)
+        [:thumb, :large].each do |style|
+          @dummy.avatar.stubs(:extra_options_for).with(style)
+        end
+      end
+
+      [:thumb, :large].each do |style|
+        should "call extra_options_for(#{style})" do
+          @dummy.avatar.expects(:extra_options_for).with(style)
+          @dummy.avatar = @file
+        end
+      end
+    end
+  end
+
   context "Assigning an attachment" do
     setup do
       rebuild_model

test/integration_test.rb

@@ -91,6 +91,38 @@ class IntegrationTest < Test::Unit::TestCase
     end
   end
   
+  context "A model with no thumbnail_convert_options setting" do
+    setup do
+      rebuild_model :styles => { :large => "300x300>",
+                                 :medium => "100x100",
+                                 :thumb => ["32x32#", :gif] },
+                    :default_style => :medium,
+                    :url => "/:attachment/:class/:style/:id/:basename.:extension",
+                    :path => ":rails_root/tmp/:attachment/:class/:style/:id/:basename.:extension"
+      @dummy     = Dummy.new
+    end
+    
+    should "have its definition return nil when asked about convert_options" do
+      assert ! Dummy.attachment_definitions[:avatar][:thumbnail_convert_options]
+    end
+
+    context "redefined to have convert_options setting" do
+      setup do
+        rebuild_model :styles => { :large => "300x300>",
+                                   :medium => "100x100",
+                                   :thumb => ["32x32#", :gif] },
+                      :thumbnail_convert_options => "-strip -depth 8",
+                      :default_style => :medium,
+                      :url => "/:attachment/:class/:style/:id/:basename.:extension",
+                      :path => ":rails_root/tmp/:attachment/:class/:style/:id/:basename.:extension"
+      end
+
+      should "have its definition return convert_options value when asked about convert_options" do
+        assert_equal "-strip -depth 8", Dummy.attachment_definitions[:avatar][:thumbnail_convert_options]
+      end
+    end
+  end
+  
   context "A model with a filesystem attachment" do
     setup do
       rebuild_model :styles => { :large => "300x300>",

test/thumbnail_test.rb

@@ -90,6 +90,10 @@ class ThumbnailTest < Test::Unit::TestCase
       should "have whiny_thumbnails turned on by default" do
         assert @thumb.whiny_thumbnails
       end
+      
+      should "have convert_options set to nil by default" do
+        assert_equal nil, @thumb.convert_options
+      end
 
       should "send the right command to convert when sent #make" do
         @thumb.expects(:system).with do |arg|
@@ -103,5 +107,39 @@ class ThumbnailTest < Test::Unit::TestCase
         assert_match /100x50/, `identify #{dst.path}`
       end
     end
+    
+    context "being thumbnailed with convert options set" do
+      setup do
+        @thumb = Paperclip::Thumbnail.new(@file, "100x50#", format=nil, convert_options="-strip -depth 8", whiny_thumbnails=true)
+      end
+
+      should "have convert_options value set" do
+        assert_equal "-strip -depth 8", @thumb.convert_options
+      end
+
+      should "send the right command to convert when sent #make" do
+        @thumb.expects(:system).with do |arg|
+          arg.match %r{convert\s+"#{File.expand_path(@thumb.file.path)}"\s+-scale\s+\"x50\"\s+-crop\s+\"100x50\+114\+0\"\s+\+repage\s+-strip\s+-depth\s+8\s+".*?"}
+        end
+        @thumb.make
+      end
+
+      should "create the thumbnail when sent #make" do
+        dst = @thumb.make
+        assert_match /100x50/, `identify #{dst.path}`
+      end
+      
+      context "redefined to have bad convert_options setting" do
+        setup do
+          @thumb = Paperclip::Thumbnail.new(@file, "100x50#", format=nil, convert_options="-this-aint-no-option", whiny_thumbnails=true)
+        end
+
+        should "error when trying to create the thumbnail" do
+          assert_raises(Paperclip::PaperclipError) do
+            @thumb.make
+          end
+        end
+      end      
+    end
   end
 end