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

Translable routing segments #4050

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
8 participants
@DASPRiD
Member

DASPRiD commented Mar 16, 2013

Preamble
This is the initial prototype for the translatable segments. It is considered work in progress, although it should be fully functional. Eventually, only the unit tests are missing.

I already implemented the parsing of translatable segments very early in the beginning, but totally forgot about them until now. This evening I was finally able to make up a good way to implement them.

Introduction
So, first of, I completely killed translatable parameters, and only left translatable literals in. This has the reason, that translatable parameters would eventually only serve a very small set of possible values, which can eventually be represented with child-routes and translatable literals. This also makes the implementation a lot easier.

To not force an already configured translator when the router itself is fetched from the service manager the very first time, I decided to move the translation logic completely into the match() and assemble() methods. This also allows to re-use the matching methods with different locales (there may be some use-cases for that, e.g. testing URLs in a CMS).

New soft dependency
Zend\I18n is now a soft dependency of Zend\Mvc and is added as "suggests" in the composer.json file. To avoid a hard dependency, a new route stack with the name Zend\Mvc\Router\Http\TranslatorAwareTreeRouteStack was implemented. This one has to be used in case you want to use translatable segments. It just extends the normal tree route stack, so it has the same functionality.

The segment route itself is not a problem for this, as it only has a use Zend\I18n\Translator\Translator in its head, but only actually uses that class if translatable segments exist.

Usage
Eventually, I introduced a few new features here and there, to make the implementation as sane as possible. No BC breaks were done for it, at least as far as I know. All existing unit tests pass (except for the ones which tested for the "not implemented" exceptions, but those were already removed by the initial comit).

To make use of the translatable segments, you first have to inject a translator into the router. This can be done either automatically by the ServiceManager (thanks to the TranslatorAwareInterface) or manually through $router->setTranslator().

Optionally you can specify a text domain to use for route translations with $router->setTranslatorTextDomain(). If you don't set one, the "default" text domain will be used.

Now you are ready to create translatable segments. Take for instance the following route definition, where the translatable segment is encapsulated in curly brackets:

/{bacon}/:id

When matching, the bacon parameter will be replaced with the translation of the message id "bacon" in the matching regex and then the regex will be evaluated against the request as usual. When assembling, the same thing happens again. This is a much more efficient approach than the one we had in ZF1, where the route had to get all translations from the translator.

You also have the possibility to change the translator, the text domain and the locale on-the-fly both for matching and for assembling. For the match() method, a third (non-interface) array $options = array() parameter was added after the $pathOffset parameter. This allows to pass down "translator", "text_domain" and "locale" down the tree. The same options are also taken into account by the assemble() method.

Sidenote
I also fixed a few formatting issues and changed some docblocks, nothing to worry about :)

@@ -204,7 +202,7 @@ protected function buildRegex(array $parts, array $constraints, &$groupIndex = 1
*
* @param array $parts
* @param array $mergedParams
* @param bool $isOptional
* @param boolean $isOptional

This comment has been minimized.

@iquabius

iquabius Mar 17, 2013

Contributor

What is the right way to refer to the boolean type in the phpDocs? boolean or bool?
I see a lot of bools in doc blocks of zf2 code, and this made me curious.

This comment has been minimized.

@weierophinney

weierophinney Mar 17, 2013

Member

bool is what we have standardized on; we use the short form of a type
usually.

On Saturday, March 16, 2013, Josias Duarte Busiquia wrote:

In library/Zend/Mvc/Router/Http/Hostname.php:

@@ -204,7 +202,7 @@ protected function buildRegex(array $parts, array $constraints, &$groupIndex = 1
*
* @param array $parts
* @param array $mergedParams

  • \* @param  bool $isOptional
    
  • \* @param  boolean $isOptional
    

What is the right way to refer to the boolean type in the phpDocs? booleanor
bool?
I see a lot of bools in doc blocks of zf2 code, and this made me curious.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4050/files#r3404753
.

Matthew Weier O'Phinney
matthew@weierophinney.net
http://mwop.net/

This comment has been minimized.

@DASPRiD

DASPRiD Mar 17, 2013

Member

Interesting, I kinda missed that memo ;)

@marc-mabe

This comment has been minimized.

Member

marc-mabe commented Mar 18, 2013

Than Zend\I18n is a requirement of Zend\Mvc? Shouldn't that be optional?

@DASPRiD

This comment has been minimized.

Member

DASPRiD commented Mar 21, 2013

Hard dependency removed and PR description updated.

@weierophinney

This comment has been minimized.

Member

weierophinney commented Mar 28, 2013

@DASPRiD This is looking good. I'll review again when you have tests. :)

@weierophinney

This comment has been minimized.

Member

weierophinney commented Apr 3, 2013

@DASPRiD bump... about those tests...

@roelvanduijnhoven

This comment has been minimized.

Contributor

roelvanduijnhoven commented Apr 17, 2013

Looking good 👍, will definitely use this for my project

@weierophinney

This comment has been minimized.

Member

weierophinney commented Apr 21, 2013

@DASPRiD Any chance you'll have tests ready by end-of-week so we can include in 2.2.0?

@DASPRiD

This comment has been minimized.

Member

DASPRiD commented Apr 21, 2013

@weierophinney I'll have that on my todo list, yes.

@DASPRiD

This comment has been minimized.

Member

DASPRiD commented Apr 28, 2013

Unit tests ready and feature ready to merge.

@ghost ghost assigned weierophinney Apr 29, 2013

weierophinney added a commit that referenced this pull request Apr 29, 2013

Merge pull request #4050 from DASPRiD/feature/translatable-segments
Translable routing segments

Conflicts:
	library/Zend/Mvc/Router/Http/Part.php
	library/Zend/Mvc/Router/Http/TreeRouteStack.php
	library/Zend/Mvc/Router/SimpleRouteStack.php

weierophinney added a commit that referenced this pull request Apr 29, 2013

[#4050] CS fixes
- per php-cs-fixer
- s/boolean/bool/g

weierophinney added a commit that referenced this pull request Apr 29, 2013

@weierophinney

This comment has been minimized.

Member

weierophinney commented Apr 29, 2013

Merged to develop for release in 2.2.0.

@pfx2

This comment has been minimized.

pfx2 commented May 16, 2013

Hello,

Thanks for this feature, very usefull.

Quick question: I currently instanciate manually a TranslatorAwareTreeRouteStack object. But you say that this can be done automatically by the ServiceManager.
I don't understand that, since TreeRouteStack is hardcoded in Zend\Mvc\Service\RouterFactory

So is it possible? Could you provide an example in the documentation?

@chozhiyath

This comment has been minimized.

I use inbuilt PHP Server for my ZF2 app (php -S 0.0.0.0:80). But when I use Hostname router, the uri returns the SERVER_NAME where as it is just 0.0.0.0 and hence does not match any of my routes! As a temporary hack I wrote

if (php_sapi_name() == 'cli-server') {
$_SERVER['SERVER_NAME'] = $_SERVER['HTTP_HOST'];
}

to fix the issue. Any thoughts?

@tylkomat

This comment has been minimized.

tylkomat commented on library/Zend/Mvc/Router/Http/Part.php in c71099e Mar 3, 2014

You should agree on a common specification of what it should be bool or boolean @weierophinney changed everything in the other direction in another commit.

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