refactor

Gemfile.lock

@@ -1,13 +1,13 @@
 GIT
   remote: git://github.com/johnbintz/fakefs.git
-  revision: 005ddaaeb2b2881391c31ac9846a55ce5a42c206
+  revision: 7363b6f13bfcd9f583bbf7cd1e0d65c2dc656db7
   specs:
     fakefs (0.3.1)
 
 PATH
   remote: .
   specs:
-    compass (0.11.3.b352e8b)
+    compass (0.11.3.c8cb9f2)
       chunky_png (~> 1.2)
       fssm (>= 0.2.7)
       sass (~> 3.1)
@@ -17,12 +17,12 @@ GEM
   specs:
     ZenTest (4.5.0)
     abstract (1.0.0)
-    actionmailer (3.0.7)
+    actionmailer (3.0.9.rc5)
-      actionpack (= 3.0.7)
+      actionpack (= 3.0.9.rc5)
-      mail (~> 2.2.15)
+      mail (~> 2.2.19)
-    actionpack (3.0.7)
+    actionpack (3.0.9.rc5)
-      activemodel (= 3.0.7)
+      activemodel (= 3.0.9.rc5)
-      activesupport (= 3.0.7)
+      activesupport (= 3.0.9.rc5)
       builder (~> 2.1.2)
       erubis (~> 2.6.6)
       i18n (~> 0.5.0)
@@ -30,21 +30,21 @@ GEM
       rack-mount (~> 0.6.14)
       rack-test (~> 0.5.7)
       tzinfo (~> 0.3.23)
-    activemodel (3.0.7)
+    activemodel (3.0.9.rc5)
-      activesupport (= 3.0.7)
+      activesupport (= 3.0.9.rc5)
       builder (~> 2.1.2)
       i18n (~> 0.5.0)
-    activerecord (3.0.7)
+    activerecord (3.0.9.rc5)
-      activemodel (= 3.0.7)
+      activemodel (= 3.0.9.rc5)
-      activesupport (= 3.0.7)
+      activesupport (= 3.0.9.rc5)
-      arel (~> 2.0.2)
+      arel (~> 2.0.10)
       tzinfo (~> 0.3.23)
-    activeresource (3.0.7)
+    activeresource (3.0.9.rc5)
-      activemodel (= 3.0.7)
+      activemodel (= 3.0.9.rc5)
-      activesupport (= 3.0.7)
+      activesupport (= 3.0.9.rc5)
-    activesupport (3.0.7)
+    activesupport (3.0.9.rc5)
-    addressable (2.2.5)
+    addressable (2.2.6)
-    arel (2.0.9)
+    arel (2.0.10)
     autotest (4.4.6)
       ZenTest (>= 4.4.1)
     autotest-fsevent (0.2.5)
@@ -61,7 +61,7 @@ GEM
       term-ansicolor (~> 1.0.5)
     diff-lcs (1.1.2)
     em-dir-watcher (0.9.4)
-    em-websocket (0.2.1)
+    em-websocket (0.3.0)
       addressable (>= 2.1.1)
       eventmachine (>= 0.12.9)
     erubis (2.6.6)
@@ -71,14 +71,14 @@ GEM
     gherkin (2.2.9)
       json (~> 1.4.6)
       term-ansicolor (~> 1.0.5)
-    haml (3.1.1)
+    haml (3.1.2)
     i18n (0.5.0)
     json (1.4.6)
     livereload (1.6)
       em-dir-watcher (>= 0.1)
       em-websocket (>= 0.1.2)
       ruby-json (>= 1.1.2)
-    mail (2.2.17)
+    mail (2.2.19)
       activesupport (>= 2.3.6)
       i18n (>= 0.4.0)
       mime-types (~> 1.16)
@@ -86,27 +86,29 @@ GEM
     mime-types (1.16)
     mocha (0.9.12)
     polyglot (0.3.1)
-    rack (1.2.2)
+    rack (1.2.3)
     rack-mount (0.6.14)
       rack (>= 1.0.0)
     rack-test (0.5.7)
       rack (>= 1.0)
-    rails (3.0.7)
+    rails (3.0.9.rc5)
-      actionmailer (= 3.0.7)
+      actionmailer (= 3.0.9.rc5)
-      actionpack (= 3.0.7)
+      actionpack (= 3.0.9.rc5)
-      activerecord (= 3.0.7)
+      activerecord (= 3.0.9.rc5)
-      activeresource (= 3.0.7)
+      activeresource (= 3.0.9.rc5)
-      activesupport (= 3.0.7)
+      activesupport (= 3.0.9.rc5)
       bundler (~> 1.0)
-      railties (= 3.0.7)
+      railties (= 3.0.9.rc5)
-    railties (3.0.7)
+    railties (3.0.9.rc5)
-      actionpack (= 3.0.7)
+      actionpack (= 3.0.9.rc5)
-      activesupport (= 3.0.7)
+      activesupport (= 3.0.9.rc5)
       rake (>= 0.8.7)
+      rdoc (~> 3.4)
       thor (~> 0.14.4)
-    rake (0.8.7)
+    rake (0.9.2)
     rb-fsevent (0.4.0)
     rcov (0.9.9)
+    rdoc (3.6.1)
     rspec (2.0.1)
       rspec-core (~> 2.0.1)
       rspec-expectations (~> 2.0.1)
@@ -118,16 +120,16 @@ GEM
       rspec-core (~> 2.0.1)
       rspec-expectations (~> 2.0.1)
     ruby-json (1.1.2)
-    ruby-prof (0.10.5)
+    ruby-prof (0.10.7)
     rubyzip (0.9.4)
-    sass (3.1.1)
+    sass (3.1.2)
     sys-uname (0.8.5)
     term-ansicolor (1.0.5)
     thor (0.14.6)
     timecop (0.3.5)
     treetop (1.4.9)
       polyglot (>= 0.3.1)
-    tzinfo (0.3.26)
+    tzinfo (0.3.28)
 
 PLATFORMS
   ruby

Rakefile

@@ -29,6 +29,16 @@ To run with an alternate version of Rails, make test/rails a symlink to that ver
 To run with an alternate version of Haml & Sass, make test/haml a symlink to that version.
 END
 
+Rake::TestTask.new :units do |t|
+  t.libs << 'lib'
+  t.libs << 'test'
+  test_files = FileList['test/units/**/*_test.rb']
+  test_files.exclude('test/rails/*', 'test/haml/*')
+  t.test_files = test_files
+  t.verbose = true
+end
+
+
 desc "Compile Examples into HTML and CSS"
 task :examples do
   linked_haml = "tests/haml"

lib/compass/commands/sprite.rb

@@ -39,10 +39,11 @@ module Compass
 
       def perform
         relative_uri = options[:uri].gsub(/^#{Compass.configuration.images_dir}\//, '')
-        sprites = Compass::SpriteImporter.new(:uri => relative_uri, :options => Compass.sass_engine_options)
+        name = Compass::SpriteImporter.sprite_name(relative_uri)
-        options[:output_file] ||= File.join(Compass.configuration.sass_path, "sprites", "_#{sprites.name}.#{Compass.configuration.preferred_syntax}")
+        sprites = Compass::SpriteImporter.new
+        options[:output_file] ||= File.join(Compass.configuration.sass_path, "sprites", "_#{name}.#{Compass.configuration.preferred_syntax}")
         options[:skip_overrides] ||= false
-        contents = sprites.content_for_images(options[:skip_overrides])
+        contents = Compass::SpriteImporter.content_for_images(relative_uri, name, options[:skip_overrides])
         if options[:output_file][-4..-1] != "scss"
           contents = Sass::Engine.new(contents, Compass.sass_engine_options.merge(:syntax => :scss)).to_tree.to_sass
         end

lib/compass/sass_extensions/functions/sprites.rb

@@ -22,7 +22,7 @@ module Compass::SassExtensions::Functions::Sprites
   def sprite_map(glob, kwargs = {})
     kwargs.extend VariableReader
     Compass::SassExtensions::Sprites::SpriteMap.from_uri(glob, self, kwargs)
-  end 
+  end
   Sass::Script::Functions.declare :sprite_map, [:glob], :var_kwargs => true
 
   # Returns the image and background position for use in a single shorthand property:

lib/compass/sass_extensions/sprites/sprite_map.rb

@@ -12,11 +12,13 @@ module Compass
         # Initialize a new sprite object from a relative file path
         # the path is relative to the <tt>images_path</tt> confguration option
         def self.from_uri(uri, context, kwargs)
-          importer = ::Compass::SpriteImporter.new(:uri => uri.value, :options => {})
+          uri = uri.value
-          sprites = importer.files.map do |sprite|
+          name, path = Compass::SpriteImporter.path_and_name(uri)
-            sprite.gsub(Compass.configuration.images_path+"/", "")
+          files = Compass::SpriteImporter.files(uri)
+          sprites = files.map do |sprite|
+            sprite.gsub("#{Compass.configuration.images_path}/", "")
           end
-          new(sprites, importer.path, importer.name, context, kwargs)
+          new(sprites, path, name, context, kwargs)
         end
 
         def initialize(sprites, path, name, context, kwargs)

lib/compass/sprite_importer.rb

@@ -1,6 +1,5 @@
 module Compass
   class SpriteImporter < Sass::Importers::Base
-    attr_accessor :uri, :options
     VAILD_FILE_NAME = /\A#{Sass::SCSS::RX::IDENT}\Z/
     SPRITE_IMPORTER_REGEX = %r{((.+/)?([^\*.]+))/(.+?)\.png}
     VALID_EXTENSIONS = ['.png']
@@ -11,29 +10,15 @@ module Compass
       glob = "*-{,s}#{hex*10}{#{VALID_EXTENSIONS.join(",")}}"
       Dir.glob(File.join(path, "**", glob))
     end
-
-    def self.load(uri, options)
-      klass = Compass::SpriteImporter.new
-      klass.uri, klass.options = uri, options
-      klass
-    end
-    
-    def initialize(options ={})
-      @uri, @options = '', {}
-      options.each do |key, value|
-        send("#{key}=", value)
-      end
-    end
     
     def find(uri, options)
-      @uri, @options = uri, options
       if uri =~ SPRITE_IMPORTER_REGEX
-        return sass_engine
+        return self.class.sass_engine(uri, self.class.sprite_name(uri), self, options)
       end
+      nil
     end
     
     def find_relative(uri, base, options)
-      @uri, @options = uri, options
       find(File.join(base, uri), options)
     end
     
@@ -50,45 +35,43 @@ module Compass
     end
     
     def mtime(uri, options)
-      @uri, @options = uri, options
+      self.class.files(uri).sort.inject(Time.at(0)) do |max_time, file|
-      files.sort.inject(Time.at(0)) do |max_time, file|
         (t = File.mtime(file)) > max_time ? t : max_time
       end
     end
     
     def key(uri, options={})
-      @uri, @options = uri, options
+      [self.class.name + ":sprite:" + File.dirname(File.expand_path(uri)), File.basename(uri)]
-      [self.class.name + ":" + File.dirname(File.expand_path(uri)), File.basename(uri)]
     end
     
     def self.path_and_name(uri)
       if uri =~ SPRITE_IMPORTER_REGEX
         [$1, $3]
       else
-        raise Compass::Error "invalid sprite path"
+        raise Compass::Error, "invalid sprite path"
       end
     end
 
     # Name of this spite
-    def name
+    def self.sprite_name(uri)
-      ensure_path_and_name!
+      _, name = path_and_name(uri)
-      @name
+      name
     end
 
     # The on-disk location of this sprite
-    def path
+    def self.path(uri)
-      ensure_path_and_name!
+      path, _ = path_and_name(uri)
-      @path
+      path
     end
     
-    # Returns the Glob of image files for this sprite
+    # Returns the Glob of image files for the uri
-    def files
+    def self.files(uri)
       Dir[File.join(Compass.configuration.images_path, uri)].sort
     end
 
     # Returns an Array of image names without the file extension
-    def sprite_names
+    def self.sprite_names(uri)
-      files.collect do |file|
+      files(uri).collect do |file|
         filename = File.basename(file, '.png')
         unless VAILD_FILE_NAME =~ filename
           raise Compass::Error, "Sprite file names must be legal css identifiers. Please rename #{File.basename(file)}"
@@ -97,31 +80,18 @@ module Compass
       end
     end
     
-    def validate_sprites!
+    # Returns the sass _options for this sprite
-      files.each do |file|
+    def self.sass_options(name, importer, options)
-        unless VALID_EXTENSIONS.include? File.extname(file)
+      options.merge!(:filename => name, :syntax => :scss, :importer => importer)
-          raise Compass::Error, "Invalid sprite extension only: #{VALID_EXTENSIONS.join(',')} images are allowed"
-        end
-      end
-    end
-    
-    # Returns the sass options for this sprite
-    def sass_options
-      options.merge(:filename => name, :syntax => :scss, :importer => self)
     end
     
     # Returns a Sass::Engine for this sprite object
-    def sass_engine
+    def self.sass_engine(uri, name, importer, options)
-      validate_sprites!
+      Sass::Engine.new(content_for_images(uri, name, options[:skip_overrides]), sass_options(name, importer, options))
-      Sass::Engine.new(content_for_images, sass_options)
     end
-    
-    def ensure_path_and_name!
-      @path, @name = self.class.path_and_name(uri)
-    end 
 
     # Generates the Sass for this sprite file
-    def content_for_images(skip_overrides = false)
+    def self.content_for_images(uri, name, skip_overrides = false)
       <<-SCSS
 @import "compass/utilities/sprites/base";
 
@@ -135,7 +105,7 @@ $#{name}-repeat: no-repeat !default;
 $#{name}-prefix: '' !default;
 $#{name}-clean-up: true !default;
 
-#{skip_overrides ? "$#{name}-sprites: sprite-map(\"#{uri}\", $cleanup: $#{name}-clean-up);" : generate_overrides }
+#{skip_overrides ? "$#{name}-sprites: sprite-map(\"#{uri}\", $cleanup: $#{name}-clean-up);" : generate_overrides(uri, name) }
 
 // All sprites should extend this class
 // The #{name}-sprite mixin will do so for you.
@@ -167,7 +137,7 @@ $#{name}-clean-up: true !default;
 
 // Generates a class for each sprited image.
 @mixin all-#{name}-sprites($dimensions: $#{name}-sprite-dimensions, $prefix: sprite-map-name($#{name}-sprites)) {
-  @include #{name}-sprites(#{sprite_names.join(" ")}, $dimensions, $prefix);
+  @include #{name}-sprites(#{sprite_names(uri).join(" ")}, $dimensions, $prefix);
 }
 SCSS
     end
@@ -176,12 +146,13 @@ SCSS
     # <tt>$#{name}-#{sprite_name}-position </tt>
     # <tt> $#{name}-#{sprite_name}-spacing </tt>
     # <tt> #{name}-#{sprite_name}-repeat: </tt>
-    def generate_overrides
+    def self.generate_overrides(uri, name)
+      sprites = sprite_names(uri)
       content = <<-TXT
 // These variables control the generated sprite output
 // You can override them selectively before you import this file.
       TXT
-      sprite_names.map do |sprite_name| 
+      sprites.map do |sprite_name| 
         content += <<-SCSS
 $#{name}-#{sprite_name}-position: $#{name}-position !default;
 $#{name}-#{sprite_name}-spacing: $#{name}-spacing !default;
@@ -190,7 +161,7 @@ $#{name}-#{sprite_name}-repeat: $#{name}-repeat !default;
       end.join
 
       content += "\n$#{name}-sprites: sprite-map(\"#{uri}\", \n$cleanup: $#{name}-clean-up,\n"
-      content += sprite_names.map do |sprite_name| 
+      content += sprites.map do |sprite_name| 
 %Q{  $#{sprite_name}-position: $#{name}-#{sprite_name}-position,
   $#{sprite_name}-spacing: $#{name}-#{sprite_name}-spacing,
   $#{sprite_name}-repeat: $#{name}-#{sprite_name}-repeat}

test/test_helper.rb

@@ -34,4 +34,17 @@ class Test::Unit::TestCase
   include Compass::IoHelper
   extend Compass::TestCaseHelper::ClassMethods
   
-end 
+end 
+
+module SpriteHelper
+  URI = "selectors/*.png"
+  
+  def test_sprite_map(options)
+    importer = Compass::SpriteImporter.new
+    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)
+    Compass::SassExtensions::Sprites::SpriteMap.new(sprite_names.map{|n| "selectors/#{n}.png"}, path, name, sass_engine, options)
+  end
+  
+end

test/units/sprites/image_test.rb

@@ -1,9 +1,9 @@
 require 'test_helper'
 require 'mocha'
 require 'ostruct'
+
 class SpritesImageTest < Test::Unit::TestCase
-
+  include SpriteHelper
-
   def setup
     @images_src_path = File.join(File.dirname(__FILE__), '..', '..', 'fixtures', 'sprites', 'public', 'images')
     file = StringIO.new("images_path = #{@images_src_path.inspect}\n")
@@ -18,12 +18,8 @@ class SpritesImageTest < Test::Unit::TestCase
   let(:sprite_path) { File.join(@images_src_path, sprite_filename) }
   let(:sprite_name) { File.basename(sprite_filename, '.png') }
   
-  def parent
-    importer = Compass::SpriteImporter.new(:uri => "selectors/*.png", :options => options)
-    @parent ||= Compass::SassExtensions::Sprites::SpriteMap.new(importer.sprite_names.map{|n| "selectors/#{n}.png"}, importer.path, importer.name, importer.sass_engine, importer.options)
-  end
   
-  let(:options) do
+  def options
     options = {:offset => @offset}
     options.stubs(:get_var).with(anything).returns(nil)
     ::OpenStruct.any_instance.stubs(:unitless?).returns(true)
@@ -38,7 +34,7 @@ class SpritesImageTest < Test::Unit::TestCase
   let(:digest) { Digest::MD5.file(sprite_path).hexdigest }
 
 
-  let(:image) { Compass::SassExtensions::Sprites::Image.new(parent, File.join(sprite_filename), options)}
+  let(:image) { Compass::SassExtensions::Sprites::Image.new(test_sprite_map(options), File.join(sprite_filename), options)}
 
   test 'initialize' do
     assert_equal sprite_name, image.name

test/units/sprites/importer_test.rb

@@ -7,7 +7,7 @@ class ImporterTest < Test::Unit::TestCase
     @images_src_path = File.join(File.dirname(__FILE__), '..', '..', 'fixtures', 'sprites', 'public', 'images')
     file = StringIO.new("images_path = #{@images_src_path.inspect}\n")
     Compass.add_configuration(file, "sprite_config")
-    @importer = Compass::SpriteImporter.new(:uri => URI, :options => options)
+    @importer = Compass::SpriteImporter.new
   end
 
   def teardown
@@ -18,20 +18,16 @@ class ImporterTest < Test::Unit::TestCase
     {:foo => 'bar'}
   end
   
-  test "load should return an instance of SpriteImporter" do
-    assert Compass::SpriteImporter.load(URI, options).is_a?(Compass::SpriteImporter)
-  end
-  
   test "name should return the sprite name" do
-    assert_equal 'selectors', @importer.name
+    assert_equal 'selectors', Compass::SpriteImporter.sprite_name(URI)
   end
   
   test "path should return the sprite path" do
-    assert_equal 'selectors', @importer.path
+    assert_equal 'selectors',  Compass::SpriteImporter.path(URI)
   end
   
   test "should return all the sprite names" do
-    assert_equal ["ten-by-ten", "ten-by-ten_active", "ten-by-ten_hover", "ten-by-ten_target"], @importer.sprite_names
+    assert_equal ["ten-by-ten", "ten-by-ten_active", "ten-by-ten_hover", "ten-by-ten_target"], Compass::SpriteImporter.sprite_names(URI)
   end
   
   test "should have correct mtime" do
@@ -47,19 +43,21 @@ class ImporterTest < Test::Unit::TestCase
   end
   
   test "sass options should contain options" do
-    assert_equal 'bar', @importer.sass_options[:foo]
+    opts = Compass::SpriteImporter.sass_options(name, @importer, options)
+    assert_equal 'bar', opts[:foo]
   end
   
   test "should fail given bad sprite extensions" do
     @images_src_path = File.join(File.dirname(__FILE__), '..', '..', 'fixtures', 'sprites', 'public', 'images')
     file = StringIO.new("images_path = #{@images_src_path.inspect}\n")
     Compass.add_configuration(file, "sprite_config")
-    importer = Compass::SpriteImporter.new(:uri => 'bad_extensions/*.jpg', :options => options)
+    importer = Compass::SpriteImporter.new
+    uri = "bad_extensions/*.jpg"
     begin
-      importer.sass_engine
+      Compass::SpriteImporter.sass_engine(uri, Compass::SpriteImporter.sprite_name(uri), importer, {})
       assert false, "An invalid sprite file made it past validation."
     rescue Compass::Error => e
-      assert e.message.include?('.png')
+      assert e.message.include?("invalid sprite path")
     end
   end
   

test/units/sprites/sprite_map_test.rb

@@ -1,6 +1,7 @@
 require 'test_helper'
 
 class SpriteMapTest < Test::Unit::TestCase
+  include SpriteHelper
   
   def setup
     Hash.send(:include, Compass::SassExtensions::Functions::Sprites::VariableReader)
@@ -12,16 +13,12 @@ class SpriteMapTest < Test::Unit::TestCase
     Compass.add_configuration(config)
     Compass.configure_sass_plugin!
     @options = {'cleanup' => Sass::Script::Bool.new(true)}
-    setup_map
+    @base = test_sprite_map(@options)
-  end
-  
-  def setup_map
-    @importer = Compass::SpriteImporter.new(:uri => "selectors/*.png", :options => @options)
-    @base = Compass::SassExtensions::Sprites::SpriteMap.new(@importer.sprite_names.map{|n| "selectors/#{n}.png"}, @importer.path, @importer.name, @importer.sass_engine, @importer.options)
   end
 
   def teardown
     FileUtils.rm_r @images_tmp_path
+    @base = nil
   end
   
   it "should have the correct size" do
@@ -29,7 +26,7 @@ class SpriteMapTest < Test::Unit::TestCase
   end
   
   it "should have the sprite names" do
-    assert_equal @importer.sprite_names, @base.sprite_names
+    assert_equal Compass::SpriteImporter.sprite_names(URI), @base.sprite_names
   end
   
   it 'should have image filenames' do
@@ -82,7 +79,7 @@ class SpriteMapTest < Test::Unit::TestCase
     file_to_remove = File.join(@images_tmp_path, 'selectors', 'ten-by-ten.png')
     FileUtils.rm file_to_remove
     assert !File.exists?(file_to_remove), "Failed to remove sprite file"
-    setup_map
+    @base = test_sprite_map(@options)
     @base.generate
     assert !File.exists?(file), "Sprite file did not get removed"
   end