Decouple I18n\View\Helper\AbstractTranslatorHelper from ext\intl #4380

Closed
wants to merge 1 commit into
from

4 participants

@Slamdunk

https://github.com/zendframework/zf2/blob/release-2.2.0rc1/library/Zend/I18n/View/Helper/AbstractTranslatorHelper.php#L34-L45

is too wide and involves also

https://github.com/zendframework/zf2/blob/release-2.2.0rc1/library/Zend/Form/View/Helper/AbstractHelper.php#L13

which has nothing to do with ext\intl apart for FormMonthSelect and its descendants

Without this fix, no Form\View\Helper can be used if ext\intl is missing.

I had to put some attribute initialization into constructors to avoid unmanagable fatal errors.

@prolic

+1

@mwillbanks mwillbanks commented on the diff May 2, 2013
library/Zend/Form/View/Helper/FormDateTimeSelect.php
@@ -23,7 +23,17 @@ class FormDateTimeSelect extends FormDateSelectHelper
*
* @var int
*/
- protected $timeType = IntlDateFormatter::LONG;
+ protected $timeType;
+
+ /**
+ * @throws Exception\ExtensionsNotLoadedException if ext/intl is not present
+ */
+ public function __construct()
+ {
+ parent::__construct();
+
+ $this->timeType = IntlDateFormatter::LONG;
@mwillbanks
mwillbanks added a line comment May 2, 2013

You are missing the exception in this area.

@weierophinney
Zend Framework member
weierophinney added a line comment May 2, 2013

@mwillbanks It's thrown by the parent constructor.

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

Please also supply unit tests to verify the behavior.

@Slamdunk

@mwillbanks there is no test written for other extension checking like

https://github.com/zendframework/zf2/blob/release-2.2.0rc1/library/Zend/I18n/Filter/AbstractLocale.php#L21-L29

and many others.

Btw, I'll be happy to create tests if anyone can make a Travis build without ext\intl loaded.

@weierophinney weierophinney commented on the diff May 2, 2013
library/Zend/I18n/Validator/DateTime.php
@@ -12,6 +12,7 @@
use Locale;
use IntlDateFormatter;
use Traversable;
+use Zend\I18n\Exception as I18nException;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff May 2, 2013
library/Zend/I18n/Validator/DateTime.php
*/
public function __construct($options = array())
{
+ if (!extension_loaded('intl')) {
+ throw new I18nException\ExtensionNotLoadedException(sprintf(
@weierophinney
Zend Framework member
weierophinney added a line comment May 2, 2013

Mismatch with the docblock; as suggested above, don't alias to I18nException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff May 2, 2013
library/Zend/Form/View/Helper/FormDateTimeSelect.php
@@ -23,7 +23,17 @@ class FormDateTimeSelect extends FormDateSelectHelper
*
* @var int
*/
- protected $timeType = IntlDateFormatter::LONG;
+ protected $timeType;
+
+ /**
+ * @throws Exception\ExtensionsNotLoadedException if ext/intl is not present
@weierophinney
Zend Framework member
weierophinney added a line comment May 2, 2013

s/Extensions/Extension/ (need to do this in all typehints)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney was assigned May 2, 2013
@weierophinney weierophinney added a commit that referenced this pull request May 2, 2013
@weierophinney weierophinney [#4380] CS fixes
- s/Extensions/Extension/g
- Added explanatory comments
- Alias both exception types in DateTime validator
f4a0131
@weierophinney weierophinney added a commit that closed this pull request May 2, 2013
@weierophinney weierophinney Merge branch 'hotfix/4380'
Close #4380
f1a6877
@weierophinney weierophinney added a commit that referenced this pull request May 2, 2013
@weierophinney weierophinney Merge branch 'hotfix/4380' into develop
Forward port #4380
abf9f74
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney [#4380] CS fixes
- s/Extensions/Extension/g
- Added explanatory comments
- Alias both exception types in DateTime validator
fd23bc5
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/4380'
Close #4380
16421e5
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/4380' into develop
Forward port #4380
fafc484
@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#4380 from Slamdunk/hot…
…fix/intl-optional

Decouple I18n\View\Helper\AbstractTranslatorHelper from ext\intl
fe2f165
@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#4380] CS fixes
- s/Extensions/Extension/g
- Added explanatory comments
- Alias both exception types in DateTime validator
a2203c3
@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4380' d340adb
@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4380' into develop ad757bb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment