Skip to content

Commit 43d0964

Browse files
committed
Look for the embedded binary only when it would run
FormatterProvider asks for the formatter on every file formatted, and finding the embedded binary lists a directory. Most projects run the Java-based formatter, or a binary of the user's own, and need no such lookup, so FormatterSelection now takes the embedded binary as a Supplier and asks only when nothing else decides. Removing an earlier session's leftovers now happens once per IDE session too: projects that open together would otherwise race to delete the same directories and log each other's misses.
1 parent 649f32b commit 43d0964

5 files changed

Lines changed: 31 additions & 11 deletions

File tree

‎open-java-format-idea-plugin/src/main/java/com/palantir/javaformat/intellij/EmbeddedNativeImage.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ final class EmbeddedNativeImage {
6969
private final AtomicReference<Activity> activity = new AtomicReference<>(new Activity.Idle());
7070
private final List<Runnable> listeners = new CopyOnWriteArrayList<>();
7171
private final AtomicBoolean noticeShown = new AtomicBoolean();
72+
private final AtomicBoolean staleRemoved = new AtomicBoolean();
7273

7374
EmbeddedNativeImage() {
7475
this(
@@ -243,10 +244,11 @@ Installed install(String version, ProgressIndicator indicator) throws IOExceptio
243244

244245
/**
245246
* Removes what an earlier session could not: the binaries of other versions, one of which Windows may have held
246-
* open while it formatted, and unfinished downloads. Never while a download is under way, whose file would go too.
247+
* open while it formatted, and unfinished downloads. Once per IDE session, by the first project that opens, and
248+
* never while a download is under way, whose file would go too.
247249
*/
248250
void removeStale() {
249-
if (!(activity.get() instanceof Activity.Downloading)) {
251+
if (!(activity.get() instanceof Activity.Downloading) && staleRemoved.compareAndSet(false, true)) {
250252
removeAllBut(installed().map(Installed::version).orElse(""));
251253
}
252254
}

‎open-java-format-idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Optional<FormatterService> get(Project project, PalantirJavaFormatSettings setti
6161
project,
6262
settings.getImplementationClassPath(),
6363
settings.injectedVersionIsOutdated(),
64-
FormatterSelection.of(settings, embeddedNativeImage.get())));
64+
FormatterSelection.of(settings, embeddedNativeImage)));
6565
}
6666

6767
private static Optional<FormatterService> createFormatter(FormatterCacheKey cacheKey) {

‎open-java-format-idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterSelection.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import java.nio.file.Path;
1818
import java.util.Optional;
19+
import java.util.function.Supplier;
1920

2021
/**
2122
* Which formatter a project's settings run. FormatterProvider starts it, and the settings page shows it as the
@@ -25,15 +26,16 @@ sealed interface FormatterSelection {
2526

2627
/**
2728
* A path of the user's own wins over the embedded binary. Without either, the Java-based formatter runs even with
28-
* the native one switched on: the embedded binary may not be downloaded yet.
29+
* the native one switched on: the embedded binary may not be downloaded yet. The embedded binary is looked for
30+
* only when it would run, since that lists a directory and this runs for every file formatted.
2931
*/
30-
static FormatterSelection of(PalantirJavaFormatSettings settings, Optional<Path> embeddedNativeImage) {
32+
static FormatterSelection of(PalantirJavaFormatSettings settings, Supplier<Optional<Path>> embeddedNativeImage) {
3133
if (!settings.useNativeImage()) {
3234
return new JavaBased(false);
3335
}
3436
return settings.getNativeImagePath()
3537
.<FormatterSelection>map(path -> new NativeImage(path, false))
36-
.or(() -> embeddedNativeImage.map(path -> new NativeImage(path, true)))
38+
.or(() -> embeddedNativeImage.get().map(path -> new NativeImage(path, true)))
3739
.orElseGet(() -> new JavaBased(true));
3840
}
3941

‎open-java-format-idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ private void showEmbeddedNativeImage() {
235235
embeddedAction.setVisible(line.action().isPresent());
236236
// What runs with the settings as applied: the embedded binary can arrive while the page is open.
237237
formatterType.setText(FormatterSelection.of(
238-
PalantirJavaFormatSettings.getInstance(project), embeddedNativeImage.installedPath())
238+
PalantirJavaFormatSettings.getInstance(project), embeddedNativeImage::installedPath)
239239
.describe());
240240
}
241241

‎open-java-format-idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterSelectionTest.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.palantir.javaformat.intellij.FormatterSelection.NativeImage;
2121
import java.nio.file.Path;
2222
import java.util.Optional;
23+
import java.util.function.Supplier;
2324
import org.junit.jupiter.api.Test;
2425

2526
final class FormatterSelectionTest {
@@ -34,28 +35,43 @@ void runsTheBinaryTheUserChose() {
3435
settings.setUseNativeImage(true);
3536
settings.setNativeImagePath(Optional.of(CUSTOM));
3637

37-
assertThat(FormatterSelection.of(settings, Optional.of(EMBEDDED))).isEqualTo(new NativeImage(CUSTOM, false));
38+
assertThat(FormatterSelection.of(settings, () -> Optional.of(EMBEDDED)))
39+
.isEqualTo(new NativeImage(CUSTOM, false));
3840
}
3941

4042
@Test
4143
void runsTheEmbeddedBinaryWithoutAPathOfTheUsersOwn() {
4244
settings.setUseNativeImage(true);
4345

44-
assertThat(FormatterSelection.of(settings, Optional.of(EMBEDDED))).isEqualTo(new NativeImage(EMBEDDED, true));
46+
assertThat(FormatterSelection.of(settings, () -> Optional.of(EMBEDDED)))
47+
.isEqualTo(new NativeImage(EMBEDDED, true));
4548
}
4649

4750
@Test
4851
void fallsBackToTheJavaBasedFormatterWithoutABinary() {
4952
settings.setUseNativeImage(true);
5053

51-
assertThat(FormatterSelection.of(settings, Optional.empty())).isEqualTo(new JavaBased(true));
54+
assertThat(FormatterSelection.of(settings, Optional::empty)).isEqualTo(new JavaBased(true));
5255
}
5356

5457
@Test
5558
void runsTheJavaBasedFormatterUnlessTheNativeOneIsOn() {
5659
settings.setNativeImagePath(Optional.of(CUSTOM));
5760

58-
assertThat(FormatterSelection.of(settings, Optional.of(EMBEDDED))).isEqualTo(new JavaBased(false));
61+
assertThat(FormatterSelection.of(settings, () -> Optional.of(EMBEDDED))).isEqualTo(new JavaBased(false));
62+
}
63+
64+
@Test
65+
void looksForTheEmbeddedBinaryOnlyWhenItWouldRun() {
66+
// It is asked for every file formatted, and most projects run the Java-based formatter.
67+
Supplier<Optional<Path>> unasked = () -> {
68+
throw new AssertionError("the embedded binary was looked for");
69+
};
70+
settings.setNativeImagePath(Optional.of(CUSTOM));
71+
72+
assertThat(FormatterSelection.of(settings, unasked)).isEqualTo(new JavaBased(false));
73+
settings.setUseNativeImage(true);
74+
assertThat(FormatterSelection.of(settings, unasked)).isEqualTo(new NativeImage(CUSTOM, false));
5975
}
6076

6177
@Test

0 commit comments

Comments
 (0)