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

routes' names are very restrictive #3

Merged
merged 1 commit into from
May 18, 2012

Conversation

@dbu
Copy link
Member

dbu commented May 16, 2012

hm, mine is probably too restrictive, but yours does look too permissive. it would allow spaces and whatnot in the name. do we have a spec telling what a valid route name is for symfony?

@uwej711
Copy link
Member Author

uwej711 commented May 16, 2012

@dbu
Copy link
Member

dbu commented May 16, 2012

thanks, that was where i copied it from i think. this means we keep the current rule, right? we do not want anything in the generated name that symfony then will complain on.

@dbu
Copy link
Member

dbu commented May 17, 2012

do we use the name somewhere? maybe we could transform the passed in name with the same regexp to make it more user friendly. however, this could lead to false matches like /ö and /ä becoming both __. for the current way we build the collection, things work fine as we walk up the tree in a breadcrumb sort of way, so every route has a different length.

for the getRoutecollection implementation #5 we will need to think carefully about naming too, when dumping the whole tree we must avoid name clashes that occur with the simple replace i have.

@uwej711
Copy link
Member Author

uwej711 commented May 18, 2012

My problem was that I had a route which contains a '-' (minus) which I think is very common. Symfony later complains about the route name since it only allows letters, digits, underscore and dot. So I just took the short cut to change everything which is not letter, digit, underscore or dot to an underscore ...

dbu added a commit that referenced this pull request May 18, 2012
routes' names are very restrictive
@dbu dbu merged commit 8c83f16 into symfony-cmf:master May 18, 2012
@dbu
Copy link
Member

dbu commented May 18, 2012

oh hell, i must have been tired. i thought you are relaxing the expression (i confused old and new somehow).

this is definitely much better than what we had until now. we will have to think about name collisions more when we implement getRouteCollection. then we might change this one to make the naming consistent.

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.

2 participants