config

config copied to clipboard

Just creating this issue based on PR 43 :smile:

I'm not sure what the benefit is to being able to return a callable when including the PHP config. We would be able to do any logic within the file before returning an array.

Personally the only time I can see this being valid is if we was going to pass a param in when invoking.

Obviously changing this would break BC so not a major issue with keeping it in, was just wondering what the use case would be.

I guess a use-case would be if you wanted to determine certain config parameters ahead-of-time, say you want to check for an extension, and then set a config value to true or false.

What kind of parameter would you pass into it?

I'm not sure what you could pass through, as I'm not actually sure what's available at that point. But looking at it, there doesn't seem to be any benefit as for example ...

<?php 

// do some logic
if ($something) {
    $value = 'x';
else {
    $value = 'y';
}

return [
    'conditional_config' => $value
];

or

<?php 

return function () {
    // do some logic
    if ($something) {
        $value = 'x';
    else {
        $value = 'y';
    }

    return [
        'conditional_config' => $value
    ];
};

I'm pretty sure these would have the same outcome.

maybe if it was more like this I would see a benefit, but I'm not entirely sure how this would be implemented or what benefit it would have.

<?php 

return function ($currentConfig) {
    // do some logic
    if ($currentConfig->get('something')) {
        $value = 'x';
    else {
        $value = 'y';
    }

    return [
        'conditional_config' => $value
    ];
};

Hmm, actually you do make a very good point. I'm not sure how many people use callable functions as their config packages, I've personally never used them. What would you recommend we should do?

Regardless if many people are using it, it is still a BC break so would need a major version bump if removed. Looking at Packagist there's currently been 6,000+ installs any of which could be dependant on this functionality.

I'd probably say create a 1.0.0 milestone and note it for removal or potentially look into whether there can be any data passed into the callable that would be beneficial. If I get some time later I'll take a look too.

Yep, sounds fair. I do have an idea for a potential use-case: say I'm loading multiple files, perhaps I want to check the current config before I set any values. In that case, the current config could be passed over to the new config callable. Does that sound acceptable?

Yup that sounds like a fair use case, although at a glance I'm not sure how easy it would be to implement.

Maybe at this point passing $this->data into the parse method, making it an optional param in the interface and then the parsers can choose whether to make use of it etc etc. Then this can be used in the PHP parser to pass through to the callable.

Yep, that's exactly what I was thinking

Looked into this a bit more. Passing it through as mentioned works fine. But the more I look at it I question the use case. Config is ideally static data so doing conditional logic based on currently loaded data seems out of scope. Let me know what you think.

To be completely honest, I agree, it is kind of out-of-scope. I'd leave it as-is for now, and we can revisit it for a potential 1.x release at some point

You can do this with https://github.com/samrap/gestalt by defining custom loaders. Closure and class based loaders are supported:

$config = Configuration::load(function() {
    $items = parse_ini_file('config/app.ini', true);

    return $items;
});

$config = Configuration::load(new IniDirectoryLoader);