Skip to content

Comments

only process spar directives within html assets.#18

Open
botandrose wants to merge 1 commit intoGoBoundless:masterfrom
botandrose:directives_only_for_html_assets
Open

only process spar directives within html assets.#18
botandrose wants to merge 1 commit intoGoBoundless:masterfrom
botandrose:directives_only_for_html_assets

Conversation

@botandrose
Copy link
Contributor

Heya, ran into some weird problems when trying to include handlebars-1.0.rc.1.js and ember-1.0.0-pre.2.js via sprockets. It turns out that both those javascript files contain several instances of [{ ... }] in the source, and so the spar directive processor is trying to gsub them out and failing. This patch only applies the directive processor if the current asset is an html document. Is this acceptable? Do you think one would ever want to use a spar directive within a non-html file?

@hmaurer
Copy link
Contributor

hmaurer commented Jan 5, 2013

Hey! I ran into the same issue a while ago:
#12

Personally I was using spar directives in my js files for api urls depending on the environment. I think we better change the default syntax to something not conflicting than restrict the usage to some file types.

In my projects I used [% ... %] which doesn't seem to raise any conflict atm.

default:
  interpolator_regex: "\[%(.*?)%\]"

@botandrose
Copy link
Contributor Author

Ah, I didn't realize you could change the regex. Nice! That seems like a good immediate fix.

However, this makes me ask, why do we even have this separate interpolation layer? Why not just process the templates with erb, with the context being the current environment config hash turned into an ostruct? I could imagine a concern about naming collisions if you use the config object as the top level erb context, but if 99% of your app is in javascript, and ruby is only handling configuration management...

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants