Skip to content
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

:cache_location option is ignored #87

Closed
wants to merge 2 commits into from

Conversation

ordinaryzelig
Copy link

I used the instructions in the docs for Sass::Plugin::Rack to set the :cache_location option to something other than '.sass-cache'. But the cache_location was being ignored and the default '.sass-cache' was being used. I dug into the code and found out why: Sass::Plugin::Configuration#options was prematurely instantiating a new Sass::CacheStores::Filesystem object with the value of options[:cache_location]. The problem with this is that when I set the :cache_location like in the Sass::Plugin::Rack docs, calling options() immediately instantiates the Filesystem cache store even before I have a chance to change :cache_location. So setting :cache_location is useless.

To fix this, I set the default_proc in the default_options hash to set and instantiate the Sass::CacheStores::Filesystem object. This effectively delays the instantiation until it is needed. By that time, the :cache_location option can be set to something else.

I added the default_proc to the Rails and Merb plugins as well. I wrote tests for them in separate files for each. They were just copies of plugin_configuration_test.rb, but requiring the respective plugin file beforehand. But the Merb plugin required the Merb constant to be defined, which I didn't know how to do, so I removed that test.

So that's 1 commit. The other commit is a small one in the Sass::Plugin::Rack docs where 'merge' should be 'merge!'.

Sass::Plugin::Configuration was instantiating
Sass::CacheStores::Filesystem too early. Because of that,
setting the :cache_location after the fact would be useless.
@josepjaume
Copy link

I'm having the same issue. Will this be merged?

@kud
Copy link

kud commented Jun 6, 2012

+1

my sass_options = {:cache_location => "./cache/.sass-cache"} isn't used by compass.

@MoOx
Copy link

MoOx commented Jun 6, 2012

Use
sass_options = { :cache_location => "#{Compass.configuration.project_path}/.cache/.sass-cache" }

@kud
Copy link

kud commented Jun 6, 2012

I've used cache_path = 'cache/sass_cache' to work.

@h2non
Copy link

h2non commented Nov 28, 2013

Why this pull request was ignored?? I have the same issue
Please, consider to merge it!

@nex3
Copy link
Contributor

nex3 commented Jan 8, 2014

I believe this has been since fixed. The default cache store is now initialized based on :cache_location within the Engine constructor.

@nex3 nex3 closed this Jan 8, 2014
unicornrainbow pushed a commit to unicornrainbow/sass that referenced this pull request Nov 17, 2019
nex3 pushed a commit that referenced this pull request May 17, 2023
I bumped the version in #87 without realizing it had already been incremented on main since that PR was made.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants