Issue Details (XML | Word | Printable)

Key: GROOVY-2195
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Jochen Theodorou
Reporter: Gabriele Garuglieri
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
groovy

ConfigSlurper does not resolve within closures references to outer properties

Created: 12/Oct/07 05:40 AM   Updated: 02/Nov/07 10:12 AM
Component/s: groovy-jdk
Affects Version/s: 1.1-beta-3
Fix Version/s: 1.1-rc-2

Time Tracking:
Not Specified

File Attachments: 1. Text File ConfigSlurper.patch (0.6 kB)

Environment: Windows XP sp2
Java 1.6.0_02-b06


 Description  « Hide
When processing a scoped closure ConfigSlurper does not resolve references to properties defined out of closure scope.

For example this config file:
a0 = "Goofy"
a1 = "$a0"
a2."$a0" = "Mickey Mouse and " + "$a0"
group1 {
a0 = "Donald Duck"
}
group2 {
a0 = "$a0"
a1 = "$group1.a0"
}
a3 = "$group1.a0"

produces the following:

a0=Goofy
a1=Goofy
a2.Goofy=Mickey Mouse and Goofy
group1.a0=Donald Duck
group2.a0=[:]
group2.a1=[:]
a3=Donald Duck

Applying this patch to ConfigSlurper

— C:/export/java/Groovy/groovy-1.1-beta-3/src/main/groovy/util/ConfigSlurper-save.groovy Fri Sep 21 00:40:58 2007
+++ C:/export/java/Groovy/groovy-1.1-beta-3/src/main/groovy/util/ConfigSlurper.groovy Thu Oct 11 10:10:30 2007
@@ -159,8 +159,9 @@

if(current[name]) { result = current[name] - }

  • else { + } else if (config[name]) { + result = config[name] + } else { result = new ConfigObject() current[name] = result }

makes it correctly resolving "group2" references:
group2.a0=Goofy
group2.a1=Donald Duck



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Gabriele Garuglieri added a comment - 12/Oct/07 06:07 AM
Sorry, it looks like the cut and paste somehow mangled the patch.
I'm attaching it as a file.

Guillaume Laforge added a comment - 12/Oct/07 08:42 AM
Thanks for the patch, Gabriele.
If you want to add some specific unit test demonstrating this, please feel free to do so.

Guillaume Laforge added a comment - 12/Oct/07 10:13 AM
I reopen the issue cause it seems there's a regression in one of the tests of ConfigSlurperTest, in the test checking the serialization.
Gabriele, could you please have a second look at your patch, and the test cases, please?

Guillaume Laforge added a comment - 29/Oct/07 05:05 AM
The suggested change was breaking some tests.
What shall we do with this issue?

Gabriele Garuglieri added a comment - 30/Oct/07 10:44 AM
Ok guys, i didn't disappear. It's only that i have my real job to pursue and i'm really on a tight schedule.
Testing more closely the current behaviour of ConfigSlurper and the side effects the patch is causing, looks that it is so by design and not by mistake.
This means that the patch i proposed looks to be changing the design in a way that would require to recode ConfigSlurper in a way that it's currently beyond my actual knowledge and beyond the time i have available to spend on it, so i think i have to give up.
So what to say?
The patch is not applicable, so disregard it and feel free to close the bug as WAD if you like.
I wish to thank you for your kind attention anyway.

Guillaume Laforge added a comment - 30/Oct/07 10:52 AM
We can keep this issue open till you find some more time to have a deeper look at the issue.
If you and Graeme think it's a desirable change to improve ConfigSlurper even more, then it's certainly worth waiting some more till we all have time to implement that feature properly.
Thanks a lot Gabriele for your time and this patch.

Jochen Theodorou added a comment - 02/Nov/07 10:12 AM
fixed