Html/JS/Json integration

[fvgh]

Providing extension for HTML, JavaScript and JSON.
Gradle and Maven are supported.
Extensions supporting Eclipse-WTP formatter.

Example for gradle usage can be found here.

[nedtwigg]

For all of these XxxDefaults classes, it would be better if their constants were public static String delimiterExpr() {} rather than public static final String DELIMITER_EXPR = .... See https://wiki.sei.cmu.edu/confluence/display/java/DCL59-J.+Do+not+apply+public+final+to+constants+whose+value+might+change+in+later+releases for details.

We've had to change quite a few of our DELIMITER_EXPR so far, I'd expect these might need to be revised in the future as well...

[nedtwigg]

My concern is that web projects generally do not have the same constrained layout that JVM projects have. I wonder how many projects will be able to use the default FILE_FILTER arguments - anyone who does use them is going to have to read their documentation first, and they're fairly large constants. **/* is much less efficient than src/main/css/**, and there probably aren't many projects that use exactly that layout - I think it's unavoidable that most users will have to set the target argument for web projects.

An alternative is to do something closer to what we're doing for prettier:

format 'js', {
  target 'src/main/js/**/*.js'
  eclipseWtp(JS) // default version
}
format 'html', {
  target 'src/main/html/**/*.htm'
  eclipseWtp(HTML, '1.7.0') // specifies version 1.7.0
}

My worry is that this PR adds 1,000 lines, and it's mostly boilerplate and defaults. In order for users to get the benefit of these defaults and boilerplate, we have to document it all, and users have to read it all, and even after we've done that work (us devs and them users), I expect most users will still probably have to throw it away and set their own target. And if that's the case, I think we can reduce the size of the code and the docs a ton by using our approach for prettier.

What do you think about:

public class EclipseWtpFormatterStep {
  ...
  public enum Kind {
    CSS, JS, HTML, ...
  }
}

public class FormatExtension {
  ...
  public class EclipseWtpConfig {
    private final EclipseBasedStepBuilder builder;
    EclipseConfig(EclipseWtpFormatterStep.Kind kind, String version) {
      builder = EclipseWtpFormatterStep.createBuilder(kind, GradleProvisioner.fromProject(getProject()));
      builder.setVersion(version);
      addStep(builder.build());
}

[fvgh]

@nedtwigg I have no preferences on this subject. I thought that the outcome of our previous discussion was to provide separate extensions. And since you accepted #311, I thought that this was the final word.

What I had in mind when I provided #279, is just one eclipseWtp and that per default Eclipse decides by filename extension. But you are right, there should be also an optional selector.

This is really a decision how Spotless configuration shall look like. The separate extensions have the advantage, that you can provide reasonable defaults for different languages. For example I could imagine that the license header can be enhanced, so that the license text does not necessarily contains the language specific comment delimiters.
But it is more coding effort. Whether the code should be restructured is another topic. Same is true for the documentation.

What you propose now is quite close to what I had in mind during #279.

Your decision.

[nedtwigg]

Sorry if I steered you off course :) I'm pretty swamped, but I try really hard to not be the slowpoke that's keeping somebody's hard work form getting merged-in so that they can use it. Seeing the CSS PR on its own, it was a decent hunk of docs, but not a huge amount, so it was easy to merge and release. Seeing the html/js/json blocks here, it's a lot of code and docs, which brings a higher bar to merge. Had you continued to submit one at a time, you probably coulda tricked me into merging it all, no questions asked ;-)

In our previous discussion which you linked, my intent was to emphasize

The real proof will be the docs. If it's easier to document html {}, css {} - dedicated formats, then that's what we should do.

Since that discussion, prettier has been merged in, and we can see how it gets used in the wild:

https://github.com/mytakedotorg/mytakedotorg/blob/342779de86bfe7a116c3a1b9aa7d9c9cf08fd829/client/build.gradle#L28-L38

That approach is easy to read, and it's easy to see exactly what is getting formatted. In the per-format approach with default includes, the user has to read quite a lot in order to tell what is and isn't getting formatted. A casual user of the buildscript, who might not know where the Spotless docs even are, would have a hard time debugging the per-format case, compared to the prettier example that I linked above.

In the long term, I think we should

• integrate WTP in the same way that prettier is integrated
• deprecate the CSS extension and remove it from the docs

In the short term, if you plan to use these formatters at work, I'm happy to merge this as-is so that you can get on with your formatting, and we can open up an issue to redo them in a more generic way.

[fvgh]

@nedtwigg
Thanks for clarification. I will provide an WTP integration as requested in another another PR.

Afterwards I will mark the XML and CSS as deprecated in a separate PR, whereas I will refer to the new Java methods. In the ReadMe I will remove the sections entirely (since the interfaces shall not be used anymore). Please confirm that it is understood that both extensions should be removed (anything else would make no sense).

Agreed? Then we can close this PR. I would like to keep this branch (not the PR) alive until the other two PRs are agreed.

[nedtwigg]

I agree 100%. Your WTP formatter is so versatile, I think it was inevitable that we would stumble around a little while we searched for just the right approach :) Thanks for the valuable prototyping to get here.

[fvgh]

@nedtwigg I provided a new branch, containing a proposal for WTP formatter configurable via custom target.
Unfortunately I found no way, to configure the WTP formatter type via enum in Gradle. have I missed something?
Furthermore I found that the XML formatter in Maven never worked. I thought that the dependency filter works recursively, but it did not. I added a test now, that verifies the solution. Sorry about not testing my previous solution properly.

Let me know what you think about the approach. After we agreed on an implementation, I will:

• update existing unit tests
• mark all the extensions and type specific calls deprecated (referring to the new methods)
• replace the previous documentations by two (Maven+Gradle) generic once
• provide PR and change-log updates

[nedtwigg]

Looks great. I added a commit with some comments: 11147a9

[fvgh]

PR replaced by #325.