[Dotenv] Throwing an error when loading nothing #22364
Conversation
| @@ -47,7 +47,13 @@ | |||
| public function load(/*...$paths*/) | |||
There was a problem hiding this comment.
if we want to enforce one arg, what about doing it here load($path/*, ...$paths*/)?
There was a problem hiding this comment.
@nicolas-grekas suggestion looks better indeed.
There was a problem hiding this comment.
if we want to enforce one arg, what about doing it here
load($path/*, ...$paths*/)?
For me, this introduces two bad things:
- add an
array_unshiftcall; - pollute the method signature.
There was a problem hiding this comment.
Well, it does not really pollute the method signature if we really want to require at least 1 arg. In PHP 5.6+, public function load(...$paths) means 0+ arguments, not 1+.
There was a problem hiding this comment.
and in the current code, this does not introduce any array_unshift. $paths = func_get_args(); will still give us all arguments
There was a problem hiding this comment.
@stof ok, good points.
| @@ -39,15 +39,18 @@ | |||
| /** | |||
| * Loads one or several .env files. | |||
| * | |||
| * @param ...string A list of files to load | |||
| * @param $path A file to load | |||
| * @param ...string A list of additionnal files to load | |||
There was a problem hiding this comment.
Should be string[] instead of ...string
There was a problem hiding this comment.
not true. string[] means the outside is passing an array. Here, the signature is not an array but a variadic arg
| @@ -39,15 +39,18 @@ | |||
| /** | |||
| * Loads one or several .env files. | |||
| * | |||
| * @param ...string A list of files to load | |||
| * @param $path A file to load | |||
| { | ||
| // func_get_args() to be replaced by a variadic argument for Symfony 4.0 | ||
| foreach (func_get_args() as $path) { | ||
| $paths = func_get_args(); |
There was a problem hiding this comment.
Why making this change? Using func_get_args() below is still fine to me.
There was a problem hiding this comment.
Ah yes, it’s a relic from my first patch.
| @@ -39,12 +39,13 @@ | |||
| /** | |||
| * Loads one or several .env files. | |||
| * | |||
| * @param ...string A list of files to load | |||
| * @param string $path A file to load | |||
| * @param string[] $paths A list of additionnal files to load | |||
There was a problem hiding this comment.
@nicolas-grekas says the opposite #22364 (review)
There was a problem hiding this comment.
I see... we dont allow for arrays right? So im not sure about []
There was a problem hiding this comment.
looking at eg http://stackoverflow.com/questions/14513356/phpdoc-documenting-a-function-with-a-variable-number-of-arguments
...string should be way to go, sorry for the bad comment
|
Thank you @sanpii. |
This PR was merged into the 3.3-dev branch. Discussion ---------- [Dotenv] Throwing an error when loading nothing | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a This is a quick fix to prevent anoying when you follow a tutorial using another dotenv component. For example, this code uses [vlucas/phpdotenv](https://packagist.org/packages/vlucas/phpdotenv): ```php $dotenv = new Dotenv\Dotenv(__DIR__.'/../'); $dotenv->load(); ``` Silently failed with symfony/dotenv. Commits ------- 911bc68 [Dotenv] Throwing an error when loading nothing
This is a quick fix to prevent anoying when you follow a tutorial using another dotenv component. For example, this code uses vlucas/phpdotenv:
Silently failed with symfony/dotenv.