Groovy-Eclipse based Groovy formatter step implementation...

[fvgh]

... including description and smoke test.

The implementation is independent form the other spotless modules.
Follow the build instructions provided in the README.md.

[fvgh]

Since the step implementation is independent, I thought you might give it a review already.
But there is a problem. If the source-jar is generated, the .java source files are assembled in the greclipse build folder. As mentioned earlier, the _ext is not part of the build modules, but it is part of the spotless java (**/.java) check. The FormatExtension provides exclusion for the module build folders, but since _ext is not part of the modules...
But this problem only occurs if a developer builds (actually the build does not require the sources, but the publish and IDE tasks do) the greclipse and then tries to run the spotless task from the main directory.

[nedtwigg]

This LGTM. I'm inclined to publish ext/gr-eclipse to mavencentral so that we don't have to rely on the mavenLocal TestProvisioner. Let me know if this is okay with you @fvgh, and I'll merge and publish.

[fvgh]

Depends how much effort it is for you in case a further patch is required.
Yes, I did some smoke-tests and quite some code inspection (to get it running in the first place).
But I called them smoke-test (ignoring the common use for that term), because they are only showing the principal behavior is compatible with the spotless interface.

[nedtwigg]

Most of the effort is the initial deploy. Future patches are easy. I'll merge and publish this tonight.

[fvgh]

OK. For the _ext problem I can open a dedicated issue where I can explain my concerns a little bit clearer. But I still need some research on composite builds and I also think you had in the past some discussion about how spotless shall handle sub-projects.
Do you think it would be worth opening something new, or had this topic already been discussed fully?

[nedtwigg]

Spotless is more "switchboard" than formatter. It combines formatters from other places. We try to accomplish this with as little glue code as possible - ideally, we can just call a few methods on a jar from mavencentral, as we do with freshmark, google-java-format, scala-fmt, and ktlint.

In some cases, (such as eclipse-jdt and eclipse-groovy) we have no choice but to rebundle existing formatters so that they can be consumed by spotless. This "rebundling" is not part of Spotless' core mission. Eclipse JDT is actually in the process of migrating towards maven central - hopefully we won't need ext/eclipse-jdt in a year or two.

The only purpose of _ext is to huck a jar onto mavencentral that has whatever glue/shim/rebundled code is necessary. I don't care much about its quality, because it's just a shim. We want to make sure it's repeatable - so we can update to new versions, etc. But it's really just there to patch the holes between the p2/eclipse world and the broader gradle/maven world.

If you want to make changes in a shim from _ext, and test them in lib/lib-extra/plugin-gradle, it's awkward. You've got to publish the shim to mavenLocal, then point the testcase to mavenLocal. It's not a clean process.

Most formatters don't need shims, so I don't want to force every Spotless contributor to muck with p2. If you figure out an elegant way around this problem, I'm all for it 👍 But I'm also fine with continuing to kick this minor can down the road. That means I'll just publish the shim as-is, and we can make changes to it later.

[fvgh]

I think we misunderstand each other. I have a concrete problem with the spotless in configuration in spotlessSelf.gradle, where I wondered how to reconfigure it. I think I see your point with _ext, it would just nice to tell 'target' to exclude _ext. But though I think the spotless configuration is easy for 99% of the use-cases, there might be composition builds where it needs more flexibility. And that flexibility would than also let me allow to reconfigure spotlessSelf.gradle. I think that target config should be similar to the way ANT deals with files sets. There you can use attributes for the easy way, and elements, if easy is not flexible enough.
So easy is:

spotless {
	freshmark {
		target '**/*.md', '**/*.MD'

And complex would look like:

spotless {
	freshmark {
		target {
                     include '**/*.md', '**/*.MD',
                     exclude '**/build/*.md'
                }

To my understanding of groovy and and the Java implementation you did in the plugin-gradle, it should actually be simple to accept a closure as well for target, and the rest of the tree implementation already support excludes.

But the question is, whether this really should be solved by making the exclude pattern available, or whether spotless should respect more the gradle composite build structure (but here I still need to read a little bit more). I of course understand that spotless should also be usable to format resource and so on, so it is the question whether it should really follow a project/sub-project restriction. But I think that this had been already discussed somewhere.
Anyhow, as I said, I should read more instead of occupying your time.... And this has nothing to do with this PR-

[nedtwigg]

Sorry, yup, I misunderstood. What you want is possible. You can create a FileTree like so, and pass it to target. Target parses strings in a convenient way, but passes already-realized FileCollection instances unchanged.

Sounds like you'd like to use this to change to _ext before I merge? I'll wait until I get the go-ahead from you to merge and publish.

[fvgh]

Thanks for the clarification. Should not have raised the topic. Told you that I need to read a little bit more 😄

But go ahead.
I would like to keep this branch separated from the rest of the modules. As I wrote initially, this is anyway just relevant for the developer. Want to solve the issue when adding the EclipseFormatterStep to the extra-lib.

[nedtwigg]

Ask away! If I can point you to a doc or code snippet rather than you having to scan the whole darn thing, I see that as a high-leverage use of our time :)