Skip to content

Allowed extensions #1312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc-src/content/help/tutorials/extending.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ The sprite engine is the work horse of sprite generation it's the interface for

### Requirements

A sprite engine requires a constant be defined called `VALID_EXTENSIONS` that returns and array of valid image extensions ex. `['.png', '.jpg']`

A sprite engine requires two methods `construct_sprite`, and `save(filename)`

Once inside the class you have access to `images` which is a collection of [Compass::SassExtensions::Sprites::Image](http://rdoc.info/github/chriseppstein/compass/dda7c9/Compass/SassExtensions/Sprites/Image)
Expand Down
4 changes: 3 additions & 1 deletion lib/compass/sass_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
raise LoadError, "It looks like you've got an incompatible version of Sass. This often happens when you have an old haml gem installed. Please upgrade Haml to v3.1 or above."
end

module Compass::SassExtensions
module Compass
module SassExtensions
end
end

require 'compass/sass_extensions/functions'
Expand Down
5 changes: 2 additions & 3 deletions lib/compass/sass_extensions/sprites.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
require 'digest/md5'
require 'compass/sprite_importer'

module Compass
module SassExtensions
module Sprites
end
end
end

require 'digest/md5'
require 'compass/sass_extensions/sprites/images'
require 'compass/sass_extensions/sprites/layout'
require 'compass/sass_extensions/sprites/image_row'
Expand All @@ -18,4 +16,5 @@ module Sprites
require 'compass/sass_extensions/sprites/image_methods'
require 'compass/sass_extensions/sprites/sprite_map'
require 'compass/sass_extensions/sprites/engines'
require 'compass/sprite_importer'

6 changes: 3 additions & 3 deletions lib/compass/sass_extensions/sprites/engines.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@

module Compass
module SassExtensions
module Sprites
class Engine
attr_accessor :width, :height, :images, :canvas
def initialize(width, height, images)
def initialize(width=nil, height=nil, images=nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this change for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea was 4 months ago :(

@width, @height, @images = width, height, images
@canvas = nil
end
Expand All @@ -21,5 +22,4 @@ def save(filename)
end
end


require 'compass/sass_extensions/sprites/engines/chunky_png_engine'
require 'compass/sass_extensions/sprites/engines/chunky_png_engine'
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ module SassExtensions
module Sprites
class ChunkyPngEngine < Compass::SassExtensions::Sprites::Engine

VALID_EXTENSIONS = ['.png']

def construct_sprite
@canvas = ChunkyPNG::Image.new(width, height, ChunkyPNG::Color::TRANSPARENT)
images.each do |image|
Expand Down
19 changes: 18 additions & 1 deletion lib/compass/sass_extensions/sprites/sprite_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ def self.relative_name(sprite)
end
end

def self.sprite_engine_class
constantize("Compass::SassExtensions::Sprites::#{self.modulize}Engine")
end

def initialize(sprites, path, name, context, kwargs)
@image_names = sprites
@path = path
Expand Down Expand Up @@ -81,10 +85,23 @@ def method_missing(meth, *args, &block)

private

def modulize
def self.modulize
@modulize ||= Compass::configuration.sprite_engine.to_s.scan(/([^_.]+)/).flatten.map {|chunk| "#{chunk[0].chr.upcase}#{chunk[1..-1]}" }.join
end

def self.constantize(camel_cased_word)
names = camel_cased_word.split('::')
names.shift if names.empty? || names.first.empty?

constant = Object
names.each do |name|
constant = constant.const_defined?(name) ? constant.const_get(name) : constant.const_missing(name)
end
constant
end



Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neeeeeewlineeeeees.

end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/compass/sass_extensions/sprites/sprite_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def compute_image_metadata!
end

def init_engine
@engine = eval("::Compass::SassExtensions::Sprites::#{modulize}Engine.new(nil, nil, nil)")
@engine = self.class.sprite_engine_class.new
@engine.width = @width
@engine.height = @height
@engine.images = @images
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just pass these arguments to the initializer instead of making them optional?

Expand Down
22 changes: 18 additions & 4 deletions lib/compass/sprite_importer.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
require 'erb'
require 'compass'
require 'compass/sprite_importer/binding'




Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neeeeeewlineeeeees!

module Compass
class SpriteImporter < Sass::Importers::Base
VAILD_FILE_NAME = /\A#{Sass::SCSS::RX::IDENT}\Z/
SPRITE_IMPORTER_REGEX = %r{((.+/)?([^\*.]+))/(.+?)\.png}
VALID_EXTENSIONS = ['.png']
SPRITE_IMPORTER_REGEX = %r{((.+/)?([^\*.]+))/(.+?)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with relaxing this glob restriction, but unless the user specifies a valid extension in the glob explicitly, I think we should return nil from find unless we find an image with a valid extension in the folder. This will keep it from conflicting with globs that import sass.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the entire point of this was to move the required extensions down a level to the engine classes so things like svg gif or whatever people wanted could be created and also allow me to move forward using the same code to work on a svg -> font glyph engine

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. My point is that when the glob matches, we always return a sass engine. Instead, the sprite engine should be able to look at that glob and decide if it has files in it that it can process. If not, this importer should return nil and then a different importer can handle it. Or maybe Sass will just give an error that says that it cannot import that.


TEMPLATE_FOLDER = File.join(File.expand_path('../', __FILE__), 'sprite_importer')
CONTENT_TEMPLATE_FILE = File.join(TEMPLATE_FOLDER, 'content.erb')
Expand All @@ -15,7 +19,7 @@ class SpriteImporter < Sass::Importers::Base
# finds all sprite files
def self.find_all_sprite_map_files(path)
hex = "[0-9a-f]"
glob = "*-s#{hex*10}{#{VALID_EXTENSIONS.join(",")}}"
glob = "*-s#{hex*10}{#{valid_extensions.join(",")}}"
Sass::Util.glob(File.join(path, "**", glob))
end

Expand Down Expand Up @@ -53,7 +57,7 @@ def key(uri, options={})
end

def self.path_and_name(uri)
if uri =~ SPRITE_IMPORTER_REGEX
if uri =~ sprite_importer_regex_with_ext
[$1, $3]
else
raise Compass::Error, "invalid sprite path"
Expand Down Expand Up @@ -107,6 +111,16 @@ def self.content_for_images(uri, name, skip_overrides = false)
binder = Compass::Sprites::Binding.new(:name => name, :uri => uri, :skip_overrides => skip_overrides, :sprite_names => sprite_names(uri), :files => files(uri))
CONTENT_TEMPLATE.result(binder.get_binding)
end

private

def self.valid_extensions
@valid_extensions ||= SassExtensions::Sprites::SpriteMap.sprite_engine_class::VALID_EXTENSIONS
end

def self.sprite_importer_regex_with_ext
@importer_regex ||= %r{#{SPRITE_IMPORTER_REGEX}(#{valid_extensions.join('|')})}
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here is where the glob is scoped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I'd still like to not return any engine if there's no files that match the glob.

end
end