Permalink
Browse files

fix circular require warnigs

  • Loading branch information...
1 parent 8badbab commit ff0d98a0536687b7d2d6f918a198ec81e1494077 @tenderlove committed Mar 21, 2013
Showing with 4 additions and 6 deletions.
  1. +0 −5 lib/sass/script/literal.rb
  2. +3 −0 lib/sass/script/operation.rb
  3. +1 −1 lib/sass/version.rb
@@ -5,11 +5,6 @@ module Sass::Script
# are designed to be overridden by subclasses which may change the semantics somewhat.
# The operations listed here are just the defaults.
class Literal < Node
- require 'sass/script/string'
- require 'sass/script/number'
- require 'sass/script/color'
- require 'sass/script/bool'
- require 'sass/script/null'
@nex3
nex3 Mar 22, 2013

I'd like to have the way these subclasses are required be consistent across all of them. I'd rather see them all required here as they are now, and the back-requires in the individual files be removed.

@tenderlove
tenderlove Mar 22, 2013 owner

I can change the requires to the other direction, but I think that files should require what they need. The String class needs the Literal class, so it should require the Literal class, not the other way around.

@nex3
nex3 Mar 22, 2013

I like that philosophy in general, but it doesn't really work out if circular requires aren't allowed. Literal needs Bool, and Bool needs Literal. A semantic use of require there would be circular.

Given that a fully semantic model doesn't work, the "require from superclass" model at least guarantees that the superclass will be defined when the subclass is required.

@tenderlove
tenderlove Mar 22, 2013 owner

Ok. I've changed it in d2ee3a6

@ged
ged Mar 28, 2013

A pattern that retains semantic properties but avoids circular requires:

require 'sass/script/literal' unless defined? Sass::Script::Literal
require 'sass/script/list'
require 'sass/script/arg_list'
@@ -1,7 +1,10 @@
require 'set'
+require 'sass/script/literal'
require 'sass/script/string'
require 'sass/script/number'
require 'sass/script/color'
+require 'sass/script/bool'
+require 'sass/script/null'
require 'sass/script/functions'
require 'sass/script/unary_operation'
require 'sass/script/interpolation'
View
@@ -2,7 +2,7 @@
# This is necessary for loading Sass when Haml is required in Rails 3.
# Once the split is complete, we can remove it.
-require File.dirname(__FILE__) + '/../sass'
+require File.dirname(__FILE__) + '/../sass' unless defined?(SASS_BEGUN_TO_LOAD)
@nex3
nex3 Mar 22, 2013

As per the comment, this can be removed entirely now.

@tenderlove
tenderlove Mar 22, 2013 owner

Sounds good, I'll rm this code! :D

require 'sass/util'
module Sass

0 comments on commit ff0d98a

Please sign in to comment.