From 325306dcec16b8c8ff1c00b97f08e5c847740826 Mon Sep 17 00:00:00 2001 From: Scott Davis Date: Tue, 10 May 2011 21:04:57 -0400 Subject: [PATCH] Fixed and issue with the options keyword in the Sprite::Base class. it was casing the passed options to be reset to an empty hash. I renamed it to kwargs for consistancy. I also refactored my previous changes to be unobtrusive to people currently using the sprite-map function without an @import. --- .../sass_extensions/functions/sprites.rb | 6 ++--- lib/compass/sass_extensions/sprites/base.rb | 23 +++++++++++-------- .../sass_extensions/sprites/sprite_map.rb | 4 ++-- test/integrations/sprites_test.rb | 4 ++-- test/units/sprites/base_test.rb | 5 ++-- test/units/sprites/image_test.rb | 2 +- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/lib/compass/sass_extensions/functions/sprites.rb b/lib/compass/sass_extensions/functions/sprites.rb index 4c426cf4..1ab53e26 100644 --- a/lib/compass/sass_extensions/functions/sprites.rb +++ b/lib/compass/sass_extensions/functions/sprites.rb @@ -19,11 +19,11 @@ module Compass::SassExtensions::Functions::Sprites # # The sprite map object will generate the sprite map image, if necessary, # the first time it is converted to a url. Simply constructing it has no side-effects. - def sprite_map(glob, cleanup, kwargs = {}) + def sprite_map(glob, kwargs = {}) kwargs.extend VariableReader - Compass::SassExtensions::Sprites::Base.from_uri(glob, self, kwargs, cleanup) + Compass::SassExtensions::Sprites::Base.from_uri(glob, self, kwargs) end - Sass::Script::Functions.declare :sprite_map, [:glob, :cleanup], :var_kwargs => true + Sass::Script::Functions.declare :sprite_map, [:glob], :var_kwargs => true # Returns the image and background position for use in a single shorthand property: # diff --git a/lib/compass/sass_extensions/sprites/base.rb b/lib/compass/sass_extensions/sprites/base.rb index 0a8506f3..4bdea36b 100644 --- a/lib/compass/sass_extensions/sprites/base.rb +++ b/lib/compass/sass_extensions/sprites/base.rb @@ -6,13 +6,12 @@ module Compass # Initialize a new aprite object from a relative file path # the path is relative to the images_path confguration option - def self.from_uri(uri, context, kwargs, cleanup = Sass::Script::Bool.new(true)) + def self.from_uri(uri, context, kwargs) sprite_map = ::Compass::SpriteMap.new(uri.value, {}) - sprites = sprite_map.files.map do |sprite| sprite.gsub(Compass.configuration.images_path+"/", "") end - new(sprites, sprite_map, context, cleanup, kwargs) + new(sprites, sprite_map, context, kwargs) end # Loads the sprite engine @@ -24,18 +23,22 @@ module Compass # We should do so only when the packing algorithm changes SPRITE_VERSION = "1" - attr_accessor :image_names, :path, :name, :options, :map, :cleanup + attr_accessor :image_names, :path, :name, :map, :kwargs attr_accessor :images, :width, :height - def initialize(image_names, map, context, cleanup, options) + def initialize(sprites, sprite_map, context, kwargs) require_engine! - @image_names, @path, @name, @options, @cleanup = image_names, map.path, map.name, options, cleanup + @image_names = sprites + @path = sprite_map.path + @name = sprite_map.name + @kwargs = kwargs + @kwargs['cleanup'] ||= Sass::Script::Bool.new(true) @images = nil @width = nil @height = nil @evaluation_context = context - @map = map + @map = sprite_map validate! compute_image_metadata! end @@ -58,7 +61,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, options) + image = Compass::SassExtensions::Sprites::Image.new(self, relative_file, kwargs) @width = [ @width, image.width + image.offset ].max image end @@ -118,7 +121,7 @@ module Compass # Generate a sprite image if necessary def generate if generation_required? - if @cleanup.value + if kwargs.get_var('cleanup').value cleanup_old_sprites end sprite_data = construct_sprite @@ -183,7 +186,7 @@ module Compass to_s end - def to_s(options = self.options) + def to_s(kwargs = self.kwargs) sprite_url(self).value end diff --git a/lib/compass/sass_extensions/sprites/sprite_map.rb b/lib/compass/sass_extensions/sprites/sprite_map.rb index fd971f96..7e19fdf5 100644 --- a/lib/compass/sass_extensions/sprites/sprite_map.rb +++ b/lib/compass/sass_extensions/sprites/sprite_map.rb @@ -76,7 +76,7 @@ $#{name}-repeat: no-repeat !default; $#{name}-prefix: '' !default; $#{name}-clean-up: true !default; -#{skip_overrides ? "$#{name}-sprites: sprite-map(\"#{uri}\", $#{name}-clean-up);" : generate_overrides } +#{skip_overrides ? "$#{name}-sprites: sprite-map(\"#{uri}\", $cleanup: $#{name}-clean-up);" : generate_overrides } // All sprites should extend this class // The #{name}-sprite mixin will do so for you. @@ -130,7 +130,7 @@ $#{name}-#{sprite_name}-repeat: $#{name}-repeat !default; SCSS end.join - content += "\n$#{name}-sprites: sprite-map(\"#{uri}\", $#{name}-clean-up,\n" + content += "\n$#{name}-sprites: sprite-map(\"#{uri}\",\n$cleanup: $#{name}-clean-up,\n" content += sprite_names.map do |sprite_name| %Q{ $#{sprite_name}-position: $#{name}-#{sprite_name}-position, $#{sprite_name}-spacing: $#{name}-#{sprite_name}-spacing, diff --git a/test/integrations/sprites_test.rb b/test/integrations/sprites_test.rb index 74f122f5..549427d4 100644 --- a/test/integrations/sprites_test.rb +++ b/test/integrations/sprites_test.rb @@ -259,7 +259,7 @@ class SpritesTest < Test::Unit::TestCase it "should use position adjustments in functions" do css = render <<-SCSS - $squares: sprite-map("squares/*.png", true, $position: 100%); + $squares: sprite-map("squares/*.png", $position: 100%); .squares-sprite { background: $squares no-repeat; } @@ -411,7 +411,7 @@ class SpritesTest < Test::Unit::TestCase it "should work even if @import is missing" do css = render <<-SCSS .squares { - background: sprite(sprite-map("squares/*.png", true), twenty-by-twenty) no-repeat; + background: sprite(sprite-map("squares/*.png"), twenty-by-twenty) no-repeat; } SCSS assert_correct css, <<-CSS diff --git a/test/units/sprites/base_test.rb b/test/units/sprites/base_test.rb index d6c2d322..e79a11c3 100644 --- a/test/units/sprites/base_test.rb +++ b/test/units/sprites/base_test.rb @@ -11,13 +11,13 @@ class SpritesBaseTest < Test::Unit::TestCase config.images_path = @images_tmp_path Compass.add_configuration(config) Compass.configure_sass_plugin! - @options = {} + @options = {'cleanup' => Sass::Script::Bool.new(true)} setup_map end def setup_map @map = Compass::SpriteMap.new("selectors/*.png", @options) - @base = Compass::SassExtensions::Sprites::Base.new(@map.sprite_names.map{|n| "selectors/#{n}.png"}, @map, @map.sass_engine, Sass::Script::Bool.new(true), @map.options) + @base = Compass::SassExtensions::Sprites::Base.new(@map.sprite_names.map{|n| "selectors/#{n}.png"}, @map, @map.sass_engine, @map.options) end def teardown @@ -82,7 +82,6 @@ class SpritesBaseTest < 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" - @options["cleanup"] = true setup_map @base.generate assert !File.exists?(file), "Sprite file did not get removed" diff --git a/test/units/sprites/image_test.rb b/test/units/sprites/image_test.rb index 6a7d5a2e..01609932 100644 --- a/test/units/sprites/image_test.rb +++ b/test/units/sprites/image_test.rb @@ -20,7 +20,7 @@ class SpritesImageTest < Test::Unit::TestCase def parent map = Compass::SpriteMap.new("selectors/*.png", options) - @parent ||= Compass::SassExtensions::Sprites::Base.new(map.sprite_names.map{|n| "selectors/#{n}.png"}, map, map.sass_engine, Sass::Script::Bool.new(true), map.options) + @parent ||= Compass::SassExtensions::Sprites::Base.new(map.sprite_names.map{|n| "selectors/#{n}.png"}, map, map.sass_engine, map.options) end let(:options) do