Fixed geometry compatability issues and excessive stderr printing.

lib/paperclip.rb

@@ -58,6 +58,18 @@ module Paperclip
       File.join(*path)
       File.join(*path)
     end
     end
 
 
+    def run cmd, params = "", expected_outcodes = 0
+      output = `#{%Q[#{path_for_command(cmd)} #{params} 2>#{bit_bucket}].gsub(/\s+/, " ")}`
+      unless [expected_outcodes].flatten.include?($?.exitstatus)
+        raise PaperclipCommandLineError, "Error while running #{cmd}"
+      end
+      output
+    end
+
+    def bit_bucket
+      File.exists?("/dev/null") ? "/dev/null" : "NUL"
+    end
+
     def included base #:nodoc:
     def included base #:nodoc:
       base.extend ClassMethods
       base.extend ClassMethods
     end
     end
@@ -66,6 +78,9 @@ module Paperclip
   class PaperclipError < StandardError #:nodoc:
   class PaperclipError < StandardError #:nodoc:
   end
   end
 
 
+  class PaperclipCommandLineError < StandardError #:nodoc:
+  end
+
   class NotIdentifiedByImageMagickError < PaperclipError #:nodoc:
   class NotIdentifiedByImageMagickError < PaperclipError #:nodoc:
   end
   end
 
 

lib/paperclip/geometry.rb

 module Paperclip
 module Paperclip
-  
+
   # Defines the geometry of an image.
   # Defines the geometry of an image.
   class Geometry
   class Geometry
     attr_accessor :height, :width, :modifier
     attr_accessor :height, :width, :modifier
 
 
     # Gives a Geometry representing the given height and width
     # Gives a Geometry representing the given height and width
     def initialize width = nil, height = nil, modifier = nil
     def initialize width = nil, height = nil, modifier = nil
-      height = nil if height == ""
-      width  = nil if width  == ""
-      @height = (height || width).to_f
-      @width  = (width  || height).to_f
+      @height = height.to_f
+      @width  = width.to_f
       @modifier = modifier
       @modifier = modifier
     end
     end
 
 
@@ -17,13 +15,18 @@ module Paperclip
     # File or path.
     # File or path.
     def self.from_file file
     def self.from_file file
       file = file.path if file.respond_to? "path"
       file = file.path if file.respond_to? "path"
-      parse(`#{Paperclip.path_for_command('identify')} "#{file}"`) ||
+      geometry = begin
+                   Paperclip.run("identify", %Q[-format "%wx%h" "#{file}"])
+                 rescue PaperclipCommandLineError
+                   ""
+                 end
+      parse(geometry) ||
         raise(NotIdentifiedByImageMagickError.new("#{file} is not recognized by the 'identify' command."))
         raise(NotIdentifiedByImageMagickError.new("#{file} is not recognized by the 'identify' command."))
     end
     end
 
 
     # Parses a "WxH" formatted string, where W is the width and H is the height.
     # Parses a "WxH" formatted string, where W is the width and H is the height.
     def self.parse string
     def self.parse string
-      if match = (string && string.match(/\b(\d*)x(\d*)\b([\>\<\#\@\%^!])?/))
+      if match = (string && string.match(/\b(\d*)x?(\d*)\b([\>\<\#\@\%^!])?/))
         Geometry.new(*match[1,3])
         Geometry.new(*match[1,3])
       end
       end
     end
     end
@@ -60,7 +63,10 @@ module Paperclip
 
 
     # Returns the width and height in a format suitable to be passed to Geometry.parse
     # Returns the width and height in a format suitable to be passed to Geometry.parse
     def to_s
     def to_s
-      "%dx%d%s" % [width, height, modifier]
+      s = ""
+      s << width.to_i.to_s if width > 0
+      s << "x#{height.to_i}#{modifier}" if height > 0
+      s
     end
     end
 
 
     # Same as to_s
     # Same as to_s
@@ -76,7 +82,6 @@ module Paperclip
     # overhanging image would be cropped. Useful for square thumbnail images. The cropping 
     # overhanging image would be cropped. Useful for square thumbnail images. The cropping 
     # is weighted at the center of the Geometry.
     # is weighted at the center of the Geometry.
     def transformation_to dst, crop = false
     def transformation_to dst, crop = false
-
       if crop
       if crop
         ratio = Geometry.new( dst.width / self.width, dst.height / self.height )
         ratio = Geometry.new( dst.width / self.width, dst.height / self.height )
         scale_geometry, scale = scaling(dst, ratio)
         scale_geometry, scale = scaling(dst, ratio)
@@ -84,7 +89,7 @@ module Paperclip
       else
       else
         scale_geometry        = dst.to_s
         scale_geometry        = dst.to_s
       end
       end
-      
+
       [ scale_geometry, crop_geometry ]
       [ scale_geometry, crop_geometry ]
     end
     end
 
 

lib/paperclip/thumbnail.rb

@@ -48,15 +48,15 @@ module Paperclip
       dst.binmode
       dst.binmode
 
 
       command = <<-end_command
       command = <<-end_command
-        #{ Paperclip.path_for_command('convert') }
         "#{ File.expand_path(src.path) }[0]"
         "#{ File.expand_path(src.path) }[0]"
         #{ transformation_command }
         #{ transformation_command }
         "#{ File.expand_path(dst.path) }"
         "#{ File.expand_path(dst.path) }"
       end_command
       end_command
-      success = system(command.gsub(/\s+/, " "))
 
 
-      if !success && $?.exitstatus != 0 && @whiny_thumbnails
-        raise PaperclipError, "There was an error processing this thumbnail"
+      begin
+        success = Paperclip.run("convert", command.gsub(/\s+/, " "))
+      rescue PaperclipCommandLineError
+        raise PaperclipError, "There was an error processing the thumbnail for #{@basename}" if @whiny_thumbnails
       end
       end
 
 
       dst
       dst

test/geometry_test.rb

@@ -12,15 +12,15 @@ class GeometryTest < Test::Unit::TestCase
       assert_equal 768, @geo.height
       assert_equal 768, @geo.height
     end
     end
 
 
-    should "correctly create a square if the height dimension is missing" do
+    should "set height to 0 if height dimension is missing" do
       assert @geo = Paperclip::Geometry.new(1024)
       assert @geo = Paperclip::Geometry.new(1024)
       assert_equal 1024, @geo.width
       assert_equal 1024, @geo.width
-      assert_equal 1024, @geo.height
+      assert_equal 0, @geo.height
     end
     end
 
 
-    should "correctly create a square if the width dimension is missing" do
+    should "set width to 0 if width dimension is missing" do
       assert @geo = Paperclip::Geometry.new(nil, 768)
       assert @geo = Paperclip::Geometry.new(nil, 768)
-      assert_equal 768, @geo.width
+      assert_equal 0, @geo.width
       assert_equal 768, @geo.height
       assert_equal 768, @geo.height
     end
     end
 
 
@@ -32,14 +32,20 @@ class GeometryTest < Test::Unit::TestCase
 
 
     should "be generated from a xH-formatted string" do
     should "be generated from a xH-formatted string" do
       assert @geo = Paperclip::Geometry.parse("x600")
       assert @geo = Paperclip::Geometry.parse("x600")
-      assert_equal 600, @geo.width
+      assert_equal 0, @geo.width
       assert_equal 600, @geo.height
       assert_equal 600, @geo.height
     end
     end
-      
+
     should "be generated from a Wx-formatted string" do
     should "be generated from a Wx-formatted string" do
       assert @geo = Paperclip::Geometry.parse("800x")
       assert @geo = Paperclip::Geometry.parse("800x")
       assert_equal 800, @geo.width
       assert_equal 800, @geo.width
-      assert_equal 800, @geo.height
+      assert_equal 0, @geo.height
+    end
+
+    should "be generated from a W-formatted string" do
+      assert @geo = Paperclip::Geometry.parse("800")
+      assert_equal 800, @geo.width
+      assert_equal 0, @geo.height
     end
     end
 
 
     should "ensure the modifier is nil if only one dimension present" do
     should "ensure the modifier is nil if only one dimension present" do
@@ -65,6 +71,26 @@ class GeometryTest < Test::Unit::TestCase
       assert_equal "123x456>", @src.transformation_to(@dst).to_s
       assert_equal "123x456>", @src.transformation_to(@dst).to_s
     end
     end
 
 
+    should "generate correct ImageMagick formatting string for W-formatted string" do
+      assert @geo = Paperclip::Geometry.parse("800")
+      assert_equal "800", @geo.to_s
+    end
+
+    should "generate correct ImageMagick formatting string for Wx-formatted string" do
+      assert @geo = Paperclip::Geometry.parse("800x")
+      assert_equal "800", @geo.to_s
+    end
+
+    should "generate correct ImageMagick formatting string for xH-formatted string" do
+      assert @geo = Paperclip::Geometry.parse("x600")
+      assert_equal "x600", @geo.to_s
+    end
+
+    should "generate correct ImageMagick formatting string for WxH-formatted string" do
+      assert @geo = Paperclip::Geometry.parse("800x600")
+      assert_equal "800x600", @geo.to_s
+    end
+
     should "be generated from a file" do
     should "be generated from a file" do
       file = File.join(File.dirname(__FILE__), "fixtures", "5k.png")
       file = File.join(File.dirname(__FILE__), "fixtures", "5k.png")
       file = File.new(file)
       file = File.new(file)

test/paperclip_test.rb

 require 'test/helper.rb'
 require 'test/helper.rb'
 
 
 class PaperclipTest < Test::Unit::TestCase
 class PaperclipTest < Test::Unit::TestCase
+  context "Calling Paperclip.run" do
+    should "execute the right command" do
+      Paperclip.expects(:path_for_command).with("convert").returns("/usr/bin/convert")
+      Paperclip.expects(:bit_bucket).returns("/dev/null")
+      Paperclip.expects(:"`").with("/usr/bin/convert one.jpg two.jpg 2>/dev/null")
+      Paperclip.run("convert", "one.jpg two.jpg")
+    end
+  end
+
+  context "Paperclip.bit_bucket" do
+    context "on systems without /dev/null" do
+      setup do
+        File.expects(:exists?).with("/dev/null").returns(false)
+      end
+      
+      should "return 'NUL'" do
+        assert_equal "NUL", Paperclip.bit_bucket
+      end
+    end
+
+    context "on systems with /dev/null" do
+      setup do
+        File.expects(:exists?).with("/dev/null").returns(true)
+      end
+      
+      should "return '/dev/null'" do
+        assert_equal "/dev/null", Paperclip.bit_bucket
+      end
+    end
+  end
+
   context "An ActiveRecord model with an 'avatar' attachment" do
   context "An ActiveRecord model with an 'avatar' attachment" do
     setup do
     setup do
       rebuild_model :path => "tmp/:class/omg/:style.:extension"
       rebuild_model :path => "tmp/:class/omg/:style.:extension"

test/thumbnail_test.rb

@@ -96,7 +96,7 @@ class ThumbnailTest < Test::Unit::TestCase
       end
       end
 
 
       should "send the right command to convert when sent #make" do
       should "send the right command to convert when sent #make" do
-        @thumb.expects(:system).with do |arg|
+        Paperclip.expects(:"`").with do |arg|
           arg.match %r{convert\s+"#{File.expand_path(@thumb.file.path)}\[0\]"\s+-resize\s+\"x50\"\s+-crop\s+\"100x50\+114\+0\"\s+\+repage\s+".*?"}
           arg.match %r{convert\s+"#{File.expand_path(@thumb.file.path)}\[0\]"\s+-resize\s+\"x50\"\s+-crop\s+\"100x50\+114\+0\"\s+\+repage\s+".*?"}
         end
         end
         @thumb.make
         @thumb.make
@@ -118,7 +118,7 @@ class ThumbnailTest < Test::Unit::TestCase
       end
       end
 
 
       should "send the right command to convert when sent #make" do
       should "send the right command to convert when sent #make" do
-        @thumb.expects(:system).with do |arg|
+        Paperclip.expects(:"`").with do |arg|
           arg.match %r{convert\s+"#{File.expand_path(@thumb.file.path)}\[0\]"\s+-resize\s+"x50"\s+-crop\s+"100x50\+114\+0"\s+\+repage\s+-strip\s+-depth\s+8\s+".*?"}
           arg.match %r{convert\s+"#{File.expand_path(@thumb.file.path)}\[0\]"\s+-resize\s+"x50"\s+-crop\s+"100x50\+114\+0"\s+\+repage\s+-strip\s+-depth\s+8\s+".*?"}
         end
         end
         @thumb.make
         @thumb.make