horizontal layout now respects positions and spacing correctly

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    compass (0.12.0.alpha.0.e49581c)
+    compass (0.12.0.alpha.0.d8e43bf)
       chunky_png (~> 1.2)
       fssm (>= 0.2.7)
       sass (~> 3.1)

lib/compass/sass_extensions/sprites/image.rb

@@ -48,29 +48,28 @@ module Compass
           File.basename(relative_file, '.png')
         end
 
-        # Value of <tt> $#{name}-repeat </tt> or <tt> $repeat </tt>
+        def get_var_file(var)
-        def repeat
+          options.get_var "#{name}_#{var}"
-          [ "#{name}-repeat", "repeat" ].each { |which|
-            if var = options.get_var(which)
-              return var.value
-            end
-          }
-          "no-repeat"
         end
 
-        # Value of <tt> $#{name}-position </tt> or <tt> $position </tt> defaults o <tt>0px</tt>
+        # Value of <tt> $#{name}-repeat </tt> or <tt> $repeat </tt>
+        def repeat
+          (get_var_file("repeat") || options.get_var("repeat") || Sass::Script::String.new("no-repeat")).value
+        end
+
+        # Value of <tt> $#{name}-position </tt> or <tt> $position </tt> defaults to <tt>0px</tt>
         def position
-          options.get_var("#{name}-position") || options.get_var("position") || Sass::Script::Number.new(0, ["px"])
+          get_var_file("position") || options.get_var("position") || Sass::Script::Number.new(0, ["px"])
         end
         
         # Offset within the sprite
         def offset
           (position.unitless? || position.unit_str == "px") ? position.value : 0
         end
-        
+               
         # Spacing between this image and the next
         def spacing
-          (options.get_var("#{name}-spacing") || options.get_var("spacing") || Sass::Script::Number.new(0)).value
+          (get_var_file("spacing") || options.get_var("spacing") || Sass::Script::Number.new(0, ['px'])).value
         end
 
         # MD5 hash of this file

lib/compass/sass_extensions/sprites/layout_methods.rb

@@ -81,7 +81,7 @@ module Compass
             image.top = image.position.unit_str == '%' ? (@height - image.height) * (image.position.value / 100.0) : image.position.value
             next if index == 0
             last_image = @images[index-1]
-            image.left = last_image.left + last_image.width + [image.offset, last_image.offset].max
+            image.left = last_image.left + last_image.width + [image.spacing, last_image.spacing].max
           end
         end
         

test/units/sprites/image_test.rb

@@ -6,41 +6,36 @@ class SpritesImageTest < Test::Unit::TestCase
   include SpriteHelper
   def setup
     create_sprite_temp
-    file = StringIO.new("images_path = #{@images_src_path.inspect}\n")
-    Compass.add_configuration(file, "sprite_config")
-    @repeat = 'no-repeat'
-    @spacing = 0
-    @position = 100
-    @offset = 100
   end
 
-  let(:sprite_filename) { 'squares/ten-by-ten.png' }
+  SPRITE_FILENAME =  'selectors/ten-by-ten.png'
-  let(:sprite_path) { File.join(@images_tmp_path, sprite_filename) }
-  let(:sprite_name) { File.basename(sprite_filename, '.png') }
   
-  
+  def sprite_path 
-  def options
+    File.join(@images_tmp_path, SPRITE_FILENAME)
-    options = {:offset => @offset}
-    options.stubs(:get_var).with(anything).returns(nil)
-    ::OpenStruct.any_instance.stubs(:unitless?).returns(true)
-    options.stubs(:get_var).with("#{sprite_name}-repeat").returns(::OpenStruct.new(:value => @repeat))
-    options.stubs(:get_var).with("#{sprite_name}-spacing").returns(::OpenStruct.new(:value => @spacing))
-    options.stubs(:get_var).with("#{sprite_name}-position").returns(::OpenStruct.new(:value => @position))
-    options.stubs(:get_var).with("layout").returns(::OpenStruct.new(:value => 'vertical'))
-    options
   end
   
-
+  def sprite_name
+    File.basename(SPRITE_FILENAME, '.png')
+  end
   
-  let(:digest) { Digest::MD5.file(sprite_path).hexdigest }
+  def digest
-
+    Digest::MD5.file(sprite_path).hexdigest
-
+  end
-  let(:image) { Compass::SassExtensions::Sprites::Image.new(sprite_map_test(options), File.join(sprite_filename), options)}
+  
+  def test_map(options ={})
+    options = {'cleanup' => Sass::Script::Bool.new(true), 'layout' => Sass::Script::String.new('vertical')}.merge(options)
+    map = sprite_map_test(options)
+  end
+  
+  def test_image(options ={})
+    test_map(options).images.first
+  end
 
   test 'initialize' do
+    image = test_image
     assert_equal sprite_name, image.name
     assert_equal sprite_path, image.file
-    assert_equal sprite_filename, image.relative_file
+    assert_equal SPRITE_FILENAME, image.relative_file
     assert_equal 10, image.width
     assert_equal 10, image.height
     assert_equal digest, image.digest
@@ -49,44 +44,40 @@ class SpritesImageTest < Test::Unit::TestCase
   end
 
   test 'hover' do
-    assert_equal 'ten-by-ten_hover', image.hover.name
+    assert_equal 'ten-by-ten_hover', test_image.hover.name
   end
 
   test 'no parent' do
-    assert_nil image.parent
+    assert_nil test_image.parent
   end
-
-  test 'image type is nil' do
-    @repeat = nil
-    assert_nil image.repeat
-  end
-
   
   test 'image type is "global"' do
-    @repeat = 'global'
+    image = test_image "ten_by_ten_repeat" => Sass::Script::String.new('global')
-    assert_equal @repeat, image.repeat
+    assert_equal 'global', image.repeat
   end
   
   test 'image type is "no-repeat"' do
-    assert_equal 'no-repeat', image.repeat
+    assert_equal 'no-repeat', test_image.repeat
   end
 
   test 'image position' do
-    assert_equal Sass::Script::Number.new(100, ["px"]).value, image.position.value
+    image = test_image "ten_by_ten_position" => Sass::Script::Number.new(100, ["px"])
+    assert_equal 100, image.position.value
   end
 
   test 'image spacing' do
     @spacing = 10
-    assert_equal @spacing, image.spacing
+    image = test_image "spacing" => Sass::Script::Number.new(100, ["px"])
+    assert_equal 100, image.spacing
   end
   
   test 'offset' do
-    assert_equal @offset, image.offset
+    image = test_image "ten_by_ten_position" => Sass::Script::Number.new(100, ["px"])
+    assert_equal 100, image.offset
   end
 
   test 'neither, uses 0' do
-    @offset = 0
+    img = test_image
-    img = image
     img.position.stubs(:unitless?).returns(false)
     assert_equal 0, img.offset
   end
@@ -105,7 +96,7 @@ class SpritesImageTest < Test::Unit::TestCase
     config.images_path = @images_tmp_path
     config.sprite_load_path = [@images_tmp_path, other_folder]
     Compass.add_configuration(config, "sprite_config")
-    image = Compass::SassExtensions::Sprites::Image.new(sprite_map_test(options), "foo/my.png", options)
+    image = Compass::SassExtensions::Sprites::Image.new(test_map, "foo/my.png", {})
     assert_equal File.join(other_folder, 'foo/my.png'), image.file
     assert_equal 0, image.size
     FileUtils.rm_rf other_folder

test/units/sprites/sprite_map_test.rb

@@ -86,6 +86,16 @@ class SpriteMapTest < Test::Unit::TestCase
     assert_equal [0, 0, 0, 0], @base.images.map(&:left)
   end
   
+  it "should have a vertical layout with spacing" do
+    base = sprite_map_test(@options.merge({"spacing" => Sass::Script::Number.new(10, ['px'])}))
+    assert_equal [0, 20, 40, 60], base.images.map(&:top)
+  end
+  
+  it "should layout vertical with position" do
+    base = sprite_map_test("ten_by_ten_active_position" => Sass::Script::Number.new(10, ['px']))
+    assert_equal [0, 10, 0, 0], base.images.map(&:left)
+  end
+  
   def smart
     options = @options.merge("layout" => Sass::Script::String.new('smart'))
     importer = Compass::SpriteImporter.new
@@ -122,8 +132,9 @@ class SpriteMapTest < Test::Unit::TestCase
   end
 
   # Horizontal tests
-  def horizontal
+  def horizontal(options= {})
     opts = @options.merge("layout" => Sass::Script::String.new('horizontal'))
+    opts.merge!(options)
     sprite_map_test(opts)
   end
   
@@ -139,6 +150,17 @@ class SpriteMapTest < Test::Unit::TestCase
     assert_equal [0, 0, 0, 0], base.images.map(&:top)
   end
   
+  it "should layout horizontaly with spacing" do
+    base = horizontal("spacing" => Sass::Script::Number.new(10, ['px']))
+    assert_equal [0, 20, 40, 60], base.images.map(&:left)
+    assert_equal [0, 0, 0, 0], base.images.map(&:top)
+  end
+  
+  it "should layout horizontaly with position" do
+    base = horizontal("ten_by_ten_active_position" => Sass::Script::Number.new(10, ['px']))
+    assert_equal [0, 10, 0, 0], base.images.map(&:top)
+  end
+  
   it "should generate a horrizontal sprite" do
     base = horizontal
     base.generate