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

Yajl support #85

Merged
merged 5 commits into from Jun 15, 2011
Merged

Yajl support #85

merged 5 commits into from Jun 15, 2011

Conversation

weppos
Copy link
Contributor

@weppos weppos commented May 3, 2011

This branch adds JSON template support using Yajl.

The template is largely inspired by Builder and Nokogiri templates.

The idea originated creating a web service with Sinatra.
Quite often I find myself working with JSON responses (especially now that is the de-facto standard for AJAX script) and I found very useful to be able to write JSON templates like you can do with Builder.

I used Yajl because it's very efficient and, as a bonus, .yajl extension is not confusing like .json would be for a template file containing Ruby code.

weppos added 2 commits May 3, 2011 10:40
This template is largely inspired by the Nokogiri
and Builder templates.
@weppos
Copy link
Contributor Author

weppos commented May 3, 2011

This template works pretty well, but I just noticed a possible gotcha.

The template assumes the last statement is the "data" you want to convert to json. It might probably be a good idea to have a json buffer so that you can append elements, iterate or change the value as you wish without relying on "the last statement".

I'll experiment a little bit to see if it makes sense.

@judofyr
Copy link
Collaborator

judofyr commented May 3, 2011

What about wrapping it in something like this?

_json = (#{data}); defined?(json) ? json : _json

@rkh
Copy link
Collaborator

rkh commented May 3, 2011

👍 This would be awesome with sinatra-respond-with.

@weppos
Copy link
Contributor Author

weppos commented May 3, 2011

Hi @rkh,
thanks for stopping by. You're right, this is one of the reasons why I created this patch.

Before I forgot to do that, thanks for the huge amount of sinatra- resources and plugins you are actively maintaining. :)

In this way, the template is even more flexible.
I added some documentation to document the features.
@weppos
Copy link
Contributor Author

weppos commented May 3, 2011

I made a small refactoring in order to use a buffer variable (the gotcha is now fixed) and added some documentation.

@weppos
Copy link
Contributor Author

weppos commented May 31, 2011

It makes sense. I updated the code. All tests pass.
Any chance to get it merged into the mainstream repo? Do you believe this patch can be useful?

@rkh
Copy link
Collaborator

rkh commented Jun 4, 2011

I'm all for including it.

@rtomayko
Copy link
Owner

Hmm. I like it. It's non-invasive by taking only the .yajl extension (instead of .json or whatever). I'm not sure I like the idea of new template engines being built directly into tilt as opposed to gaining some following / usage as a separate lib and then being folded in. Let's see what happens.

rtomayko added a commit that referenced this pull request Jun 15, 2011
@rtomayko rtomayko merged commit 3108d55 into rtomayko:master Jun 15, 2011
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.

None yet

4 participants