Permalink
Browse files

Sets default content type according to template engine used instead o…

…f just text/html.

It does so by including a Mixin into the the returned string offering a content_type method. Therefore all of the following examples produce the expected results:

    # text/html
    get('/') do
      haml :index
    end

    # text/css
    get('/') do
      sass :index
    end

    # text/css
    get('/') do
      haml :index
      sass :index
    end

    # text/html
    get('/') do
      haml '= sass :index'
    end

It also allows setting the default content type for a template engine:

    set :builder, :content_type => :html

Tests and README adjustments (all languages) included.
  • Loading branch information...
rkh committed Sep 19, 2010
1 parent e5e0047 commit 1d676f41f855cdab9aacda0037a4217ec3c9c45e
Showing with 180 additions and 38 deletions.
  1. +0 −7 README.de.rdoc
  2. +0 −3 README.jp.rdoc
  3. +0 −7 README.rdoc
  4. +24 −6 lib/sinatra/base.rb
  5. +25 −1 test/builder_test.rb
  6. +25 −1 test/coffee_test.rb
  7. +1 −0 test/helper.rb
  8. +7 −7 test/helpers_test.rb
  9. +26 −2 test/less_test.rb
  10. +1 −1 test/routing_test.rb
  11. +25 −1 test/sass_test.rb
  12. +25 −1 test/scss_test.rb
  13. +21 −1 test/templates_test.rb
View
@@ -229,7 +229,6 @@ Das buidler gem wird benötigt um Builder-Templates rendern zu können:
require 'builder'
get '/' do
- content_type 'application/xml', :charset => 'utf-8'
builder :index
end
@@ -243,7 +242,6 @@ Das haml gem wird benötigt um SASS-Templates rendern zu können:
require 'sass'
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
sass :stylesheet
end
@@ -257,7 +255,6 @@ und individuell überschrieben werden.
set :sass, :style => :compact # Standard Sass-Style ist :nested
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
sass :stylesheet, :style => :expanded # überschrieben
end
@@ -269,7 +266,6 @@ Das haml gem wird benötigt um SCSS-Templates rendern zu können:
require 'sass'
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
scss :stylesheet
end
@@ -283,7 +279,6 @@ und individuell überschrieben werden.
set :scss, :style => :compact # Standard Scss-Style ist :nested
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
scss :stylesheet, :style => :expanded # überschrieben
end
@@ -295,7 +290,6 @@ Das less gem wird benötigt um Less-Templates rendern zu können:
require 'less'
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
less :stylesheet
end
@@ -434,7 +428,6 @@ Das coffee-script gem und das `coffee`-Programm werden benötigt um CoffeScript-
require 'coffee-script'
get '/application.js' do
- content_type 'text/javascript', :charset => 'utf-8'
coffee :application
end
View
@@ -154,7 +154,6 @@ builderを使うにはbuilderライブラリが必要です:
require 'builder'
get '/' do
- content_type 'application/xml', :charset => 'utf-8'
builder :index
end
@@ -168,7 +167,6 @@ Sassテンプレートを使うにはsassライブラリが必要です:
require 'sass'
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
sass :stylesheet
end
@@ -182,7 +180,6 @@ see {Options and Configurations}[http://www.sinatrarb.com/configuration.html],
set :sass, {:style => :compact } # デフォルトのSass styleは :nested
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
sass :stylesheet, :sass_options => {:style => :expanded } # 上書き
end
View
@@ -225,7 +225,6 @@ The builder gem/library is required to render builder templates:
require 'builder'
get '/' do
- content_type 'application/xml', :charset => 'utf-8'
builder :index
end
@@ -239,7 +238,6 @@ The sass gem/library is required to render Sass templates:
require 'sass'
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
sass :stylesheet
end
@@ -253,7 +251,6 @@ and overridden on an individual basis.
set :sass, :style => :compact # default Sass style is :nested
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
sass :stylesheet, :style => :expanded # overridden
end
@@ -265,7 +262,6 @@ The sass gem/library is required to render Scss templates:
require 'sass'
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
scss :stylesheet
end
@@ -279,7 +275,6 @@ and overridden on an individual basis.
set :scss, :style => :compact # default Scss style is :nested
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
scss :stylesheet, :style => :expanded # overridden
end
@@ -291,7 +286,6 @@ The less gem/library is required to render Less templates:
require 'less'
get '/stylesheet.css' do
- content_type 'text/css', :charset => 'utf-8'
less :stylesheet
end
@@ -422,7 +416,6 @@ CoffeeScript templates:
require 'coffee-script'
get '/application.js' do
- content_type 'text/javascript', :charset => 'utf-8'
coffee :application
end
View
@@ -77,10 +77,12 @@ def status(value=nil)
# evaluation is deferred until the body is read with #each.
def body(value=nil, &block)
if block_given?
- def block.each ; yield call ; end
+ def block.each; yield(call) end
response.body = block
- else
+ elsif value
response.body = value
+ else
+ response.body
end
end
@@ -137,6 +139,7 @@ def mime_type(type)
def content_type(type, params={})
mime_type = mime_type(type)
fail "Unknown media type: %p" % type if mime_type.nil?
+ params[:charset] ||= defined?(Encoding) ? Encoding.default_external.to_s.downcase : 'utf-8'
if params.any?
params = params.collect { |kv| "%s=%s" % kv }.join(', ')
response['Content-Type'] = [mime_type, params].join(";")
@@ -300,6 +303,10 @@ def back ; request.referer ; end
# :locals A hash with local variables that should be available
# in the template
module Templates
+ module ContentTyped
+ attr_accessor :content_type
+ end
+
include Tilt::CompileSite
def erb(template, options={}, locals={})
@@ -315,21 +322,22 @@ def haml(template, options={}, locals={})
end
def sass(template, options={}, locals={})
- options[:layout] = false
+ options.merge! :layout => false, :default_content_type => :css
render :sass, template, options, locals
end
def scss(template, options={}, locals={})
- options[:layout] = false
+ options.merge! :layout => false, :default_content_type => :css
render :scss, template, options, locals
end
def less(template, options={}, locals={})
- options[:layout] = false
+ options.merge! :layout => false, :default_content_type => :css
render :less, template, options, locals
end
def builder(template=nil, options={}, locals={}, &block)
+ options[:default_content_type] = :xml
options, template = template, nil if template.is_a?(Hash)
template = Proc.new { block } if template.nil?
render :builder, template, options, locals
@@ -360,7 +368,7 @@ def markaby(template, options={}, locals={})
end
def coffee(template, options={}, locals={})
- options[:layout] = false
+ options.merge! :layout => false, :default_content_type => :js
render :coffee, template, options, locals
end
@@ -376,6 +384,7 @@ def render(engine, data, options={}, locals={}, &block)
@default_layout = :layout if @default_layout.nil?
layout = options.delete(:layout)
layout = @default_layout if layout.nil? or layout == true
+ content_type = options.delete(:content_type) || options.delete(:default_content_type)
# compile and render template
layout_was = @default_layout
@@ -393,6 +402,7 @@ def render(engine, data, options={}, locals={}, &block)
end
end
+ output.extend(ContentTyped).content_type = content_type if content_type

This comment has been minimized.

Show comment Hide comment
@ryansobol

ryansobol Oct 19, 2010

Huh? Is this really the best way of adding a content_type accessor to a String instance (I presume) ? +1 clever points at least. :P

@ryansobol

ryansobol Oct 19, 2010

Huh? Is this really the best way of adding a content_type accessor to a String instance (I presume) ? +1 clever points at least. :P

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Oct 19, 2010

Owner

It avoids tons of edge cases (what if you nest different template engines, what if you call haml just for fun and return something different instead). Even though it doesn't seem so at first glance, this is rather noninvasive and OO. By extending output with a mixin we avoid monkey-patching plus get better documentation. But if you have another approach in mind, I'd love to wrap my head around it.

@rkh

rkh Oct 19, 2010

Owner

It avoids tons of edge cases (what if you nest different template engines, what if you call haml just for fun and return something different instead). Even though it doesn't seem so at first glance, this is rather noninvasive and OO. By extending output with a mixin we avoid monkey-patching plus get better documentation. But if you have another approach in mind, I'd love to wrap my head around it.

This comment has been minimized.

Show comment Hide comment
@ryansobol

ryansobol Oct 19, 2010

Your rationale seems sound, although I'm still wrestling how we get "better documentation" with this technique. I guess it makes sense if you're referring exclusively to Sinatra contributors. I might have gone with a different technique, encapsulating output and it's meta-data in a first-class object. But this surely would have lead to more code, more tests, more potential bugs, and more time. Your technique is quick and precise, which is essential for the next release.

BTW - Konstantin, you've done a f*cking amazing job with organizing this 1.1 release. I'm really excited about smart templates. When I upgrade Sinatra in my project, I'm going to delete so much code! And my users are going to love having more template language choices! I hope you don't mind a little peer-reviewing from me.

@ryansobol

ryansobol Oct 19, 2010

Your rationale seems sound, although I'm still wrestling how we get "better documentation" with this technique. I guess it makes sense if you're referring exclusively to Sinatra contributors. I might have gone with a different technique, encapsulating output and it's meta-data in a first-class object. But this surely would have lead to more code, more tests, more potential bugs, and more time. Your technique is quick and precise, which is essential for the next release.

BTW - Konstantin, you've done a f*cking amazing job with organizing this 1.1 release. I'm really excited about smart templates. When I upgrade Sinatra in my project, I'm going to delete so much code! And my users are going to love having more template language choices! I hope you don't mind a little peer-reviewing from me.

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Oct 19, 2010

Owner

Thanks. No, it's fine, welcomed even. The reason for not creating a own class is that we had to cover every possibility where such a class could be handed to rack, including using it for streaming (wrapping it in another project) or with tools like async-sinatra, as Rack expects strings. Returning something that just behaves like a string would violate the Rack spec. With documentation I mean as opposed to monkey-patching the string directly. The mixin can actually show up in generated documentation (not that it matters much without comments, but it's a start). If you got more feedback/discussion, keep it coming.

@rkh

rkh Oct 19, 2010

Owner

Thanks. No, it's fine, welcomed even. The reason for not creating a own class is that we had to cover every possibility where such a class could be handed to rack, including using it for streaming (wrapping it in another project) or with tools like async-sinatra, as Rack expects strings. Returning something that just behaves like a string would violate the Rack spec. With documentation I mean as opposed to monkey-patching the string directly. The mixin can actually show up in generated documentation (not that it matters much without comments, but it's a start). If you got more feedback/discussion, keep it coming.

This comment has been minimized.

Show comment Hide comment
@ryansobol

ryansobol Oct 19, 2010

It's no question that compatibility with the Rack spec is paramount (for all Ruby web frameworks). And I would never advocate violating that. Using the technique of a wrapper class, as I've hinted, would require more work to ensure compatibility with Rack (and with async-sinatra for that matter). Right now, I don't think it's worth the time investment.

As far as improving Sinatra's documentation, I wish I had more time to pitch in. :( It's a good thing the source is so readable already. :P I've actually added quite a few techniques into my own toolbox from periodically perusing the source.

@ryansobol

ryansobol Oct 19, 2010

It's no question that compatibility with the Rack spec is paramount (for all Ruby web frameworks). And I would never advocate violating that. Using the technique of a wrapper class, as I've hinted, would require more work to ensure compatibility with Rack (and with async-sinatra for that matter). Right now, I don't think it's worth the time investment.

As far as improving Sinatra's documentation, I wish I had more time to pitch in. :( It's a good thing the source is so readable already. :P I've actually added quite a few techniques into my own toolbox from periodically perusing the source.

output
end
@@ -457,8 +467,16 @@ def call!(env) # :nodoc:
template_cache.clear if settings.reload_templates
force_encoding(@params)
+ @response['Content-Type'] = nil
invoke { dispatch! }
invoke { error_block!(response.status) }
+ unless @response['Content-Type']
+ if body.respond_to?(:to_ary) and body.first.respond_to? :content_type
+ content_type body.first.content_type
+ else
+ content_type :html
+ end
+ end
status, header, body = @response.finish
View
@@ -2,9 +2,10 @@
require 'builder'
class BuilderTest < Test::Unit::TestCase
- def builder_app(&block)
+ def builder_app(options = {}, &block)
mock_app {
set :views, File.dirname(__FILE__) + '/views'
+ set options
get '/', &block
}
get '/'
@@ -16,6 +17,29 @@ def builder_app(&block)
assert_equal %{<?xml version="1.0" encoding="UTF-8"?>\n}, body
end
+ it 'defaults content type to xml' do
+ builder_app { builder 'xml.instruct!' }
+ assert ok?
+ assert_equal "application/xml;charset=utf-8", response['Content-Type']
+ end
+
+ it 'defaults allows setting content type per route' do
+ builder_app do
+ content_type :html
+ builder 'xml.instruct!'
+ end
+ assert ok?
+ assert_equal "text/html;charset=utf-8", response['Content-Type']
+ end
+
+ it 'defaults allows setting content type globally' do
+ builder_app(:builder => { :content_type => 'html' }) do
+ builder 'xml.instruct!'
+ end
+ assert ok?
+ assert_equal "text/html;charset=utf-8", response['Content-Type']
+ end
+
it 'renders inline blocks' do
builder_app {
@name = "Frank & Mary"
View
@@ -4,9 +4,10 @@
require 'coffee-script'
class CoffeeTest < Test::Unit::TestCase
- def coffee_app(&block)
+ def coffee_app(options = {}, &block)
mock_app {
set :views, File.dirname(__FILE__) + '/views'
+ set(options)
get '/', &block
}
get '/'
@@ -18,6 +19,29 @@ def coffee_app(&block)
assert_equal "(function() {\n alert('Aye!');\n})();\n", body
end
+ it 'defaults content type to javascript' do
+ coffee_app { coffee "alert 'Aye!'\n" }
+ assert ok?
+ assert_equal "application/javascript;charset=utf-8", response['Content-Type']
+ end
+
+ it 'defaults allows setting content type per route' do
+ coffee_app do
+ content_type :html
+ coffee "alert 'Aye!'\n"
+ end
+ assert ok?
+ assert_equal "text/html;charset=utf-8", response['Content-Type']
+ end
+
+ it 'defaults allows setting content type globally' do
+ coffee_app(:coffee => { :content_type => 'html' }) do
+ coffee "alert 'Aye!'\n"
+ end
+ assert ok?
+ assert_equal "text/html;charset=utf-8", response['Content-Type']
+ end
+
it 'renders .coffee files in views path' do
coffee_app { coffee :hello }
assert ok?
View
@@ -1,4 +1,5 @@
ENV['RACK_ENV'] = 'test'
+Encoding.default_external = "UTF-8" if defined? Encoding
begin
require 'rack'
View
@@ -291,21 +291,21 @@ def test_default
}
get '/'
- assert_equal 'text/plain', response['Content-Type']
+ assert_equal 'text/plain;charset=utf-8', response['Content-Type']
assert_equal 'Hello World', body
end
it 'takes media type parameters (like charset=)' do
mock_app {
get '/' do
- content_type 'text/html', :charset => 'utf-8'
+ content_type 'text/html', :charset => 'latin1'
"<h1>Hello, World</h1>"
end
}
get '/'
assert ok?
- assert_equal 'text/html;charset=utf-8', response['Content-Type']
+ assert_equal 'text/html;charset=latin1', response['Content-Type']
assert_equal "<h1>Hello, World</h1>", body
end
@@ -320,7 +320,7 @@ def test_default
get '/foo.xml'
assert ok?
- assert_equal 'application/foo', response['Content-Type']
+ assert_equal 'application/foo;charset=utf-8', response['Content-Type']
assert_equal 'I AM FOO', body
end
@@ -366,19 +366,19 @@ def send_file_app(opts={})
it 'sets the Content-Type response header if a mime-type can be located' do
send_file_app
get '/file.txt'
- assert_equal 'text/plain', response['Content-Type']
+ assert_equal 'text/plain;charset=utf-8', response['Content-Type']
end
it 'sets the Content-Type response header if type option is set to a file extesion' do
send_file_app :type => 'html'
get '/file.txt'
- assert_equal 'text/html', response['Content-Type']
+ assert_equal 'text/html;charset=utf-8', response['Content-Type']
end
it 'sets the Content-Type response header if type option is set to a mime type' do
send_file_app :type => 'application/octet-stream'
get '/file.txt'
- assert_equal 'application/octet-stream', response['Content-Type']
+ assert_equal 'application/octet-stream;charset=utf-8', response['Content-Type']
end
it 'sets the Content-Length response header' do
Oops, something went wrong.

0 comments on commit 1d676f4

Please sign in to comment.