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

unset _locale parameter if matching route has been found #56

Merged
merged 1 commit into from Apr 5, 2013

Conversation

uwej711
Copy link
Member

@uwej711 uwej711 commented Apr 4, 2013

See issue #55

dbu added a commit that referenced this pull request Apr 5, 2013
unset _locale parameter if matching route has been found
@dbu dbu merged commit 0169143 into symfony-cmf:master Apr 5, 2013
@dbu
Copy link
Member

dbu commented Apr 5, 2013

thanks

@dbu
Copy link
Member

dbu commented Apr 5, 2013

hm. what if there is {_locale} in the route? should we check that before unsetting?

@uwej711
Copy link
Member Author

uwej711 commented Apr 5, 2013

Hi,

What about variables in the route patterns at all?

Cheers
Uwe

Am 05.04.2013 um 11:06 schrieb David Buchmann notifications@github.com:

hm. what if there is {_locale} in the route? should we check that before unsetting?


Reply to this email directly or view it on GitHub.

@lsmith77
Copy link
Member

lsmith77 commented Apr 5, 2013

we do have the locale in the route when using the SimpleCmsBundle in the multilang mode (like we do in the SE).

@uwej711
Copy link
Member Author

uwej711 commented Apr 5, 2013

So you create a node with name {_locale} at the top of the routing tree? If
that's possible this PR is wrong and should be reverted.

2013/4/5 Lukas Kahwe Smith notifications@github.com

we do have the locale in the route when using the SimpleCmsBundle in the
multilang mode (like we do in the SE).


Reply to this email directly or view it on GitHubhttps://github.com//pull/56#issuecomment-15946000
.

@dbu
Copy link
Member

dbu commented Apr 5, 2013

I fear we have to string-search for that in the pattern. Or would getParameters work here to check?

----- Reply message -----
Von: "Uwe Jäger" notifications@github.com
An: "symfony-cmf/Routing" Routing@noreply.github.com
Cc: "David Buchmann" david@liip.ch
Betreff: [Routing] unset _locale parameter if matching route has been found (#56)
Datum: Fr., Apr. 5, 2013 11:47
So you create a node with name {_locale} at the top of the routing tree? If

that's possible this PR is wrong and should be reverted.

2013/4/5 Lukas Kahwe Smith notifications@github.com

we do have the locale in the route when using the SimpleCmsBundle in the

multilang mode (like we do in the SE).

Reply to this email directly or view it on GitHubhttps://github.com//pull/56#issuecomment-15946000

.


Reply to this email directly or view it on GitHub.

@lsmith77
Copy link
Member

lsmith77 commented Apr 5, 2013

no we do this dynamically in the route provider. but yes in the end there is a _locale placeholder. check the SE

@uwej711
Copy link
Member Author

uwej711 commented Apr 5, 2013

You do it in SimpleCmsBundle\Document\MultilangRoute.php

This could be part of the RoutingExtraBundle ?

@lsmith77
Copy link
Member

lsmith77 commented Apr 5, 2013

sorry .. i was unclear before .. its done inside the Route itself:
https://github.com/symfony-cmf/SimpleCmsBundle/blob/master/Document/MultilangPage.php#L71

@uwej711
Copy link
Member Author

uwej711 commented Apr 5, 2013

So I propose the following: we compile the route already in ContentAwareGenerator and check the variables for _locale. If it is not there, it can be safely unset. I'll try to do a PR for that.

@dbu
Copy link
Member

dbu commented Apr 6, 2013

that sounds like an excellent idea. it does not cost us anything as the
compiled route is cached when unchanged. and we won't change it at that
point. thanks uwe.

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