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

Support route globbing #69

Merged
merged 2 commits into from Apr 2, 2013
Merged

Support route globbing #69

merged 2 commits into from Apr 2, 2013

Conversation

le0pard
Copy link
Member

@le0pard le0pard commented Mar 28, 2013

Support route globbing in js-routes as in rails. Also fixed typeof problem, little cleanup of js.

@le0pard
Copy link
Member Author

le0pard commented Mar 28, 2013

Hope this pull request fix old issue #37

@jslag
Copy link

jslag commented Mar 29, 2013

👍 this solves the issue I was experiencing with paths getting spurious ?id= on their ends.

@@ -67,7 +67,7 @@ def draw_routes

match "/other_optional/(:optional_id)" => "foo#foo", :as => :foo

match 'books/*section/:title' => 'books#show', :as => :book
match 'books/:title/*section' => 'books#show', :as => :book
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you swap places of this?
In case we need to test this variant I would create another route.

May be should leave old tests for this route as some of the worked before.
And create another route as we implemented more use cases here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One moment, I will add another variant.

fix visit_globbing
add tests for objects
@bogdan
Copy link
Collaborator

bogdan commented Apr 2, 2013

great job!

bogdan added a commit that referenced this pull request Apr 2, 2013
@bogdan bogdan merged commit 488cff0 into railsware:master Apr 2, 2013
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

3 participants