Required and optional parameters for route #490

Closed
ryanwi opened this Issue Mar 21, 2012 · 7 comments

Comments

Projects
None yet
5 participants
@ryanwi

ryanwi commented Mar 21, 2012

Per SO question (http://stackoverflow.com/questions/9775591/required-and-optional-parameters-for-sinatra-route) it would be useful to have required and optional parameters in route strings for things like adding ".json" to a resource id to build a JSON response.

This works fine:

get '/widgets.?:format?'

But, this currently does not:

get '/widgets/:id.?:format?'

If you pass 'abc.json' to that it, the :id parameter will be 'abc.json' and the format parameter will be empty.

The compiled regex appears to be:

/^\/widgets\/([^\/?#]+)(?:\.|%2E)?([^\/?#]+)?$/

The "." is not part of the exclusion in the first group, so it doesn't stop there as it would for a "/" or "#".

@ryanwi

This comment has been minimized.

Show comment
Hide comment
@ryanwi

ryanwi Mar 21, 2012

I did get past this by going full regex on the route and excluding the "." from the first regex group. But, this seems like a fairly common use case that would be helpful to support in regular route strings.

get %r{/widgets\/([^\/?#\.]+)(?:\.|%2E)?([^\/?#]+)?}

ryanwi commented Mar 21, 2012

I did get past this by going full regex on the route and excluding the "." from the first regex group. But, this seems like a fairly common use case that would be helpful to support in regular route strings.

get %r{/widgets\/([^\/?#\.]+)(?:\.|%2E)?([^\/?#]+)?}
@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Mar 22, 2012

Member

Discussion about possible solutions: https://gist.github.com/2154980

Member

rkh commented Mar 22, 2012

Discussion about possible solutions: https://gist.github.com/2154980

@todd-a-jacobs

This comment has been minimized.

Show comment
Hide comment
@todd-a-jacobs

todd-a-jacobs Oct 20, 2012

+1 This issue is biting me, too. The Sinatra DSL should handle format extensions such as .xml, .json, and so forth in a sensible and consistent way.

+1 This issue is biting me, too. The Sinatra DSL should handle format extensions such as .xml, .json, and so forth in a sensible and consistent way.

@jhwist

This comment has been minimized.

Show comment
Hide comment
@jhwist

jhwist Nov 30, 2012

Reading through the discussion in https://gist.github.com/2154980 and subsequently in #492, shouldn't this be fixed with 762967f?

jhwist commented Nov 30, 2012

Reading through the discussion in https://gist.github.com/2154980 and subsequently in #492, shouldn't this be fixed with 762967f?

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Nov 30, 2012

Member

Yes, indeed.

Member

rkh commented Nov 30, 2012

Yes, indeed.

@rkh rkh closed this Nov 30, 2012

@omnilinguist

This comment has been minimized.

Show comment
Hide comment
@omnilinguist

omnilinguist Dec 11, 2012

Does any official Sinatra release include this fix? I see that 1.3.3 is the latest, but this issue is listed as Milestone 1.4.0.

In the meantime, should we apply a similar workaround that the SO poster tried? How did he generate the compiled Sinatra regex from the route?

Does any official Sinatra release include this fix? I see that 1.3.3 is the latest, but this issue is listed as Milestone 1.4.0.

In the meantime, should we apply a similar workaround that the SO poster tried? How did he generate the compiled Sinatra regex from the route?

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Dec 11, 2012

Member

@omnilinguist you could also just run against Sinatra master. I'll see that I can prep a release soon.

Member

rkh commented Dec 11, 2012

@omnilinguist you could also just run against Sinatra master. I'll see that I can prep a release soon.

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