0
@@ -7,7 +7,6 @@ module Sass
0
- :template_location => './public/stylesheets/sass',
0
:css_location => './public/stylesheets',
0
:always_update => false,
0
@@ -34,52 +33,92 @@ module Sass
0
@@options.merge!(value)
0
- # Checks each stylesheet in <tt>options[:css_location]</tt>
0
- # to see if it needs updating,
0
- # and updates it using the corresponding template
0
- # from <tt>options[:templates]</tt>
0
+ # Checks each css stylesheet to see if it needs updating,
0
+ # and updates it using the corresponding sass template if it does.
0
return if options[:never_update]
0
@@checked_for_updates = true
0
- Dir.glob(File.join(options[:template_location], "**", "*.sass")).entries.each do |file|
0
- # Get the relative path to the file with no extension
0
- name = file.sub(options[:template_location] + "/", "")[0...-5]
0
- if !forbid_update?(name) && (options[:always_update] || stylesheet_needs_update?(name))
0
- css = css_filename(name)
0
- File.delete(css) if File.exists?(css)
0
- filename = template_filename(name)
0
- l_options = @@options.dup
0
- l_options[:filename] = filename
- l_options[:load_paths] = load_paths
0
- engine = Engine.new(File.read(filename), l_options)
0
- # Create any directories that might be necessary
0
- dirs = [l_options[:css_location]]
0
- name.split("/")[0...-1].each { |dir| dirs << "#{dirs[-1]}/#{dir}" }
0
- dirs.each { |dir| Dir.mkdir(dir) unless File.exist?(dir) }
0
- # Finally, write the file
0
- File.open(css, 'w') do |file|
0
+ template_location_map.each do |template_location, css_location|
0
+ Dir.glob(File.join(template_location, "**", "*.sass")).entries.each do |file|
0
+ # Get the relative path to the file with no extension
0
+ name = file.sub(template_location + "/", "")[0...-5]
0
+ if !forbid_update?(name) && (options[:always_update] || stylesheet_needs_update?(name, template_location, css_location))
0
+ update_stylesheet(name, template_location, css_location)
private
0
+ def update_stylesheet(name, template_location, css_location)
0
+ css = css_filename(name, css_location)
0
+ File.delete(css) if File.exists?(css)
0
+ filename = template_filename(name, template_location)
0
+ l_options = @@options.dup
0
+ l_options[:filename] = filename
0
+ l_options[:load_paths] = load_paths
0
+ engine = Engine.new(File.read(filename), l_options)
0
+ # Create any directories that might be necessary
0
+ mkpath(css_location,name)
0
+ # Finally, write the file
0
+ File.open(css, 'w') do |file|
+ end
0
+ # Create any successive directories required to be able to write a file to: File.join(base,name)
0
+ name.split(File::SEPARATOR)[0...-1].each { |dir| dirs << File.join(dirs[-1],dir) }
0
+ dirs.each { |dir| Dir.mkdir(dir) unless File.exist?(dir) }
- (options[:load_paths] || []) + [options[:template_location]]
0
+ (options[:load_paths] || []) + template_locations
0
+ def template_locations
0
+ case location = (options[:template_location] || default_template_location)
0
+ #in case we get passed a list of tuples.
0
+ location.map{ |t| t.first }
+ end
0
+ case options[:template_location]
0
+ template_locations.map{ |m|options[:template_location][m] }
0
+ #in case we get passed a list of tuples.
0
+ options[:template_location].map{ |t| t.last }
+ Array(options[:css_location] || default_css_location)
0
+ # map of template locations to css locations
+ def template_location_map
0
+ template_locations.zip(css_locations).inject([]) do |memo, pair|
0
+ memo << [pair.first || memo.first.first, pair.last || memo.first.last]
def exception_string(e)
0
@@ -96,10 +135,14 @@ module Sass
0
min = [e.sass_line - 5, 0].max
0
- File.read(e.sass_filename).rstrip.split("\n")[
0
- min .. e.sass_line + 5
0
- ].each_with_index do |line, i|
0
- e_string << "#{min + i + 1}: #{line}\n"
0
+ File.read(e.sass_filename).rstrip.split("\n")[
0
+ min .. e.sass_line + 5
0
+ ].each_with_index do |line, i|
0
+ e_string << "#{min + i + 1}: #{line}\n"
+ rescue
0
+ e_string << "Couldn't read sass file: #{e.sass_filename}"
0
@@ -121,25 +164,39 @@ END
0
- def template_filename(name)
0
- "#{options[:template_location]}/#{name}.sass"
0
+ def default_css_location
0
+ Array(options[:css_location]).first
0
+ def default_template_location
0
+ if options[:template_location]
0
+ Array(options[:template_location]).first
+ File.join(default_css_location,'sass')
0
+ def template_filename(name, path = default_template_location)
0
+ "#{path}/#{name}.sass"
0
- def css_filename(name)
0
- "#{options[:css_location]}/#{name}.css"
0
+ def css_filename(name, path = default_css_location)
0
def forbid_update?(name)
0
name.sub(/^.*\//, '')[0] == ?_
0
- def stylesheet_needs_update?(name)
0
- if !File.exists?(css_filename(name))
0
+ def stylesheet_needs_update?(name, template_path = default_template_location, css_path = default_css_location)
0
+ css_file = css_filename(name, css_path)
0
+ template_file = template_filename(name, template_path)
0
+ if !File.exists?(css_file)
0
- css_mtime = File.mtime(css_filename(name))
0
- File.mtime(template_filename(name)) > css_mtime ||
0
- dependencies(template_filename(name)).any?(&dependency_updated?(css_mtime))
0
+ css_mtime = File.mtime(css_file)
+ File.mtime(template_file) > css_mtime ||
0
+ dependencies(template_file).any?(&dependency_updated?(css_mtime))
Comments
This still says colon-delimited…
I don’t think there should be this many ways to set different paths. Let’s just leave it at setting strings for both or a hash for
:template_location.Isn’t
.entrieshere redundant?Another stylistic thing: no extra lines between two ends.
Style: space after comma
What about
if location.is_a?(String) [location] else location.to_a.map { |l| l.first } endThat should handle arrays, hashes, and be more duck-typey anyway.
Again, it’d probably be easier to just call to_a here and treat it like an array of tuples.
I thought we were dropping support for pairwise arrays.
If splitting on ”/” breaks on windows, that should be fixed in a separate commit that I can pull separately into stable.
This is also a logically separate change, and should have its own commit.
This doesn’t make sense: if
options[:css_location]is nil,default_css_locationwill be nil, too.When does this or
css_filenameever get called with only one argument? I don’t think we need those default_ methods at all.Sorry, meant that comment for
def template_filenameAgain, I don’t think there’s any reason for this to have optional args.
If we’re expecting people to use hashes, we should test this with a hash.
Style: Space after comma
“overrides”
No we were dropping support for colon delimited paths. Pairwise arrays are really handy and are just another representation of a map.
Good point.
I like that.
I like that better.
ok.
fixed.
fixed.
oops.
This is one of the examples where using pairwise arrays is more natural than a hash. I agree so I’ll add a test case that asserts that hashes are not treated differently than pairwise arrays.
Yes, so I just learned.
I’m going to unsquash this change set and rewrite it again, taking care to avoid combining logically separate changes.
Agreed, it was useful for backwards compatibility with the test cases, but I updated them to pass the locations explicitly
Fixed, I also cleaned up this line as it was too long to be readable.
i’m a CS major not an engrish major!
Well change that to I have a CS degree not a an engrish degree. I’ve not been in college for a while now!
Thanks for taking time to do such a thorough review. I’ve addressed all the issues you’ve raised except this one. I feel that making the change would be a mistake and I’ll follow up with you in a separate email.