polyglot-scala: Proposal for better Config

[lefou]

TL;DR

Allow empty Config() instances to model empty XML leaf nodes.
Before I prepare a PR, I'd like to have your support.

---

Long Version

The dynamic apply of Configs companion object is very convenient.

Unfortunatelly, creating empty elements (no context and/or no attribute) is currently not supported.
But if you need that, e.g. when using the maven-antrun-plugin, you have to fall back to the class constructor of Config, which is a lot less convenient, harder to read and feels inconsistent compared to the rest of the pom.scala.

Examle:

configuration = Config(
  key1 = "value1"
)

produces:

<configuration>
  <key1>value1</key>
</configuration>

but to add a

<key2/>

I need to convert the complete block to use the constructor of Config which is a cumbersome process and harder to read.

configuration = new Config(Seq(
  "key1" -> Some( "value1"),
  "key2" -> None
))

What if we would allow and support the empty Config() or Config.Empty for this? With this, it would be much more convenient:

configuration = Config(
  key1 = "value1",
  key2 = Config.Empty
)

This was the first part of this proposal.

The second part is about making the Config constructor simpler. The current constructor looks like this:

class Config(val elements: immutable.Seq[(String, Option[Any])])

Now that we have elaborated a way to produce empty elements, there is no longer a need to have that Option. An empty element is one that holds a Config.Empty, all others are non-empty.

So the simplified constructor would look like:

class Config(val elements: immutable.Seq[(String, Any)])

What do you think?

After a discussion and your "go", I'm glad to provide a pull request.

@huntc Could you comment, please?

[h2000]

After a discussion with @lefou i think that an empty element Config() is needed:

For example to construct a single tag <classpath/> in the plugin tag:

  <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>exec-maven-plugin</artifactId>
        <version>1.1.1</version>
        <extensions>false</extensions>
        <inherited>true</inherited>
        <configuration>
          <executable>java</executable>
          <arguments>
            <argument>-d64</argument>
            <argument>-XstartOnFirstThread</argument>
            <argument>-Duser.language=de</argument>
            <argument>-Duser.region=DE</argument>
            <argument>-classpath</argument>
            <classpath />
            <argument>foo.bar.Foo</argument>
          </arguments>
        </configuration>
    </plugin>

is in the current version the following needed:

Plugin(
    gav = Plugins.exec,
    configuration = Config(
         executable = "java",
         arguments = new Config(Seq(
            "argument" -> Some("-d64"),
            "argument" -> Some("-XstartOnFirstThread"),
            "argument" -> Some("-Duser.language=de"),
            "argument" -> Some("-Duser.region=DE"),
            "argument" -> Some("-classpath"),
            "classpath" -> None,
            "argument"  -> Some("foo.bar.Foo"
        )))
    )

with Config() it could be written as

val execPlugin = Plugin(
    gav = Plugins.exec,
    configuration = Config(
         executable = "java",
         arguments = Config(
            argument = "-d64",
            argument = "-XstartOnFirstThread",
            argument = "-Duser.language=de",
            argument = "-Duser.region=DE",
            argument = "-classpath",
            classpath = Config() ,
            argument = ("foo.bar.Foo)
        )))

[lefou]

As it turns out, the whole proposal isn't necessary, because there is already a way to express it, though it is not documented anywhere at the moment. You can simple write a None on the rhs of a configuration entry to effectively create a empty XML leaf node.

To take up the example of the last comment, it could already we written as follows:

Plugin(
        "org.codehaus.mojo" % "exec-maven-plugin" % "1.1.1",
        configuration = Config(
          executable = "java",
          arguments = Config(
            argument = "-d64",
            argument = "-XstartOnFirstThread",
            argument = "-Duser.language=de",
            argument = "-Duser.region=DE",
            argument = "-classpath",
            classpath = None,
            argument = "foo.bar.Foo"
          )
        )
)

I'll submit a new test case that contains this example.

[lefou]

We might close this issue after adding some documentation about that feature.