-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Random function with Fixes #1062
Changes from 4 commits
5df141c
8d66e64
c7d5bf2
3469710
8f406d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -401,6 +401,24 @@ def self.signature(method_name, arg_arity, kwarg_arity) | |
@signatures[method_name].first | ||
end | ||
|
||
# Sets the random seed used by Sass's internal random number generator. | ||
# | ||
# This can be used to ensure consistent random number sequences which | ||
# allows for consistent results when testing, etc. | ||
# | ||
# @param seed [Integer] | ||
# @return [Integer] The same seed. | ||
def self.random_seed=(seed) | ||
@random_number_generator = Random.new(seed) | ||
end | ||
|
||
# Get Sass's internal random number generator. | ||
# | ||
# @return [Random] | ||
def self.random_number_generator | ||
@random_number_generator ||= Random.new | ||
end | ||
|
||
# The context in which methods in {Script::Functions} are evaluated. | ||
# That means that all instance methods of {EvaluationContext} | ||
# are available to use in functions. | ||
|
@@ -2018,9 +2036,9 @@ def if(condition, if_true, if_false) | |
# @overload unique_id() | ||
# @return [Sass::Script::Value::String] | ||
def unique_id | ||
Thread.current[:sass_last_unique_id] ||= rand(36**8) | ||
Thread.current[:sass_last_unique_id] ||= generator.rand(36**8) | ||
# avoid the temptation of trying to guess the next unique value. | ||
value = (Thread.current[:sass_last_unique_id] += (rand(10) + 1)) | ||
value = (Thread.current[:sass_last_unique_id] += (generator.rand(10) + 1)) | ||
# the u makes this a legal identifier if it would otherwise start with a number. | ||
identifier("u" + value.to_s(36).rjust(8, '0')) | ||
end | ||
|
@@ -2157,7 +2175,6 @@ def mixin_exists(name) | |
end | ||
declare :mixin_exists, [:name] | ||
|
||
|
||
# Return a string containing the value as its Sass representation. | ||
# | ||
# @param value [Sass::Script::Value::Base] The value to inspect. | ||
|
@@ -2168,6 +2185,28 @@ def inspect(value) | |
end | ||
declare :inspect, [:value] | ||
|
||
# @overload random() | ||
# Return a decimal between 0 and 1, inclusive of 0, but not 1. | ||
# @return [Sass::Script::Number] A decimal value. | ||
# @overload random($limit) | ||
# Return an integer between 1 and $limit inclusive. | ||
# @param $limit The maximum of the random integer to be returned, a positive integer. | ||
# @return [Sass::Script::Number] An integer. | ||
# @raise [ArgumentError] if the limit is not 1 or greater | ||
def random(limit = nil) | ||
if limit | ||
assert_integer limit, "limit" | ||
if limit.value < 1 | ||
raise ArgumentError.new("Expected the limit to be 1 or greater") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency, this message should be |
||
end | ||
number(1 + generator.rand(limit.value)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right that this is inclusive of |
||
else | ||
number(generator.rand) | ||
end | ||
end | ||
declare :random, [] | ||
declare :random, [:limit] | ||
|
||
private | ||
|
||
# This method implements the pattern of transforming a numeric value into | ||
|
@@ -2205,5 +2244,12 @@ def to_h(obj) | |
WARNING | ||
obj.to_h | ||
end | ||
|
||
# Convenient access to Sass's internal random number generator. | ||
# | ||
# @return [Random] | ||
def generator | ||
Sass::Script::Functions.random_number_generator | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
# In Ruby 1.9+, there is a built-in Random class, which is globally defined. | ||
# In case we are running on Ruby 1.8.7 (detected, by having no Random class defined), | ||
# this method defines a mock Random class. | ||
unless defined?(::Random) | ||
# Shim for ruby 1.8.7 support. | ||
class Random | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Name this CrossPlatformRandom or something and put it within the Sass::Util namespace. We don't want to cause conflicts if someone else defines a top-level Random class on 1.8. |
||
def initialize(seed = nil) | ||
srand(seed) if seed | ||
end | ||
|
||
def rand(*args) | ||
Kernel.rand(*args) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1574,6 +1574,55 @@ def test_inspect | |
assert_equal "(a: 1, b: 2)", evaluate("inspect((a: 1, b: 2))") | ||
end | ||
|
||
def test_random | ||
Sass::Script::Functions.random_seed = 1 | ||
assert_equal "0.41702", evaluate("random()") | ||
assert_equal "13", evaluate("random(100)") | ||
end | ||
|
||
def test_random_works_without_a_seed | ||
if Sass::Script::Functions.instance_variable_defined?("@random_number_generator") | ||
Sass::Script::Functions.send(:remove_instance_variable, "@random_number_generator") | ||
end | ||
assert_nothing_raised do | ||
result = perform("random()") | ||
assert_kind_of Sass::Script::Number, result | ||
assert result.value >= 0, "Random number was below 0" | ||
assert result.value <= 1, "Random number was above 1" | ||
end | ||
end | ||
|
||
def test_random_works_with_limit | ||
if Sass::Script::Functions.instance_variable_defined?("@random_number_generator") | ||
Sass::Script::Functions.send(:remove_instance_variable, "@random_number_generator") | ||
end | ||
assert_nothing_raised do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unnecessary, since |
||
# Passing 1 as the limit should always return 1, since limit calls return | ||
# integers from 1 to the argument, so when the argument is 1, its a predicatble | ||
# outcome | ||
assert "1", evaluate("random(1)") | ||
end | ||
|
||
# Limit must be 1 or greater | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer these be separate tests with descriptive names. |
||
assert_raises Sass::SyntaxError do | ||
evaluate("random(0)") | ||
end | ||
|
||
# Limit cannot be negative | ||
assert_raises Sass::SyntaxError do | ||
evaluate("random(-1)") | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should also be a test for a non-integer limit. |
||
end | ||
|
||
# This could *possibly* fail, but exceedingly unlikely | ||
def test_random_is_semi_unique | ||
if Sass::Script::Functions.instance_variable_defined?("@random_number_generator") | ||
Sass::Script::Functions.send(:remove_instance_variable, "@random_number_generator") | ||
end | ||
assert_nothing_raised do | ||
assert_not_equal evaluate("random()"), evaluate("random()") | ||
end | ||
end | ||
|
||
## Regression Tests | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is exclusive of
$limit
as well.Also,
$limit
should be code-quoted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damnit. I misread it. Here is the documentation in Ruby. Good catch, Nathan. Sorry about that. I feel like some other documentation had confused me. Changes coming. http://www.ruby-doc.org/core-2.1.0/Random.html#method-c-rand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait. Sorry. So, the way that @chriseppstein implemented this, its actually "Random.rand($limit) + 1" So, while the ruby implementation is not inclusive, by adding one, we are making ours inclusive of the limit.