Better fix for #4284 #4293
Better fix for #4284 #4293
Conversation
- Marking as required in the framework makes it impossible to install the full ZF2 distribution when ext/intl is not available. - Based on discussion in zendframework#4284, both zf2 and zend-i18n composer.json files should be in sync; otherwise, full zf2 will be installed if ext/intl is not available when attempting to install zend-i18n. - Updated all code to raise a new "ExtensionNotLoadedException" when ext/intl is required but not detected.
|
Build failure is due to a Travis issue on 5.4 (unable to download php-cs-fixer, or unable to coerce path to php-cs-fixer). |
|
|
||
| use DomainException; | ||
|
|
||
| class ExtensionNotLoadedException extends DomainException implements |
There was a problem hiding this comment.
Please extend from Zend\I18n\Exception\DomainException, if it does not exist, create it.
There was a problem hiding this comment.
There's no reason to do this. The only requirement we've explicitly stated is that exceptions should extend SPL exceptions, not that the inheritance has to be SPL -> namespaced exception of same name -> more specific version. In terms of inheritance trees, it's simpler and cleaner to define the exceptions as extending simply an SPL exception and implementing the component-specific marker exception.
There was a problem hiding this comment.
Well, we've done this through the framework.
There was a problem hiding this comment.
Actually, not everywhere. It's typically been up to the component author;
I've been steering to simple extension.
On Monday, April 22, 2013, prolic wrote:
In library/Zend/I18n/Exception/ExtensionNotLoadedException.php:
@@ -0,0 +1,16 @@
+<?php
+/**
- * Zend Framework (http://framework.zend.com/)
- * @link http://github.com/zendframework/zf2 for the canonical source repository
- * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
- * @license http://framework.zend.com/license/new-bsd New BSD License
- */
+
+namespace Zend\I18n\Exception;
+
+use DomainException;
+
+class ExtensionNotLoadedException extends DomainException implementsWell, we've done this through the framework.
—
Reply to this email directly or view it on GitHubhttps://github.com//pull/4293/files#r3905911
.
Matthew Weier O'Phinney
matthew@weierophinney.net
http://mwop.net/
- `s/InvalidArgument/ExtensionNotLoaded/` in `@throws` annotations
| * @throws Exception\InvalidArgumentException | ||
| */ | ||
| public static function factory($options) | ||
| { | ||
| if (!extension_loaded('intl')) { |
There was a problem hiding this comment.
This should be in the constructor – create it.
| __NAMESPACE__ | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
The translator requires the intl extension only to get a default locale if none was set Locale::getDefault().
It would be better to check for class exists and throw the exception only at this point a default locale couldn't detected or simply hard define one default locale.
- instead of __construct, as ext/intl is only needed if no locale is explicitly set.
Forward port #4293 Conflicts: library/Zend/I18n/View/Helper/CurrencyFormat.php library/Zend/I18n/View/Helper/DateFormat.php library/Zend/I18n/View/Helper/NumberFormat.php library/Zend/I18n/View/Helper/Plural.php library/Zend/I18n/composer.json
Forward port zendframework/zendframework#4293 Conflicts: library/Zend/I18n/View/Helper/CurrencyFormat.php library/Zend/I18n/View/Helper/DateFormat.php library/Zend/I18n/View/Helper/NumberFormat.php library/Zend/I18n/View/Helper/Plural.php library/Zend/I18n/composer.json
This commit provides a better fix for #4284 by making
ext/intlan optional dependency, but enforcing its requirement via tests toextension_loadedand raising anExtensionNotLoadedExceptionwhen not found.