search

rstacruz / sinatra-assetpack

Package your assets transparently in Sinatra.
http://ricostacruz.com/sinatra-assetpack/
MIT License
542 stars 97 forks source link

Add assets.cache_package_paths and unit test. #133

Open niallsmart opened 10 years ago

niallsmart commented 10 years ago

The current version of AssetPack always caches the list of files in a package between requests. This is not ideal in a development environment, when files are being added or reorganized fairly frequently.

This PR adds support for a new configuration option cache_package_paths which can be used to control the cache behavior. It disables caching of package contents in the development environment by default.

niallsmart commented 10 years ago

FYI – I also considered the name cache_package_contents but that implied it was actually caching the compiled contents. Another good option might be cache_package_definitions or cache_file_list.

j15e commented 10 years ago

Hello, Niall, thanks for the contribution but I think we are getting lost with caching optoons.

This is not your fault but I think this is getting messy, we would now have 3 options related to the caching of various parts of the asset manager with existing reload_files_cache, dynamic_asset_cache & this new one.

I think a rewrite of the caching option should be done to get only 1 caching option, all on or all off, not many small specific caching options that are getting complex to understand. Or, if we really want to keep specific options, they should be a bit more standardized & regrouped.

So for this particular reason I won't merge in master for now as-is although it be very tempting.

I won't work on a cleanup myself right now as I have less time available for the maintenance of this project : this is open for contributions.

niallsmart commented 10 years ago

@j15e yes I agree, it is getting a little confusing. I'm not even sure of the interactions between AssetPack's cache and Sinatra's caching. I will read through the code a little more and make a proposal that should hopefully result in simpler code/API. I do think it's important though to enable the developer to disable caching of the package file lists though, it's a bit of a pain to have to restart the server every time a new file is added or a file is renamed.

dentarg commented 10 years ago

@niallsmart Not a real solution, but I use https://github.com/alexch/rerun to restart the server when things happen in the filesystem. It works really well.