-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Issue #4443 - Zend\I18n\Translator\Loader\PhpArray can now load files from include path #4515
Conversation
@@ -29,7 +29,8 @@ class PhpArray implements FileLoaderInterface | |||
*/ | |||
public function load($locale, $filename) | |||
{ | |||
if (!is_file($filename) || !is_readable($filename)) { | |||
if (!stream_resolve_include_path($filename) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please hold the result of stream_resolve_include_path
within an own variable and (if not false
) use this variable further checks and for include
itself
@marc-mabe Done. Thanks for the feedback. |
@@ -29,14 +29,15 @@ class PhpArray implements FileLoaderInterface | |||
*/ | |||
public function load($locale, $filename) | |||
{ | |||
if (!is_file($filename) || !is_readable($filename)) { | |||
$fromIncludePath = stream_resolve_include_path($filename); | |||
if (!$fromIncludePath && (!is_file($filename) || !is_readable($filename))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream_resolve_include_path()
simply resolves to first found file using the configured include_path
. If no relative path will be given this will be used without include_path
. It doesn't check the type (see stream_resolve_include_path('/tmp')
) or readability (see stream_resolve_include_path('/root')
)
So please use $fromIncludePath
:
if (!$fromIncludePath || !is_file($fromIncludePath) || !is_readable($fromIncludePath)) {
throw new Exception\InvalidArgumentException(sprintf(
'Could not find or open file %s for reading',
$filename
));
}
$messages = include $fromIncludePath;
One question: what exactly is the use case for this? |
The use case is, when you can't use absolute or relative path in the module config file to load (Zend) PHP translations, e.g. your module is located somewhere outside of the application but wants to load translations from other libraries in the include path. And the exception lied, because the file can be opened for reading. |
This looks kinda inconsistent to me, as the other loaders are not able to look in the same paths. |
Should I add the same feature for the other loaders? |
@robertboloc Yes, please. |
Done |
The last issue is, that now it can't load from PHAR, because stream_resolve_include_path() returns false when $filename starts with phar:// |
Updated the code to allow loading from phar. |
Issue #4443 - Zend\I18n\Translator\Loader\PhpArray can now load files from include path
Merged to develop for release with 2.3.0. |
@weierophinney can this be reverted? We don't use the include path at all in ZF2, we shouldn't start now (nor should modify the environment, which is a thing we're all proud of) |
@Ocramius The developer of this and the config one made some good arguments for having the capabilities, which largely center around non-code-related file resolution. While I agree that one should stay away from the include_path, the facilities only provide overhead in specific situations, and are basically opt-in. |
…endframework/zendframework#4443 Issue zendframework/zendframework#4443 - Zend\I18n\Translator\Loader\PhpArray can now load files from include path
Added possibility to load translation files from the include path.
Issue #4443