Skip to content

Commit

Permalink
Change to using the EclipseWtpFormatterStep enum as the argument.
Browse files Browse the repository at this point in the history
  • Loading branch information
@nedtwigg
nedtwigg committed Dec 18, 2018
1 parent 6369881 commit 11147a9
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 33 deletions.
19 changes: 0 additions & 19 deletions lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/EclipseWtpFormatterStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,9 @@
*/
package com.diffplug.spotless.extra.wtp;

import static java.util.Arrays.stream;
import static java.util.stream.Collectors.joining;

import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Locale;
import java.util.Objects;
import java.util.Properties;

import com.diffplug.spotless.FormatterFunc;
Expand Down Expand Up @@ -53,20 +48,6 @@ public enum EclipseWtpFormatterStep {
this.formatterCall = formatterCall;
}

/** Similar to {@link #valueOf(Class, String)}, ignores whitespace, case and adds helpful exception messages. */
public static EclipseWtpFormatterStep valueFrom(String name) {
Objects.requireNonNull(name);
name = name.trim().toUpperCase(Locale.ENGLISH);
try {
return valueOf(name);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(e.getMessage() +
" (allowed values: " +
stream(values()).map(enumType -> enumType.toString()).collect(joining("; ")) +
")");
}
}

This comment has been minimized.

# to view

Copy link
@nedtwigg

nedtwigg Dec 18, 2018

Author Member

In general, making an argument case-insensitive is just opening a can of worms. The less code we have to maintain, the better!
public EclipseBasedStepBuilder createBuilder(Provisioner provisioner) {
return new EclipseBasedStepBuilder(NAME, " - " + toString(), provisioner, state -> formatterCall.apply(implementationClassName, state));
}
Expand Down
17 changes: 5 additions & 12 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,13 +488,9 @@ public PrettierConfig prettier() {
public class EclipseWtpConfig {
private final EclipseBasedStepBuilder builder;

EclipseWtpConfig(EclipseWtpFormatterStep type) {
this(type, EclipseWtpFormatterStep.defaultVersion());
}

EclipseWtpConfig(EclipseWtpFormatterStep type, String version) {
builder = type.createBuilder(GradleProvisioner.fromProject(getProject()));
builder.setVersion(EclipseWtpFormatterStep.defaultVersion());
builder.setVersion(version);
addStep(builder.build());

This comment has been minimized.

# to view

Copy link
@nedtwigg

nedtwigg Dec 18, 2018

Author Member

Little bugfix. Job of the class is to maintain an invariant, easier to do well with fewer constructors.
}

Expand All @@ -504,17 +500,14 @@ public void configFile(Object... configFiles) {
builder.setPreferences(project.files(configFiles).getFiles());
replaceStep(builder.build());
}

}

public EclipseWtpConfig eclipseWtp(String type) {

This comment has been minimized.

# to view

Copy link
@nedtwigg

nedtwigg Dec 18, 2018

Author Member

Groovy converts Strings to enums automatically:

http://mrhaki.blogspot.com/2010/12/groovy-goodness-transform-string-into.html

This comment has been minimized.

# to view

Copy link
@fvgh

fvgh Dec 22, 2018

Member

Due to your example in #315, I thought that there is a way to use the Enum directly in Gradle without an import. Seems I got that wrong.
return new EclipseWtpConfig(EclipseWtpFormatterStep.valueFrom(type));
public EclipseWtpConfig eclipseWtp(EclipseWtpFormatterStep type) {
return eclipseWtp(type, EclipseWtpFormatterStep.defaultVersion());
}

public EclipseWtpConfig eclipseWtp(String type, String version) {
return new EclipseWtpConfig(
EclipseWtpFormatterStep.valueFrom(type),
version);
public EclipseWtpConfig eclipseWtp(EclipseWtpFormatterStep type, String version) {
return new EclipseWtpConfig(type, version);
}

/** Sets up a format task according to the values in this extension. */
Expand Down
4 changes: 2 additions & 2 deletions plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/EclipseWtp.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

public class EclipseWtp implements FormatterStepFactory {
@Parameter
private String type;

This comment has been minimized.

# to view

Copy link
@nedtwigg

nedtwigg Dec 18, 2018

Author Member

All my maven tests are failing because it can't download deps (before this change too). So I'm not 100% sure that this change works. If it doesn't, then going back to your String sounds good to me.

This comment has been minimized.

# to view

Copy link
@fvgh

fvgh Dec 22, 2018 •
edited
Loading

Member

Yes, the resolveArtifacts seems not to download the artifacts, though I understood the method documentation differently. Problem also occurred in my version when I did a clean build.
private EclipseWtpFormatterStep type;

@Parameter
private String version;
Expand All @@ -38,7 +38,7 @@ public class EclipseWtp implements FormatterStepFactory {

@Override
public FormatterStep newFormatterStep(FormatterStepConfig stepConfig) {
EclipseBasedStepBuilder eclipseConfig = EclipseWtpFormatterStep.valueFrom(type).createBuilder(stepConfig.getProvisioner());
EclipseBasedStepBuilder eclipseConfig = type.createBuilder(stepConfig.getProvisioner());
eclipseConfig.setVersion(version == null ? EclipseWtpFormatterStep.defaultVersion() : version);
if (null != files) {
eclipseConfig.setPreferences(
Expand Down

0 comments on commit 11147a9

Please # to comment.