refactoring and code cleanup

Guardfile

@@ -1,12 +1,16 @@
-guard :test do
+group :tests do
-  watch(%r{^lib/(.+)\.rb$}) { |m| "test/#{m[1]}_test.rb" }
+  guard :test do
-  watch(%r{^test/.+_test\.rb$})
+    watch(%r{^lib/(.+)\.rb$}) { |m| "test/#{m[1]}_test.rb" }
-  watch(%r{^test/units/.+_test\.rb$})
+    watch(%r{^test/.+_test\.rb$})
-  watch('test/test_helper.rb') { "test" }
+    watch(%r{^test/units/.+_test\.rb$})
+    watch('test/test_helper.rb') { "test" }
+  end
 end
 
-guard :cucumber do
+group :features do
-  watch(%r{^features/.+\.feature$})
+  guard :cucumber do
-  watch(%r{^features/support/.+$}) { 'features' }
+    watch(%r{^features/.+\.feature$})
-  watch(%r{^features/step_definitions/(.+)_steps\.rb$}) { |m| Dir[File.join("**/#{m[1]}.feature")][0] || 'features' }
+    watch(%r{^features/support/.+$}) { 'features' }
+    watch(%r{^features/step_definitions/(.+)_steps\.rb$}) { |m| Dir[File.join("**/#{m[1]}.feature")][0] || 'features' }
+  end
 end

lib/compass/sass_extensions/sprites.rb

@@ -2,11 +2,13 @@ require 'digest/md5'
 require 'compass/sprite_importer'
 
 module Compass
+  class SpriteException < Exception; end
   module SassExtensions
     module Sprites
     end
   end
 end
+
 require 'compass/sass_extensions/sprites/image_row'
 require 'compass/sass_extensions/sprites/row_fitter'
 require 'compass/sass_extensions/sprites/image'

lib/compass/sass_extensions/sprites/engines/chunky_png_engine.rb

@@ -13,7 +13,7 @@ module Compass
           @canvas = ChunkyPNG::Image.new(width, height, ChunkyPNG::Color::TRANSPARENT)
           images.each do |image|
             input_png  = ChunkyPNG::Image.from_file(image.file)
-            if image.repeat == "no-repeat"
+            if image.no_repeat?
               canvas.replace! input_png, image.left, image.top
             else
               x = image.left - (image.left / image.width).ceil * image.width

lib/compass/sass_extensions/sprites/image.rb

@@ -6,6 +6,11 @@ module Compass
         TARGET = %r{[_-]target$}
         HOVER = %r{[_-]hover$}
         PARENT = %r{(.+)[-_](.+)$}
+
+        REPEAT_X = 'repeat-x'
+        NO_REPEAT = 'no-repeat'
+
+        VALID_REPEATS = [REPEAT_X, NO_REPEAT]
         
         attr_reader :relative_file, :options, :base
         attr_accessor :top, :left
@@ -53,8 +58,23 @@ module Compass
         end
 
         # Value of <tt> $#{name}-repeat </tt> or <tt> $repeat </tt>
-        def repeat
+        def repeat 
-         @repeat ||= (get_var_file("repeat") || options.get_var("repeat") || Sass::Script::String.new("no-repeat")).value
+          @repeat ||= begin 
+            rep = (get_var_file("repeat") || options.get_var("repeat") || Sass::Script::String.new(NO_REPEAT)).value
+            unless VALID_REPEATS.include? rep
+              raise SpriteException, "Invalid option for repeat \"#{rep}\" - valid options are #{VALID_REPEATS.join(', ')}"
+            end
+
+            rep
+          end
+        end
+
+        def repeat_x?
+          repeat == REPEAT_X
+        end
+
+        def no_repeat?
+          repeat == NO_REPEAT
         end
 
         # Value of <tt> $#{name}-position </tt> or <tt> $position </tt> defaults to <tt>0px</tt>

lib/compass/sass_extensions/sprites/layout_methods.rb

@@ -5,6 +5,7 @@ module Compass
         HORIZONTAL = 'horizontal'
         DIAGONAL = 'diagonal'
         SMART = 'smart'
+        VERTICAL = 'vertical'
         
         def smart?
           layout == SMART
@@ -17,6 +18,10 @@ module Compass
         def diagonal?
           layout == DIAGONAL
         end
+
+        def vertical?
+          layout == VERTICAL
+        end
         
         def layout
           @layout ||= @kwargs.get_var('layout').value
@@ -43,11 +48,13 @@ module Compass
                 b.size <=> a.size
               end
             end
+
             @width = width_for_vertical_layout
             calulate_vertical_postions
             @height = height_for_vertical_layout
           end
         end
+
         
         def calculate_smart_positions
           fitter = ::Compass::SassExtensions::Sprites::RowFitter.new(@images)

lib/compass/sass_extensions/sprites/sprite_map.rb

@@ -49,7 +49,14 @@ module Compass
         end
 
         def inspect
-          to_s
+          puts 'images'
+          @images.each do |img|
+            puts img.file
+          end
+          puts "options"
+          @kwargs.each do |k,v|
+            puts "#{k}:#{v}"
+          end
         end
 
         def to_s(kwargs = self.kwargs)

lib/compass/sass_extensions/sprites/sprite_methods.rb

@@ -27,8 +27,7 @@ module Compass
         # Creates the Sprite::Image objects for each image and calculates the width
         def init_images
           @images = image_names.collect do |relative_file|
-            image = Compass::SassExtensions::Sprites::Image.new(self, relative_file, kwargs)
+            Image.new(self, relative_file, kwargs)
-            image
           end
         end
         

test/integrations/sprites_test.rb

@@ -374,13 +374,13 @@ class SpritesTest < Test::Unit::TestCase
 
   it "should repeat the image" do
     css = render <<-SCSS
-      $squares-repeat: repeat;
+      $squares-repeat: repeat-x;
       @import "squares/*.png";
       @include all-squares-sprites;
     SCSS
     assert_correct css, <<-CSS
       .squares-sprite, .squares-ten-by-ten, .squares-twenty-by-twenty {
-        background: url('/squares-sbab486c25a.png') no-repeat;
+        background: url('/squares-s13833277b3.png') no-repeat;
       }
       
       .squares-ten-by-ten {

test/test_helper.rb

@@ -49,7 +49,7 @@ module SpriteHelper
     path, name = Compass::SpriteImporter.path_and_name(uri)
     sprite_names = Compass::SpriteImporter.sprite_names(uri)
     sass_engine = Compass::SpriteImporter.sass_engine(uri, name, importer, options)
-    map = Compass::SassExtensions::Sprites::SpriteMap.new(sprite_names.map{|n| "selectors/#{n}.png"}, path, name, sass_engine, options)
+    map = Compass::SassExtensions::Sprites::SpriteMap.new(sprite_names.map{|n| uri.gsub('*', n)}, path, name, sass_engine, options)
     map.options = {:compass => {:logger => Compass::NullLogger.new}}
     map
   end

test/units/sprites/image_test.rb

@@ -4,10 +4,15 @@ require 'ostruct'
 
 class SpritesImageTest < Test::Unit::TestCase
   include SpriteHelper
+
   def setup
     create_sprite_temp
   end
 
+  def teardown
+    clean_up_sprites
+  end
+
   SPRITE_FILENAME =  'selectors/ten-by-ten.png'
   
   def sprite_path 
@@ -51,13 +56,22 @@ class SpritesImageTest < Test::Unit::TestCase
     assert_nil test_image.parent
   end
   
-  test 'image type is "global"' do
+  test 'image type is "global" should raise exception' do
-    image = test_image "selectors_ten_by_ten_repeat" => Sass::Script::String.new('global')
+    assert_raise ::Compass::SpriteException do
-    assert_equal 'global', image.repeat
+      image = test_image "selectors_ten_by_ten_repeat" => Sass::Script::String.new('global')
+      image.repeat
+    end
   end
   
   test 'image type is "no-repeat"' do
-    assert_equal 'no-repeat', test_image.repeat
+    img = test_image
+    assert_equal 'no-repeat', img.repeat
+    assert img.no_repeat?
+  end
+
+  test 'image repeat-x' do
+    img = test_image "selectors_ten_by_ten_repeat" => Sass::Script::String.new('repeat-x')
+    assert img.repeat_x?
   end
 
   test 'image position' do

test/units/sprites/layout_test.rb

@@ -50,23 +50,23 @@ class LayoutTest < Test::Unit::TestCase
     sprite_map_test(opts)
   end
 
-  # REPEAT-X
-
   # VERTICAL LAYOUT
 
   it "should have a vertical layout" do
-    assert_equal [0, 10, 20, 30], vertical.images.map(&:top)
+    vert = vertical
-    assert_equal [0, 0, 0, 0], vertical.images.map(&:left)
+    assert_equal [0, 10, 20, 30], vert.images.map(&:top)
+    assert_equal [0, 0, 0, 0], vert.images.map(&:left)
+    assert vert.vertical?
   end
   
   it "should have a vertical layout with spacing" do
-    base = sprite_map_test(@options.merge({"spacing" => Sass::Script::Number.new(10, ['px'])}))
+    vert = sprite_map_test(@options.merge({"spacing" => Sass::Script::Number.new(10, ['px'])}))
-    assert_equal [0, 20, 40, 60], base.images.map(&:top)
+    assert_equal [0, 20, 40, 60], vert.images.map(&:top)
   end
   
   it "should layout vertical with position" do
-    base = sprite_map_test("selectors_ten_by_ten_active_position" => Sass::Script::Number.new(10, ['px']))
+    vert = sprite_map_test("selectors_ten_by_ten_active_position" => Sass::Script::Number.new(10, ['px']))
-    assert_equal [0, 10, 0, 0], base.images.map(&:left)
+    assert_equal [0, 10, 0, 0], vert.images.map(&:left)
   end
 
   it "should generate vertical sprites in decending order" do
@@ -80,6 +80,7 @@ class LayoutTest < Test::Unit::TestCase
   it "should have a smart layout" do
     base = smart
     base.generate
+    assert base.smart?
     assert_equal 400, base.width
     assert_equal 60, base.height
     assert_equal [[0, 0], [20, 120], [20, 0], [20, 100], [20, 160]], base.images.map {|i| [i.top, i.left]}
@@ -92,6 +93,7 @@ class LayoutTest < Test::Unit::TestCase
   it "should generate a diagonal sprite" do
     base = diagonal
     base.generate
+    assert base.diagonal?
     assert_equal 40, base.width
     assert_equal 40, base.height
     assert_equal [[30, 0], [20, 10], [10, 20], [0, 30]], base.images.map {|i| [i.top, i.left]}
@@ -103,6 +105,7 @@ class LayoutTest < Test::Unit::TestCase
   
   it "should have a horizontal layout" do
     base = horizontal
+    assert base.horizontal?
     assert_equal 10, base.height
     assert_equal 40, base.width
   end
@@ -110,7 +113,7 @@ class LayoutTest < Test::Unit::TestCase
   it "should layout images horizontaly" do
     base = horizontal
     assert_equal [0, 10, 20, 30], base.images.map(&:left)
-    assert_equal [0, 0, 0, 0], base.images.map(&:top)
+    assert_equal [0, 0, 0, 0],  base.images.map(&:top)
   end
   
   it "should layout horizontaly with spacing" do

test/units/sprites/sprite_map_test.rb

@@ -123,10 +123,9 @@ class SpriteMapTest < 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(@basegit status, "foo/my.png", {})
+    image = Compass::SassExtensions::Sprites::Image.new(@base, "foo/my.png", {})
     assert_equal File.join(other_folder, 'foo/my.png'), image.file
     assert_equal 0, image.size
-    FileUtils.rm_rf other_folder
   end
   
 end