Respect / Config

A powerful, small, deadly simple configurator and dependency injection container DSL made to be easy
http://respect.github.io/Config
Other
98 stars 7 forks source link

test for issue 30 and an attempted solution by @alganet #31

Closed iannsp closed 12 years ago

iannsp commented 12 years ago

test with the suggestions by @alganet.

seems it did not work... panda cry like a baby panda without bamboo. ;)

iannsp commented 12 years ago

@alganet do the merge before I do the PR. ;)

alganet commented 12 years ago

Sorry! I just checked it out and realized that I've used "+" instead of array_merge. I've fixed it and the test now works!

iannsp commented 12 years ago

I do the sync and yes. U are right!

nickl- commented 12 years ago

I would have gone for something more like this, or similar:

<?php

    public function testEnviromentConfiguration30()
    {
        $config = "
[development]
user = alganet

[production]
user = respect
account = [user]
";
        $expected = 'respect';
        $ENVIRONMENT = 'production';
        $config  = parse_ini_string($config,true);
        $container = new Container($config, $ENVIRONMENT);
        $this->assertEquals($expected, $container->account); //respect
    }

Or am I not getting the point of the exercise? This way you always only have the one configuration, not tempted to go beyond your scope because it is available.

Another thing why does this work:

<?php 
       $config  = $config[$ENVIRONMENT] + $config;

/** but this doesn't ??? */ 

       $config  += $config[$ENVIRONMENT];

/** nor this */

       $config  = $config + $config[$ENVIRONMENT];

Surely it's not a limitation on ini, any ideas?

The patch for my suggestion above:


diff --git a/library/Respect/Config/Container.php b/library/Respect/Config/Container.php
index 6c62512..a2c5537 100644
--- a/library/Respect/Config/Container.php
+++ b/library/Respect/Config/Container.php
@@ -10,9 +10,13 @@ class Container extends ArrayObject
 {
     protected $configurator;

-    public function __construct($configurator=null)
+    public function __construct($configurator=null, $environment=null)
     {
-        $this->configurator = $configurator;
+        $this->environment = $environment;
+        if (isset($environment))
+            $this->configurator = $configurator[$environment];
+        else
+            $this->configurator = $configurator;
     }

     public function __isset($name)
diff --git a/tests/library/Respect/Config/Issues/EnviromentConfigurationTest.php b/tests/library/Respect/Config/Issues/EnviromentConfigurationTest.php
index 8e77da2..7fecef8 100644
--- a/tests/library/Respect/Config/Issues/EnviromentConfigurationTest.php
+++ b/tests/library/Respect/Config/Issues/EnviromentConfigurationTest.php
@@ -20,8 +20,7 @@ account = [user]
         $expected = 'respect';
         $ENVIRONMENT = 'production';
         $config  = parse_ini_string($config,true);
-        $config  = array_merge($config[$ENVIRONMENT], $config);
-        $container = new Container($config);
+        $container = new Container($config, $ENVIRONMENT);
         $this->assertEquals($expected, $container->account); //respect
        }
 }

This was just the simplest way to prove the point. Ideally the configurator internal would keep all the values and maybe when calling $container::envirenment() it will only expose that environment to you, which you can just change again.

Another way which should work out of the box with the current implementation:

Why can't I do this?

<?php

$container->production->account

You can then keep both maybe something like

<?php

$dev = $container->environment(DEVELOPMENT_ENVIRONMENT);

$stage = $container->environment(STAGING_ENVIRONMENT);

/** what about ?? */

$deploy = $container->environment(STAGING_ENVIRONMENT & PRODUCTION_ENVIRONMENT);

I'm probably missing the point, right?