Any way to set config items in a section? #1327

Closed
michaelbrooks opened this Issue May 4, 2012 · 20 comments

Projects

None yet

10 participants

@michaelbrooks

You can load config variables into sections with $this->config->load('some_config', TRUE), and you can get these config variables using $this->config->item('variable', 'some_config'), but I can't find a way to set them manually using $this->config->set_item(), for example.

Contributor

Sorry @michaelbrooks but I don't quite understand what you're asking here, could you please clarify

Sure. In the simple case, configs are loaded and accessed:

$this->config->load('some_config');
$this->config->item('variable');

Sometimes it might be useful to manually override a config variable, and there is a facility for that:

$this->config->set_item('variable', 'value');

To avoid conflicts between config files, I am using this form of the load function:

$this->config->load('some_config', TRUE);

This loads the variables in config/some_config.php into an array named 'some_config' within the Config class, preventing conflicts. These items must be accessed using the array name:

$this->config->item('variable', 'some_config');

However, to my knowledge there exists no version of the set_item function that allows manually overriding config settings that were loaded in this manner. For my own application I have resorted to setting the contents of the Config class's variable array myself:

$this->config->config['some_section']['variable'] = 'value';

But this would provide a more consistent interface:

$this->config->set_value('variable', 'value', 'some_config');

I think this would be a pretty small addition, so I'd be happy to submit a pull request myself if you think this is a useful fix.

Contributor
narfbg commented May 18, 2012

Some references, just to keep track of this stuff: #180, #328, #708, #1058

... and since there are 4 issues referenced - obviously, this should be considered to be a bug. I'll think about a fix tomorrow/during daylight time (3AM here).

Guess it might be more complicated than I thought.

About #1058, I did not mean to suggest that it should rewrite the config file. I'm just assuming that if there was a good reason for set_item to exist, it should work for config items in subsections as well as "global" config items.

Contributor
narfbg commented May 18, 2012

OK, after looking more carefully at this - it's a bit different than #180 and #708. But the good news is that it's probably easier to fix. :)

As I see it, set_item() should be changed so that it can accept an array as it's first parameter. That array should have two elements - one for the config section and one for the actual (sub)item to be set.
The question is - how should those keys be named (unless numeric indexes are used) ? I'd suggest that both 'section' and 'config' are accepted for the "parent" and the "child" should be named 'item'. Any objections?

For providing an interface that is consistent with item(), I think it makes more sense for the section to be a 3rd parameter rather than bundled in an array. Unless you want to change item() to accept an array as well.

As you said, using an array raises a couple of issues, like numeric vs. associative. A downside is that both are more verbose than just using a 3rd parameter. Numeric arrays would be more concise. On the other hand, an associative array automatically provides an obvious constraint on how many elements are to be passed in (a 'section' and a 'item' or whatever).

Overall, I prefer not using an array at all. The main reason is that (unless I'm mistaken) there is no intention of allowing more than just the 'section' and 'item' arguments at any point in the future. Using an array would make a lot of sense if the interface might accept additional parameters later, but I don't think it makes sense in this case.

Contributor
narfbg commented May 19, 2012

My reasoning behind using an array is that the order of arguments doesn't wouldn't make much sense if a third parameter is added. And yes - I want both strings and arrays to be accepted as the first parameter.

I agree that the order wouldn't make much sense. But then the order doesn't make much sense with $this->config->item('variable', 'section') either. If both item and set_item accepted an array or a single item string, that would probably be best.

Contributor
narfbg commented May 24, 2012

It's one of those errors by design that we need to work around. And if we add it as a third parameter - we're making it almost impossible to have any consistency with item() in the future. On the other hand - making item() to accept an array as well is easy.

Contributor

Another option:
$this->config->item('section.variable')
$this->config->item('variable') // To be valid too, as it is now.
?

Contributor
ckdarby commented Jan 3, 2013

@narfbg Where does this stand?

marcvdm commented Jun 1, 2013

As @ckdarby asked 5 months ago, where does this stand?

Contributor
cryode commented Jun 24, 2013

If #2391 is implemented, you can adjust config items using standard array notation. That would be one solution, on top of trying to find something method-based if necessary.

Contributor
vlakoff commented Aug 7, 2013

Your opinion on ivantcholakov's proposal? Would you agree for "dot notation" to be implemented? That's a feature already used by Laravel, and I really like it. @narfbg

Contributor
narfbg commented Aug 7, 2013

No, I'm not a proponent of using <pick a character>-separated strings in configuration.

Contributor
vlakoff commented Aug 7, 2013
config->item('item', 'section');
config->set_item(array('section'=>'section', 'item'=>'item'), 'value');

versus:

config->item('section.item');
config->set_item('section.item', 'value');
Contributor

I do this in my app (file application/core/MY_Config.php)

<?php  if ( ! defined('BASEPATH')) exit('No direct script access allowed');

class MY_Config extends CI_Config {

    function set_config($key, $value, $index = FALSE)
    {
        if (empty($index) OR ! is_string($index))
        {
            parent::set_config($key, $value);
        }

        if (isset($this->config[$index]))
        {
            $this->config[$index][$key] = $value;
        }
    }

}

/* End of file MY_Config.php */
/* Location: ./application/core/MY_Config.php */

Huh. Has this been fixed? It sounds like it has been.

Contributor
ckdarby commented Mar 3, 2014

@sarciszewski Please stop snipping through the issues asking if they've been fixed. I'm receiving the exact same comment on multiple issues from you.

Contributor
narfbg commented Mar 4, 2014

It hasn't been "fixed", but now that you're bringing it up again ... there's no way to achieve that in a clean/appropriate way, so I'm closing it.

@narfbg narfbg closed this Mar 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment