[WIP] Refactoring #6

Closed
wants to merge 10 commits into
from

Conversation

Projects
None yet
5 participants
Collaborator

ikwattro commented Apr 17, 2012

This is the base refactoring as last discussed, all new commits will be done against the refactoring branch with PR's to I'll let one day open before merging to let you comment on it.

ikwattro added some commits Apr 11, 2012

@ikwattro ikwattro test for bundle
added a detection entry in the configuration class, need to be ref with
addXXXsection

first part of refactoring, new LocaleDetectorInterface, Detection
Priority, ...

added the possible service params in the entryPoint construct

added more logic to the event listener...wip

phpunit was not in root dir

removed a comma in constructor params

added routerLocaleDetector and testClass for it

removed useless check of the router context as it is done in FW
LocaleListener
be59d51
@ikwattro ikwattro add a bindParameters function to allow config params to be automically
registered
07724be
@ikwattro ikwattro added some comments into the listener and the routerDetector bdd2362
@ikwattro ikwattro moved the lunetics logic to the BrowserLocaleDetector class 679113b
@ikwattro ikwattro the onKernelResponse sets now the locale cookie 85f6797
@ikwattro ikwattro typo fix + some new notes in README ab032b7
@ikwattro ikwattro added some tests and adjusted code iaw test errors 5c247f1
@ikwattro ikwattro made some refactoring to the browserLocaleDetector to not set default
locale if none locale found from browser
5942abe
@ikwattro ikwattro some changes 40c74d0
@ikwattro ikwattro switch listener - to be removed in privilege of links generator f4fc851
Contributor

gimler commented Apr 17, 2012

i think this is the correct on /^[\w]{2}([-_][\w]{2})?$/

Collaborator

dbu commented Apr 18, 2012

looking at the java locale http://docs.oracle.com/javase/7/docs/api/java/util/Locale.html and at the iana list of languages http://www.iana.org/assignments/language-subtag-registry (linked as the list of possible languages from wikipedia http://en.wikipedia.org/wiki/Locale i think that the language can also be 3 or more (java says {2,8}) while country is strictly limited to 2. (but could be expressed with a 3 digits code as well - no idea if this is ever used anywhere)

Where is the FilterLocaleSwitchEvent class?

Collaborator

ikwattro commented May 3, 2012

There is momently none because the switching behavior will take a complete different behavior as initially expected (ref. closed PR's). I'll get my internet landline back on monday, i'll notify here when the bundle is updated.

@kwattro any news? :)

Collaborator

ikwattro commented May 7, 2012

I have posted a problem in the cmf mailing list.
Le 7 mai 2012 15:14, "William Durand" <
reply@reply.github.com>
a écrit :

@kwattro any news :)


Reply to this email directly or view it on GitHub:
#6 (comment)

@kwattro I'm not part of the CMF organization, and this bundle is not a CMF one. Could you expose the issue in a ticket in this repository?

Collaborator

ikwattro commented May 7, 2012

Yes no problem. I'm now replying with my smartphone. I ll be at home within
2 hours and do it then. It will be easier.
Le 7 mai 2012 15:20, "William Durand" <
reply@reply.github.com>
a écrit :

@kwattro I'm not part of the CMF organization, and this bundle is not a
CMF one. Could you expose the issue in a ticket in this repository?


Reply to this email directly or view it on GitHub:
#6 (comment)

Collaborator

ikwattro commented May 7, 2012

Ok I expose the problem. The goal for the rendering of the switcher was to allow people to override the template and providing two defaults behaviors :

  1. A form for switching the locale
  2. A list of links for the different locales.

For the number there is no problem.

For the number 2, If we render the controller action, the route that will be handled is an internal route. So I was asking if we can have access to the request that call this internal request ? I can still pass the request object through the render function but it's a bit ..ugly.

Collaborator

dbu commented May 10, 2012

i think the request issue is by design. the reason being that if the subrequest is handled i.e. through ESI, the main request is not available at all. i think you need to pass the parameter, but i just tweeted to ask for input. lets see if somebody can tell for sure.

Collaborator

lsmith77 commented May 10, 2012

yes .. all parameters need to be manually passed down to sub requests if they are needed there too. that being said .. one should take to really only pass the necessary parameters so that you don't end up needlessly caching duplicate output.

Collaborator

ikwattro commented May 10, 2012

After some search, I've tested quickly on doing it with a twig extension rather than with the render function.

I've take as example the work done in the FOSFacebookBundle and it's working. I need to arrange it to match with the current implementation of the bundle. I have momently momently less time due to company prios but I'll try to have a proof for monday.

Collaborator

ikwattro commented May 10, 2012

@lsmith77 didn't see your comment. What do you suggest, pass the params with the render function or doing it with a twig extension ?

Collaborator

ikwattro commented Jun 9, 2012

Closing in adv for "rework" branch

ikwattro closed this Jun 9, 2012

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