On 18/09/2012, at 8:23 PM, Luke Daley wrote:
>
> On 18/09/2012, at 1:50 AM, Adam Murdoch wrote:
>
>>
>> On 18/09/2012, at 8:13 AM, GitHub wrote:
>>
>>> Branch: refs/heads/master
>>> Home: https://github.com/gradle/gradle
>>> Commit: d0da246b7aecc632fc90c886d6a4b3421208c840
>>>
>>> https://github.com/gradle/gradle/commit/d0da246b7aecc632fc90c886d6a4b3421208c840
>>> Author: Luke Daley <[email protected]>
>>> Date: 2012-09-17 (Mon, 17 Sep 2012)
>>>
>>> Changed paths:
>>> M
>>> subprojects/core/src/main/groovy/org/gradle/api/internal/AsmBackedClassGenerator.java
>>> A
>>> subprojects/core/src/main/groovy/org/gradle/api/internal/ClosureBackedAction.java
>>> A
>>> subprojects/core/src/testFixtures/groovy/org/gradle/api/internal/ClosureBackedActionTest.groovy
>>>
>>> Log Message:
>>> -----------
>>> Extract ClosureBackedAction to a first level class and add some test
>>> coverage.
>>>
>>>
>>> Commit: 045e919a66198ebc4985b93fb6239df129ac6a89
>>>
>>> https://github.com/gradle/gradle/commit/045e919a66198ebc4985b93fb6239df129ac6a89
>>> Author: Luke Daley <[email protected]>
>>> Date: 2012-09-17 (Mon, 17 Sep 2012)
>>>
>>> Changed paths:
>>> M subprojects/core/src/main/groovy/org/gradle/util/CollectionUtils.java
>>> M
>>> subprojects/core/src/test/groovy/org/gradle/util/CollectionUtilsTest.groovy
>>>
>>> Log Message:
>>> -----------
>>> collect for arrays
>>>
>>>
>>> Commit: 1a284bda1bb810d09c860701e052f1e6684d799f
>>>
>>> https://github.com/gradle/gradle/commit/1a284bda1bb810d09c860701e052f1e6684d799f
>>> Author: Luke Daley <[email protected]>
>>> Date: 2012-09-17 (Mon, 17 Sep 2012)
>>>
>>> Changed paths:
>>> A
>>> subprojects/core/src/main/groovy/org/gradle/api/InvalidActionClosureException.java
>>> M
>>> subprojects/core/src/main/groovy/org/gradle/api/internal/ClosureBackedAction.java
>>> M
>>> subprojects/core/src/testFixtures/groovy/org/gradle/api/internal/ClosureBackedActionTest.groovy
>>>
>>> Log Message:
>>> -----------
>>> Added explicit error handling for using an invalid closure as an action
>>> implementation.
>>>
>>>
>>> Commit: 451cc4b8cdc694c8c717080f9b13df4a1b0c2e08
>>>
>>> https://github.com/gradle/gradle/commit/451cc4b8cdc694c8c717080f9b13df4a1b0c2e08
>>> Author: Luke Daley <[email protected]>
>>> Date: 2012-09-17 (Mon, 17 Sep 2012)
>>>
>>> Changed paths:
>>> M
>>> subprojects/core/src/main/groovy/org/gradle/api/internal/AbstractClassGenerator.java
>>> M
>>> subprojects/core/src/main/groovy/org/gradle/api/internal/AsmBackedClassGenerator.java
>>> M subprojects/core/src/main/groovy/org/gradle/util/CollectionUtils.java
>>> M
>>> subprojects/core/src/test/groovy/org/gradle/api/internal/AsmBackedClassGeneratorGroovyTest.groovy
>>>
>>> Log Message:
>>> -----------
>>> Generate closure adapters for any method with an Action as the last
>>> parameter.
>>
>> I'm wondering about our strategy for mixing in the DSL stuff, longer term.
>>
>> It feels like we should move this out of the class generation and into a
>> DynamicObject implementation that the decorated class delegates to (which we
>> already have). We don't actually need to mix this stuff in statically, and
>> it might all be a bit simpler if we composed the DSL layer from a bunch of
>> dynamic property and method handlers (we might bust up DynamicObject into a
>> few separate facets to make this easier), instead of bloating out the class
>> generation.
>
> Pros:
> 1. Easier to write delegation code than ASM code
2. Scales better as we make the DSL mapping more complex, as we can compose the
behaviour.
3. We can mix in the DSL over dynamic properties and methods. That means, for
example, if I add an extra property, I can use the DSL methods to configure it.
Same for the elements of a domain object container.
4. We can reuse the mix-in stuff in the delegate of a configure closure, so
that the DSL can be used to configure an undecorated object.
>
> Cons:
> 1. Longer stack traces
This is an interesting one. Probably by default the stack traces we log should
not have any Gradle stuff in them at all, just the user stuff.
> 2. Performance
Possibly. We'd have to measure it first. By restructuring DynamicObject, we
might well find that we're faster for some things (e.g. extension properties
and methods). For example, we wouldn't have to do the double lookup of
hasProperty() and getProperty(). This could offset making the convenience
methods slower.
But the real solution to DSL performance is to avoid configuring the things we
don't need.
> 3. Groovy specific solution
That could mean a bunch of things:
* We're mixing in overloaded methods that take Closure as an alternative to
Action.
* We'd be using Groovy specific propertyMissing() and methodMissing() methods
to mix in all decoration, even stuff we know statically.
I'm guessing you meant the latter. It is a downside, but I'm not too concerned
about it. Java and Groovy are our target languages. Given that the static stuff
isn't available at compile time, if it's currently being used from Java, it's
being done reflectively. It is possible that someone's doing this, but seems
unlikely. Strictly speaking, it would be a breaking change to remove these
methods, but I think we could just release note it.
>
>> Alternatively, if we do this statically, it would be nice if we could figure
>> out a way to mix it in earlier in the development lifecycle so that it's
>> available in, say, javadocs and code completion in the IDE and from static
>> languages and so on.
>
> We can make the DSL reference aware of this stuff. I think if we clearly
> explain the rules somewhere then people can extrapolate from the raw javadocs.
>
> For code completion in IDEA or Eclipse, this could be described using their
> DSL descriptor stuff. Given that there's a runtime component though, I'm not
> sure how well we could target the addition of the behaviour with the
> descriptors. That is, I'm not sure how we'd identify in the descriptors what
> has been decorated.
We probably have to generate the descriptors, so you'd need to run the build
before you could compile or run stuff in the IDE.
>
> If this stuff was in the class files, it would mean we don't have to write
> the DSL descriptors. We could do this at compile time I guess with a gradle
> `gradle-plugin' plugin that decorates the class files. That throws up
> problems with divergence from source and maybe throws up binary compatibility
> issues.
Plus it means you couldn't easily run tests from the IDE, without first running
the build to decorate the classes.
>
>> Either way, we also need to have the DSL reference understand all the
>> mixed-in stuff.
>>
>> While we're kind of on the topic, I'm curious if anyone has some suggestions
>> for how a configuration DSL might be added for nested domain objects. So,
>> say I have:
>>
>> class Thing {
>> ThingOptions options = new ThingOptions()
>> String name
>> }
>>
>> I want methods to configure 'thing.options', but not 'thing.name', so that I
>> can do things like:
>>
>> thing.options { option1 'foo' }
>> thing.options(option1: 'foo')
>>
>> But I don't need to do things like:
>>
>> thing.name { // what would I do here? }
>
> Why not just generate it for String as well? Agreed that it's of little use,
> but what's the harm? If it makes the “rules” simpler and avoids need to
> annotate what is configurable then it might be simpler to just do it for
> everything.
It is an option, I suppose. Just bothers me for some reason.
--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com