Hoboken branch
Ryan Tomayko
use in other environments while maintaining a decent level of backward
compatibility with the current 3.2 release (lots more detail below). The
result is on my "hoboken" branch
(http://github.com/rtomayko/sinatra/tree/hoboken). Here's the gist of
what the changes would allow for:
This seems to be enjoying a lot of support in #sinatra so I wanted to
take the temperature on the mailing list to see what people think and
see if we can get a plan together for what happens next. Blake's been a
bit tied up over the holidays but I've bounced it off of him and I think
he could be convinced :)
If we decide that this is a good direction, I think we should do a new
0.9.x series of releases and start moving toward 1.0. The hoboken branch
deprecates some things (detailed below / up for debate) that would be
supported up until the 1.0 release.
Here's a more-or-less detailed description of everything that's changed
or been added (from the original commit message):
* The Event and EventContext classes have been removed. Sinatra
applications are now defined within the class context of a
Sinatra::Base subclass; each request is processed within a new
instance.
* Sinatra::Base can be used as a base class for multiple
Rack applications within a single process and can be used as
Rack middleware.
* The routing and result type processing implementation has been
simplified and enhanced a bit. There's a new route conditions
system for things like :agent/:host matching and a request
level #pass method has been added to allow an event handler to
exit immediately, passing control to the next matching route.
* Regular expressions may now be used in route patterns. Captures
are available as an array from "params[:captures]".
* The #body helper method now takes a block. The block is not
evaluated until an attempt is made to read the body.
* Options are now dynamically generated class attributes on the
Sinatra::Base subclass (instead of OpenStruct); options are
inherited by subclasses and may be overridden up the
inheritance hierarchy. The Base.set method manages all option
related stuff.
* The application file (app_file) detection heuristics are bit
more sane now. This fixes some bugs with reloading and
public/views directory detection. All thin / passenger issues
of these type should be better now.
* Error mappings are now split into to distinct layers: exception
mappings and custom error pages. Exception mappings are registered
with 'error(Exception)' and are run only when the app raises an
exception. Custom error pages are registered with error(status_code)
and are run any time the response has the status code specified.
It's also possible to register an error page for a range of status
codes: 'error(500..599)'.
* The spec and unit testing extensions have been modified to take
advantage of the ability to have multiple Sinatra applications.
The Sinatra::Test module must be included within the TestCase
in order to take advantage of these methods (unless the
'sinatra/compat' library has been required).
* Rebuilt specs from scratch for better coverage and
organization. Sinatra 3.2 unit tests have been retained
under ./compat to ensure a baseline level of compatibility with
previous versions; use the 'rake compat' task to run these.
A large number of existing Sinatra idioms have been deprecated but
continue to be supported through the 'sinatra/compat' library.
* The "set_option" and "set_options" methods have been deprecated
due to redundancy; use "set".
* The "env" option (Sinatra::Base.env) has been renamed to "environment"
and deprecated because it's too easy to confuse with the request-level
Rack environment Hash (Sinatra::Base#env).
* The request level "stop" method has been renamed "halt" and
deprecated. This is for consistency with `throw :halt`.
* The request level "entity_tag" method has been renamed "etag" and
deprecated. Both versions were previously supported.
* The request level "headers" method has been deprecated. Use
response['Header-Name'] to access and modify response headers.
* Sinatra.application is deprecated. Use Sinatra::Application instead.
* Setting Sinatra.application = nil to reset an application is
deprecated. You shouldn't have to reset objects anymore.
* The Sinatra.default_options Hash is deprecated. Modifying this
object now results in "set(key, value)" invocations on the
Sinatra::Base subclass.
* The "body.to_result" convention has been deprecated.
* The ServerError exception has been deprecated. Any Exception is now
considered a ServerError.
Questions? Comments? Flames?
Thanks,
Ryan
Simon Rozet
>
> So, last week I took a stab at reorganizing Sinatra to make it easier to
> use in other environments while maintaining a decent level of backward
> compatibility with the current 3.2 release (lots more detail below). The
> result is on my "hoboken" branch
>
> Questions? Comments? Flames?
Ryan,
A clean rewrite, with clean tests and backward compatibility? Oh yeah!
Awesome work.
I am glade you're (again) moving Sinatra forward. Thank you :)
Here are my comments, questions, and suggestion:
* I am not a fan of the `condition {}` syntax. I'd rather see
something like this:
condition :foo do |request|
request.env["HTTP_USER_AGENT"] =~ /foo/
end
get "/", :condition => proc { |request| true }
get "/", :filter => :foo # or :filter => [:foo, :bar, ...]
or why not:
condition :proc => { |request| nil } do
get "/"
post "/"
etc
end
Also, why is it only implemented for GET?
* I think that in order to help people to migrate gracefully, we
should actually display depreciation notice
* AFAIK, sinatra/test suffers from little-by-annoying issues. Wouldn't
now be a good time to fix it?
Here is a suggestion (lacking BC, though) http://gist.github.com/41270
* There are some questions regarding "plugins" in Sinatra. I am not
exactly sure what it means but I think we should look at things like
http://github.com/foca/sinatra-diddies and other various "plugins"
available at http://gist.github.com/peterc and agree on a standard
"API for plugins" or something.
That's all I can think of for now. Looking forward to see what other
people are thinking :)
> Thanks,
> Ryan
--
Simon Rozet -- <si...@rozet.name>
blake.m...@gmail.com
Ryan Tomayko
> On Mon, Dec 29, 2008 at 1:07 PM, Ryan Tomayko <r...@tomayko.com> wrote:
>> So, last week I took a stab at reorganizing Sinatra to make it easier to
>> use in other environments while maintaining a decent level of backward
>> compatibility with the current 3.2 release (lots more detail below). The
>> result is on my "hoboken" branch
>> [...]
>>
>> Questions? Comments? Flames?
>
> Awesome work.
> I am glade you're (again) moving Sinatra forward. Thank you :)
>
> Here are my comments, questions, and suggestion:
Thanks for the feedback, Simon. There's a lot of good stuff in your
response that I want to make sure gets the attention/discussion it
deserves so I'm going to break my response up into a few separate topic
threads.
One thing I want to stress is that the hoboken branch is still very much
a proposal and everything on it is up for debate. Nothing there is set
in stone. I want to take the time to vet and debate any ideas/concerns
here before we make any concrete plans.
Also, while some additional features made it into the branch, my
original intention was to limit development to 1.) reorganizing the code
to be a bit more modular / less intrusive, and 2.) deprecating
redundancies and functionality that's not often used. The goal is to get
a more solid foundation in place for implementing new features in
subsequent releases.
More on the way ...
Thanks,
Ryan
Ryan Tomayko
> something like this:
> condition :foo do |request|
> request.env["HTTP_USER_AGENT"] =~ /foo/
> end
>
> get "/", :condition => proc { |request| true }
> get "/", :filter => :foo # or :filter => [:foo, :bar, ...]
We could do something like that. What I was going for here was a simple
mechanism that could handle the conditions that were already in place --
user agent matching with :agent, and host matching with :host. I tried
to do the simplest thing that could possibly work for those cases and
can imagine more (:accept, :language, etc.) that would fit the basic
model quite well.
Also, I should note that I envision "condition" being used internally
more than as a general purpose method in apps. It's a simple framework
for implementing route options. I'm more than happy to consider
expanding it to support additional cases but I wanted to get something
generic in for what we're doing today.
> or why not:
>
> condition :proc => { |request| nil } do
> get "/"
> post "/"
> etc
> end
Yep. We should definitely do something like that. The basic idea being a
condition that applies to all events defined within a block instead of
the next route only. You could imagine how something like nested routes
could be implemented with such a construct:
route "/foo/bar" do
get "/baz"
get "/bizzle"
...
end
Like I said, I wanted a system for implementing the current route
conditionals (:agent, :host) cleanly and straightforwardly. There are
definitely other cases that likely call for additional / more advanced
constructs.
> Also, why is it only implemented for GET?
It should be implemented for all request methods. If not, it's a bug.
What gave you the impression that it was only for GET?
Thanks,
Ryan
Ryan Tomayko
> should actually display depreciation notice
Me too. Do provide a link if you have an example of best practices
around issuing deprecation warnings. I didn't want to just spew all over
STDERR if there's a more elegant way. Everything that's been deprecated
lives in "lib/sinatra/compat.rb" so we should be able to add this fairly
easily.
Do most libs use Kernel#warn for this or ...?
> AFAIK, sinatra/test suffers from little-by-annoying issues. Wouldn't
> now be a good time to fix it? Here is a suggestion (lacking BC,
> though) http://gist.github.com/41270
Yes!
The testing framework was cleaned up a bit but needs to be revisited and
thought through a bit more. I very much like the approach you've taken
in your example. And especially the idea of encapsulating the app,
request, and response within a separate object. Something like
"Sinatra::MockSession" or "Sinatra::MockClient" might be a bit better
naming scheme but the concept is dead on.
If you want to take a crack at this, consider developing it
independently within "lib/sinatra/mock.rb" or something and we can start
to move the existing testing stuff to use it once nailed down.
Thanks,
Ryan
Ryan Tomayko
> exactly sure what it means but I think we should look at things like
> http://github.com/foca/sinatra-diddies and other various "plugins"
> available at http://gist.github.com/peterc and agree on a standard
> "API for plugins" or something.
Oh, interesting. I hadn't seen that. Here's the thing: writing plugins
like this for the hoboken branch should be very straightforward without
any additional code in the framework. There's basically two receivers
that one would want to extend: 1.) Sinatra::Base class methods for
adding new app-level (DSL-ish) stuff, and 2.) Sinatra::Base instance
methods for request level helpers (like the diddies piece you've pointed
to). I doubt we need anything more formal to support "plugins".
What might be a good idea, though, would be to document common patterns
for implementing plugins. Like, do you just re-open the Sinatra::Base
class and add stuff or is it better to stuff things into modules and
then allow apps to include/extend as necessary.
Also worth noting is that existing plugins are very likely to be
impacted by the hoboken changes. This is an area where backward
compatibility kind of fell apart. The Event and EventContext classes are
gone and Sinatra::Application plays a much different role. At least a
little bit of port work is going to need to be done for plugins that
modify these classes. Plugins that are shipped as gems should probably
declare the sinatra dependency as "~> 0.3". If and when hoboken is
released, we'll increment the minor rev to 0.4 at least and 0.9 if I get
my way :)
Thanks,
Ryan
Nicolás Sanguinetti
>
> On 12/29/08 5:54 AM, Simon Rozet wrote:
>> * I am not a fan of the `condition {}` syntax. I'd rather see
>> something like this:
>> condition :foo do |request|
>> request.env["HTTP_USER_AGENT"] =~ /foo/
>> end
>>
>> get "/", :condition => proc { |request| true }
>> get "/", :filter => :foo # or :filter => [:foo, :bar, ...]
About this, I don't like to use different words for it (:condition,
:filter). I'd just use :condition => :foo (or lambda {...}, or [:foo,
:bar, :baz]).
I'd propose using :if though.
Also, for those "usual" conditions you envision Ryan, what about
having helpers like:
get "/people", :if => accepts("text/xml") do
builder :people
end
get "/people" do
haml :people
end
WDYT?
-foca
Ryan Tomayko
> Why deprecate the body.to_result?
I've never seen this used outside of Sinatra's core implementation and
it led to us modifying a bunch of core Ruby objects (we added #to_result
to Array, Fixnum, NilClass, Proc, String, and Symbol). I tried to
support all of the cases that the core object #to_result implementations
enabled directly in the "Base#invoke" method.
As of right now this isn't even technically deprecated, it's actually
completely unsupported. We should -- at the very least -- add support
for calling to_result on event return values back in and actually
deprecate it. I'm also not against supporting it outright. It has merit.
I just have never seen it used in the wild.
In fact, I should just sidebar here and mention that I tried to be
absolutely ruthless with deprecation. It's very easy to undeprecate
stuff before this becomes anything official so I figured I'd move
anything that could potentially be deprecated into one place. Take a
look in "lib/sinatra/compat.rb" and see if there's anything else that
makes sense to leave supported:
http://github.com/rtomayko/sinatra/tree/hoboken/lib/sinatra/compat.rb
Thanks,
Ryan
Aaron Quint
might be nice to have a very very simple dependency system. Something
like a super simple version of rails configure.gem. The main purpose
being to be able to define what gem or libs your app depends on,
specifically what versions, and then being able to ccatch load errors
an warn that your missing code. With Sinatra it might be cool if it
tried to load from vendor/ first and then proceeded to use gem. I'm
happy to do a quick attempt at this if there's interest.
--AQ
Aaron Quint
Aaron Quint
'condition' is just a Proc that yields the request object so you could
check against anything. Then you could define as many procs as you
want and in the route just do
get '/people', :if => proc_var do
...
It could also take an array of procs.
?
--AQ
Gus
> On 12/29/08 5:54 AM, Simon Rozet wrote:
>
> Oh, interesting. I hadn't seen that. Here's the thing: writing plugins
> like this for the hoboken branch should be very straightforward without
> any additional code in the framework.
about plugins, we're talking about gems that "weave" themselves into
Sinatra when required. I hope Simon was not suggesting a plugin
hierarchy in the vein of Rails'. I've written my fair share of them
(Rails plugins) and feel pretty bad about it now.
> What might be a good idea, though, would be to document common patterns
> for implementing plugins. Like, do you just re-open the Sinatra::Base
> class and add stuff or is it better to stuff things into modules and
> then allow apps to include/extend as necessary.
Patterns for testing would also be helpful. I did some hackery in
chicago (my plugin) to allow me to test helpers:
def should_invoke_helper(name, *args, &assert_block)
should "invoke helper #{name} with #{args.inspect}" do
@response = OpenStruct.new(:body => nil)
event_context = ::Sinatra::EventContext.new(nil, @response, nil)
@response.body = event_context.send(name, *args)
self.instance_eval(&assert_block)
end
end
So that I could do this in my test:
class HelpersTest < Test::Unit::TestCase
should_invoke_helper(:stylesheet_include, 'http://example.com') do
assert_match %r[href="http://example.com"], @response.body
end
...
end
Seeing the changes coming from Hoboken, I would no longer need to do
this; but some common idioms that get packaged into a slightly more
extensive test case helper would be awesome. Otherwise, I'll just keep
adding them to chicago :)
> Also worth noting is that existing plugins are very likely to be
> impacted by the hoboken changes.
> Thanks,
> Ryan
Thanks,
Gus
Nicolás Sanguinetti
>
> On Dec 30, 3:56 am, Ryan Tomayko <r...@tomayko.com> wrote:
>> On 12/29/08 5:54 AM, Simon Rozet wrote:
>>
>> Oh, interesting. I hadn't seen that. Here's the thing: writing plugins
>> like this for the hoboken branch should be very straightforward without
>> any additional code in the framework.
>
> Just to throw my two cents in the ring; I hope that when we're talking
> about plugins, we're talking about gems that "weave" themselves into
> Sinatra when required. I hope Simon was not suggesting a plugin
> hierarchy in the vein of Rails'. I've written my fair share of them
> (Rails plugins) and feel pretty bad about it now.
pretty sure he doesn't mean plugins ala rails.
methods in isolation =)
The good thing is, unless I missed something huge, since helpers are
normal instance methods on the app now, just having a test helper that
returns the instance of the current app let's us do all the work.
application.should_receive(:my_helper) and blah blah blah
Cheers
-foca
Blake Mizerany
On 12/29/08 5:54 AM, Simon Rozet wrote:
> I think that in order to help people to migrate gracefully, we
> should actually display depreciation notice
Me too. Do provide a link if you have an example of best practices
around issuing deprecation warnings. I didn't want to just spew all over
STDERR if there's a more elegant way. Everything that's been deprecated
lives in "lib/sinatra/compat.rb" so we should be able to add this fairly
easily.
Do most libs use Kernel#warn for this or ...?
I don't have much an opinion here but if there is something built-in for warnings then I guess I'm more for that.
> AFAIK, sinatra/test suffers from little-by-annoying issues. Wouldn't
> now be a good time to fix it? Here is a suggestion (lacking BC,
> though) http://gist.github.com/41270
Yes!
The testing framework was cleaned up a bit but needs to be revisited and
thought through a bit more. I very much like the approach you've taken
in your example. And especially the idea of encapsulating the app,
request, and response within a separate object. Something like
"Sinatra::MockSession" or "Sinatra::MockClient" might be a bit better
naming scheme but the concept is dead on.
If you want to take a crack at this, consider developing it
independently within "lib/sinatra/mock.rb" or something and we can start
to move the existing testing stuff to use it once nailed down.
I agree 100%. The test-framework needs love. Maybe we can get MockSession into Rack. I feel it would have a better home there. Rack already has MockRequest/Response for testing. For now we can make it part of Sinatra and offer the patch to Rack later?
Thanks,
Ryan
--
Blake Mizerany
blake.m...@gmail.com
Ryan Tomayko
> On Dec 30, 3:56 am, Ryan Tomayko <r...@tomayko.com> wrote:
>> On 12/29/08 5:54 AM, Simon Rozet wrote:
>>
>> Oh, interesting. I hadn't seen that. Here's the thing: writing plugins
>> like this for the hoboken branch should be very straightforward without
>> any additional code in the framework.
>
> Just to throw my two cents in the ring; I hope that when we're talking
> about plugins, we're talking about gems that "weave" themselves into
> Sinatra when required. I hope Simon was not suggesting a plugin
> hierarchy in the vein of Rails'. I've written my fair share of them
> (Rails plugins) and feel pretty bad about it now.
Yeah. We're not going down the path of auto loading code in a plugin
directory by convention or anything. Plugins should be normal ruby
libraries, which could be packaged as gems. Apps should have to
explicitly require the plugin but I think it would be alright for the
plugin to go ahead and latch onto Sinatra's core classes in most cases.
Some basic pattern + doc + maybe test helpers is all we really need
here, IMO. I've opened a ticket in lighthouse to track this. If anyone
wants to take a stab at a concrete proposal, do update the ticket:
http://sinatra.lighthouseapp.com/projects/9779/tickets/54-support-for-plugins
Thanks,
Ryan