Skip to content

Conversation

VojtechBartos
Copy link
Contributor

New regex support is cool, but what i am missing is saving matched regex parameters to route parameters dictionary in DPLDeepLink. So i little bit refactored regex property in DPLRouteMatcher supports regex with parameter name in route specification.

Examples:

/table/:name([a-zA-Z]+)/:id([0-9]+)
/table/tableName/109

route parameters
{
   "name": "tableName",
   "id": "109"
}

-----

/url/:path(.*)
/url/super/long/cool/url

route parameters
{
   "path": "super/long/cool/url"
}

What do you think ?

@chrismaddern
Copy link
Contributor

Nice! Great idea!

Let me take a look at the code in a bit -- this seems really powerful!

@chrismaddern chrismaddern self-assigned this Feb 10, 2015
@chrismaddern
Copy link
Contributor

Hey, @VojtechBartos could you possibly sign our CLA ahead of getting this merged?

The CLA is an unfortunate necessity operating open source libraries as part of a company :) I'm just glad we can dedicate time to OS!

https://docs.google.com/forms/d/1WHFWnzz5OL8c9o49KgpgyERzQUQajX5p8_80YLCghds/viewform?usp=send_form

@VojtechBartos
Copy link
Contributor Author

@chrismaddern done 👍

@chrismaddern
Copy link
Contributor

@VojtechBartos Just an FYI - we're going to spend some time looking at this asap! The functionality is great, but we've begun to collect a lot of complexity in the lazy loading of -regex.

I think there may be a way to break it out to make the strategy for route-parsing more obvious and delegate it to a better named method now it's fairly complex.

cc/ @wessmith

@VojtechBartos
Copy link
Contributor Author

hey @chrismaddern ;) have you found some time looking at this ?

@chrismaddern
Copy link
Contributor

@wessmith I just took another look at this & really love the functionality. Let's take a look at this this week & can get this & #38 fixed in a new release :)

Sorry for the delay @VojtechBartos

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

Successfully merging this pull request may close these issues.

3 participants