Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

EvaluationContext takes the environment as argument and then extracts options from it #310

Closed
wants to merge 3 commits into from

6 participants

@jede

Hi there!

I encountered a similiar example to the one shown here, and realized that there really wasn't an easy way to do this in ruby as a SASS extension. So I thought it could be a good idea to make the environment accessible to the EvaluationContext.

$variable: 5;

@function more() {
  @return $variable + 3;
}

With my addition this would now be possible:

def more
  environment.var('variable')
end
@nex3
Owner

What's the use case for this? It seems like it would make it difficult for users to tell where variables are being used.

@jede

In my case I want to be able to set a config variable which is then used in functions. This works well today using SCSS.

Don't you already have that issue with mixins, that it is hard to know when a variable is used?

@chriseppstein

+1 this brings ruby-based functions to parity with sass-based functions.

@chriseppstein

@nex3 In the past you have rejected this functionality claiming that it makes optimization harder. Do you still feel this way? This functionality would enable the dynamic construction of variable names -- not a feature that exists in sass functions.

@chriseppstein

Another thing to note is that having the environment accessible technically allows the creation of variables too.

@jede

Which type of optimization do you mean would be harder?

Imo dynamically created variables would be quite useless since you couldn't access them in the templates (you don't know their names). That would however allow them to be accessed by other ruby implemented functions, but that is already possible since you could, theoretically, create a singleton class to hold all of your custom global variables.

@chriseppstein

A use of dynamically constructed variables would be like what we do for compass sprites: according to a naming convention of the names of folders/files on disk.

@nex3
Owner

The optimizations I was concerned about in the past are looking increasingly unlikely, so I'm not particularly opposed to this feature anymore.

@christophehurpeau

I'd like that too, please !

@chriseppstein

@nex3 can we get this in 3.2?

@nex3
Owner

I'm still skeptical of allowing Sass functions access to random variables that happen to exist in the scope where the function is called. I'd feel better about this if it only allowed access to the top-level environment that contains global definitions.

@chriseppstein

talked to @nex3 about this, and I agree that ruby functions shouldn't be able to do things that sass functions can't. This would allow access to local variables in the calling context and also setting variables in the local scope.

@Snugug

I'm in need of this too, but I'd like environment to be able to access parent (the full selector string in which it is being called from, or nil if it's the global environment). So, I could have something like the following:

$test: 4px;

.bar {
  content: '#{parent()}';

  .qux {
    content: '#{parent()}';
  }
}
.baz {
  width: width();
}
def parent
  Sass::Script::String.new(environment.parent)
end
def width
  Sass::Script::Number.new(environment.var('test')
end
.bar {
  content: '.bar';
}

.bar .qux {
  content: '.bar .qux';
}

.baz {
  width: 4px;
}
@chriseppstein

@Snugug this use case is covered by #286.

@Snugug

Kinda sorta. While the use case is the same, the methodologies are slightly different. For the usecase and outcome there, he's looking to be able to use the parent selector even if one doesn't exist. In this one, I'm trying to grab the parent selector through the SassScript enviro as a string/list. His is entirely through front-facing Sass, my usecase is backend Ruby, both with the same end result. Well similar results but with much different ways of getting there which produce different usecases and different potential code.

@chriseppstein

My goal with this patch is that Ruby functions can do all the things that Sass functions can do. Once #286 lands, that would imply that ruby would have a function it could call that would be equivalent to & in sass script.

@Snugug

Ahh, all makes sense now.

@chriseppstein

@jede any interest in taking another whack at this?

@jede

@chriseppstein sure! So the problem is, if I understand @nex3 correctly, that ruby methods should not be able to access everything? It would be really good with an example of this (i.e. something this would allow that you cannot do with sass).

@nex3
Owner

Here's an example:

def evil(varname, value)
  environment.set_var(varname.value, value)
end
.foo {
  -no-op: evil('varname', 123px);
  width: $varname;
}

The problem is that this would allow functions access to all levels of the environment, including local variables, which should not be accessible by functions, whether Ruby or Sass.

@jede
@travisbot

This pull request passes (merged 7595c25 into 1bbdfb8).

@jede

This should work now. I added tests to cover the case @nex3 provided, please have a look and see what you think! Don't know why @travisbot hasn't commented, but the tests pass: http://travis-ci.org/#!/nex3/sass/builds/2377423

@nex3
Owner

This does address the issue I brought up, but it's still not completely correct. The code is still able to read variables in the local scope, even if it can't write them.

I think the correct way to handle this is to keep track of the global-scope Environment object and pass that to the functions.

@Snugug Snugug referenced this pull request in at-import/Singularity
Closed

Rewrite in Ruby #65

@chriseppstein

I took care of the last remaining point here (576aec6) and merged this to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
3  lib/sass/script/funcall.rb
@@ -103,7 +103,8 @@ def _perform(environment)
unless Functions.callable?(ruby_name)
opts(to_literal(args))
else
- opts(Functions::EvaluationContext.new(environment.options).send(ruby_name, *args))
+ local_environment = Environment.new(environment)
+ opts(Functions::EvaluationContext.new(local_environment).send(ruby_name, *args))
end
rescue ArgumentError => e
# If this is a legitimate Ruby-raised argument error, re-raise it.
View
13 lib/sass/script/functions.rb
@@ -309,14 +309,21 @@ def self.signature(method_name, arg_arity, kwarg_arity)
class EvaluationContext
include Functions
+
+ # The environment of the {Sass::Engine}
+ #
+ # @return [Environment]
+ attr_reader :environment
+
# The options hash for the {Sass::Engine} that is processing the function call
#
# @return [{Symbol => Object}]
attr_reader :options
- # @param options [{Symbol => Object}] See \{#options}
- def initialize(options)
- @options = options
+ # @param environment [Environment] See \{#environment}
+ def initialize(environment)
+ @environment = environment
+ @options = environment.options
end
# Asserts that the type of a given SassScript value
View
28 test/sass/engine_test.rb
@@ -11,6 +11,11 @@ module Sass::Script::Functions::UserFunctions
def option(name)
Sass::Script::String.new(@options[name.value.to_sym].to_s)
end
+
+ def set_a_local_variable
+ environment.set_var('variable', Sass::Script::Number.new(5))
+ return Sass::Script::Null.new
+ end
end
class SassEngineTest < Test::Unit::TestCase
@@ -1367,6 +1372,29 @@ def test_function_with_var
SASS
end
+ def test_user_defined_function_variable_scope
+ puts render(<<SASS)
+bar
+ -no-op: set_a_local_variable()
+ a: $variable
+SASS
+ flunk("Exception not raised for test_user_defined_function_variable_scope")
+ rescue Sass::SyntaxError => e
+ assert_equal('Undefined variable: "$variable".', e.message)
+ end
+
+ def test_user_defined_function_can_change_global_variable
+ assert_equal(<<CSS, render(<<SASS))
+bar {
+ a: 5; }
+CSS
+$variable: 0
+bar
+ -no-op: set_a_local_variable()
+ a: $variable
+SASS
+ end
+
def test_control_directive_in_nested_property
assert_equal(<<CSS, render(<<SASS))
foo {
View
22 test/sass/functions_test.rb
@@ -33,6 +33,10 @@ def user_defined
def _preceding_underscore
Sass::Script::String.new("I'm another user-defined string!")
end
+
+ def fetch_the_variable
+ environment.var('variable')
+ end
end
module Sass::Script::Functions
@@ -873,6 +877,11 @@ def test_user_defined_function_with_preceding_underscore
assert_equal("I'm another user-defined string!", evaluate("-preceding-underscore()"))
end
+ def test_user_defined_function_using_environment
+ environment = env('variable' => Sass::Script::String.new('The variable'))
+ assert_equal("The variable", evaluate("fetch_the_variable()", environment))
+ end
+
def test_options_on_new_literals_fails
assert_error_message(<<MSG, "call-options-on-new-literal()")
The #options attribute is not set on this Sass::Script::String.
@@ -1100,13 +1109,18 @@ def test_saturation_bounds
end
private
+ def env(hash = {})
+ env = Sass::Environment.new
+ hash.each {|k, v| env.set_var(k, v)}
+ env
+ end
- def evaluate(value)
- Sass::Script::Parser.parse(value, 0, 0).perform(Sass::Environment.new).to_s
+ def evaluate(value, environment = env)
+ perform(value, environment).to_s
end
- def perform(value)
- Sass::Script::Parser.parse(value, 0, 0).perform(Sass::Environment.new)
+ def perform(value, environment = env)
+ Sass::Script::Parser.parse(value, 0, 0).perform(environment)
end
def assert_error_message(message, value)
Something went wrong with that request. Please try again.