sprinkle-tool / sprinkle

Sprinkle is a software provisioning tool you can use to build remote servers with. eg. to install a Rails, or Sinatra stack on a brand new slice directly after its been created
https://github.com/sprinkle-tool/sprinkle
MIT License
1.15k stars 138 forks source link

Updated package rendering so that opts are available in template #128

Closed koenpunt closed 11 years ago

koenpunt commented 11 years ago

Some modifications to rendering of files, also for better support of the defaults option of my other PR when one wants to use these defaults in a template.

The ERB rendering doesn't use the local binding anymore, but now merges opts and given options (second argument of both render and template) and use that as context.

Example: This

package :nginx do
  file '/etc/nginx/nginx.conf', :contents => render('nginx', :worker_processes => opts[:worker_processes] || 2)
end

with the template:

user www-data;
worker_processes <%= @worker_processes %>;
pid /run/nginx.pid;

events {
    worker_connections 768;
    # multi_accept on;
}
...

NB. This causes that local variables won't be passed on to the template anymore.

joshgoebel commented 11 years ago

I'm confused. I thought the hash passed created local variables inside the template scope not instance variables... is that wrong? I like being able to set instance variables in the package and see them in the template... does this change that? From your test it looks like the hash creates instance... or is that evaluate vs result? What is that change about?

joshgoebel commented 11 years ago

Yep. What if the package was passed as context to evaluate? Then you could access opts, args, etc directly inside the template as well as any instance variables defined by the package, no?

koenpunt commented 11 years ago

In answer of your second comment; yes that was the case before (using binding), but there is no need in having all of @name, @metadata, @provides, @dependencies, @recommends, @optional, @verifications, @installers, @block available in the template..

koenpunt commented 11 years ago

and setting instance variable in the package will be superseded by defaults anyway I guess. Because if it aint variable, it can be statically in the template.. Or is this not a correct assumption?

joshgoebel commented 11 years ago

For now we're sticking with binding being the default by now, though I did merge your request to get the absolute path tests. I very much like the principal of least surprise and dislike how scope jumps all over in Sprinkle now. We get issue reports because people just want to write simple packages but then opts doesn't work in a verify block... or doesn't work in the new template system... I don't think people should need to learn how to access things differently from different contexts - esp if the simplicity make it appear like there should be a single context.

If you need opts it's just opts... regardless of whether you're in a package block, verify block, template, etc... I don't think exposing those other instance variables is a huge issue... if it ever becomes a problem in practice then a proxy class could be written to reduce the context a bit.