ConfigSlurper - multiple environment blocks broken

Sorry about the formatting!

add code blocks

I could try to produce a pull request of this, but the question is:

Is there a valid use case for this feature?

Regards,
Pascal

I took a look at the code of configslurper. This defect and some other open ones are caused by the fact that it parses the config as a groovy script. Fixing this would mean a complete and complicated rewrite of the class. So i think its best to just accept the limitations. Sorry!

I think this is a very natural thing to want to do. Suppose you start with a 100-line configuration, and then wish to make line 30 environment-specific - so you add an 'environment' block. Then you decide line 70 needs to be customized too - you add an environment block there. This is the natural, and seemingly obvious thing to do; but now your environment blocks are not working. This is essentially how I found this bug, and I was rather surprised to find that my first environment block was ignored - it is not at all intuitive.

If there are really a number of defects open around this behaviour, perhaps that is an argument for rewriting?

Another use case that comes up in Grails is that you have a Config.groovy with a nice environments block in it already. Then you install a plugin that needs to set up another environments block. If the plugin just adds its config to the end of Config.groovy, now you have two environments blocks and trouble ensues.

Is the fix as simple as the following change on line 202 of ConfigSlurper.groovy?

def co = new ConfigObject()

to

def co = config[ENV_SETTINGS] ?: new ConfigObject()

This seems to fix the test case - presumably the issue is that each new block creates a brand new config object, discarding those populated earlier.

I've attached a patch which fixes the issue and also supports environment blocks inside configuration blocks, which currently don't work as I (or, I think, anyone else) would expect. My previous suggestion only fixes root-level environment blocks. The patch addresses both issues.

Here's the test case:

def c = new ConfigSlurper('development').parse("""
environments {
    development {
        a = 1
    }
}
environments {
    development {
        b = 2
    }
}

blah {
    environments {
        development {
            c = 3
        }
    }
    environments {
        development {
            d = 4
        }
    }
}
""")
assert c == [a:1, b: 2, blah:[c: 3, d: 4]]

Which currently results in:

assert c == [a:1, b: 2, blah:[c: 3, d: 4]]
       | |
       | false
       [blah:[:], d:4]

Richard, can you add some tests to the patch?

+1 for this fix

I created a pull request https://github.com/groovy/groovy-core/pull/95 out of the patch and the tests submitted by Richard. All credit should go to him.

I change the issue type to improvement, since I think this is not really a bug.

patch/pull request applied