search

basz / SlmLocale

Auto detection of locale through different strategies for Zend Framework 2
Other
67 stars 34 forks source link

Add support for setting cookie names in cookie strategy #56

Closed hikaru-shindo closed 10 years ago

hikaru-shindo commented 10 years ago

At the moment it is not possible to change the name of the locale cookie. I think some people would like to change the cookie name.

juriansluiman commented 10 years ago

Sounds as a sane addition. You don't use the setter in getOptions, could you update that? Furthermore, I've a few additions:

  1. Can we extract a variable $cookie instead of calling the getter everytime?
  2. It would be nice to have a cookie name validation inside the setter. Some characters aren't allowed and we could check for those
  3. Tests are missing, can you add a test for this feature?

Anyhow, thanks for the idea and PR!

atukai commented 10 years ago

Maybe rename $cookie to $cookieName and option to cookie_name?

hikaru-shindo commented 10 years ago

I think the validation of the cookie name is not necessary. We are all adults here ;) But I added a basic validation nevertheless. I also changed variable and option names because I think atukais suggestion is more semantic.

juriansluiman commented 10 years ago

@hikaru-shindo it's rather that with the ZF2 config, it's easy to somewhere have an empty variable set as default which is never overwritten. Thus, a check if you have a sane cookie name might seem unnecessary, it helps you with debugging for those kinds of problems.

Anyhow, looks good. I'm gonna merge :)