Skip to content

Commit 1f80a21

Browse files
simschlasimschla
committed
chore: switch to write Output directly to System.err
as it turns out, depending on the os (?), there is no "real" way to detach a logger from the others (in our case the "outputLogger" from the rootLogger). By moving away from the log-only approach and instead write output directly to stdErr, we bypass this os differences.
1 parent fccdf48 commit 1f80a21

File tree

4 files changed

+54
-49
lines changed

4 files changed

+54
-49
lines changed

‎app/src/main/java/com/diffplug/spotless/cli/SpotlessMode.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ ResultType handleResult(Result result) {
4343
"File has lints: {}\n\t{}",
4444
result.target().toFile().getPath(),
4545
result.lintState()
46-
.asStringDetailed(result.target().toFile(), result.formatter())))
47-
.write();
46+
.asStringDetailed(result.target().toFile(), result.formatter())));
4847
return ResultType.DIRTY;
4948
}
5049

@@ -73,8 +72,7 @@ ResultType handleResult(Result result) {
7372
delim);
7473
}
7574
return new Output.MessageWithArgs("File is clean: {}", result.target());
76-
})
77-
.write();
75+
});
7876
} catch (IOException e) {
7977
throw ThrowingEx.asRuntime(e);
8078
}
@@ -106,8 +104,7 @@ ResultType handleResult(Result result) {
106104
"File has lints: {}\n\t{}",
107105
result.target().toFile().getPath(),
108106
result.lintState()
109-
.asStringDetailed(result.target().toFile(), result.formatter())))
110-
.write();
107+
.asStringDetailed(result.target().toFile(), result.formatter())));
111108
return ResultType.DIRTY;
112109
} else {
113110
LOGGER.debug(

‎app/src/main/java/com/diffplug/spotless/cli/logging/output/LoggingConfigurer.java‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,54 +52,46 @@ private static void configureJdkLogging(@NotNull CLIOutputLevel cliOutputLevel,
5252
final Logger spotlessLibLogger = Logger.getLogger("com.diffplug.spotless");
5353
final Logger spotlessCliLogger = Logger.getLogger("com.diffplug.spotless.cli");
5454

55-
final ConsoleHandler outputConsoleHandler = new ConsoleHandler();
56-
outputConsoleHandler.setLevel(Level.ALL); // make sure everything is logged
57-
outputConsoleHandler.setFormatter(new PlainMessageFormatter()); // Set formatter
58-
59-
final Logger outputLogger = Logger.getLogger(Output.OUTPUT_LOGGER_NAME);
60-
outputLogger.setUseParentHandlers(false);
61-
outputLogger.addHandler(outputConsoleHandler);
62-
6355
// set the logging level per logger
6456
switch (cliOutputLevel) {
6557
case QUIET -> {
66-
outputLogger.setLevel(Level.SEVERE);
58+
Output.setLevel(Level.SEVERE);
6759
spotlessCliLogger.setLevel(Level.SEVERE);
6860
spotlessLibLogger.setLevel(Level.SEVERE);
6961
rootLogger.setLevel(Level.SEVERE);
7062
}
7163
case DEFAULT -> {
72-
outputLogger.setLevel(Level.INFO);
64+
Output.setLevel(Level.INFO);
7365
spotlessCliLogger.setLevel(Level.WARNING);
7466
spotlessLibLogger.setLevel(Level.WARNING);
7567
rootLogger.setLevel(Level.SEVERE);
7668
}
7769
case V -> {
78-
outputLogger.setLevel(Level.INFO);
70+
Output.setLevel(Level.INFO);
7971
spotlessCliLogger.setLevel(Level.INFO);
8072
spotlessLibLogger.setLevel(Level.WARNING);
8173
rootLogger.setLevel(Level.SEVERE);
8274
}
8375
case VV -> {
84-
outputLogger.setLevel(Level.INFO);
76+
Output.setLevel(Level.INFO);
8577
spotlessLibLogger.setLevel(Level.INFO);
8678
spotlessCliLogger.setLevel(Level.INFO);
8779
rootLogger.setLevel(Level.SEVERE);
8880
}
8981
case VVV -> {
90-
outputLogger.setLevel(Level.ALL);
82+
Output.setLevel(Level.ALL);
9183
spotlessCliLogger.setLevel(Level.ALL);
9284
spotlessLibLogger.setLevel(Level.ALL);
9385
rootLogger.setLevel(Level.SEVERE);
9486
}
9587
case VVVV -> {
96-
outputLogger.setLevel(Level.ALL);
88+
Output.setLevel(Level.ALL);
9789
spotlessCliLogger.setLevel(Level.ALL);
9890
spotlessLibLogger.setLevel(Level.ALL);
9991
rootLogger.setLevel(Level.INFO);
10092
}
10193
case VVVVV -> {
102-
outputLogger.setLevel(Level.ALL);
94+
Output.setLevel(Level.ALL);
10395
spotlessCliLogger.setLevel(Level.ALL);
10496
spotlessLibLogger.setLevel(Level.ALL);
10597
rootLogger.setLevel(Level.ALL);

‎app/src/main/java/com/diffplug/spotless/cli/logging/output/Output.java‎

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,42 @@
1515
*/
1616
package com.diffplug.spotless.cli.logging.output;
1717

18+
import java.util.Objects;
1819
import java.util.function.Supplier;
20+
import java.util.logging.Level;
1921

2022
import org.jetbrains.annotations.NotNull;
2123
import org.slf4j.Logger;
2224
import org.slf4j.LoggerFactory;
2325

2426
public final class Output {
2527

26-
public static final String OUTPUT_LOGGER_NAME = "CLI_OUTPUT";
27-
static final Logger CLI_OUTPUT = LoggerFactory.getLogger(OUTPUT_LOGGER_NAME);
28+
private static final Logger LOGGER = LoggerFactory.getLogger(Output.class);
2829

29-
public static void write(String message, Object... args) {
30-
CLI_OUTPUT.info(message, args);
30+
private static volatile Level _level = Level.INFO;
31+
32+
public static void setLevel(Level level) {
33+
Output._level = level;
34+
}
35+
36+
public static void output(@NotNull String message, Object... args) {
37+
Objects.requireNonNull(message);
38+
System.err.printf(slf4jMessageToPrintfMessage(message), args);
39+
LOGGER.info(message, args);
40+
}
41+
42+
private static @NotNull String slf4jMessageToPrintfMessage(@NotNull String message) {
43+
Objects.requireNonNull(message);
44+
return message.replace("{}", "%s") + "%n";
3145
}
3246

33-
public static DefaultOrDetailOutputBuilder eitherDefault(Supplier<MessageWithArgs> defaultMessageSupplier) {
47+
public static DefaultOrDetailOutputBuilder eitherDefault(
48+
@NotNull Supplier<MessageWithArgs> defaultMessageSupplier) {
49+
Objects.requireNonNull(defaultMessageSupplier);
3450
return new DefaultOrDetailOutputBuilder(defaultMessageSupplier);
3551
}
3652

37-
public static class DefaultOrDetailOutputBuilder implements ToOutputWriter {
53+
public static class DefaultOrDetailOutputBuilder {
3854

3955
@NotNull private final Supplier<MessageWithArgs> defaultMessageWithArgs;
4056

@@ -44,31 +60,30 @@ public DefaultOrDetailOutputBuilder(@NotNull Supplier<MessageWithArgs> defaultMe
4460
this.defaultMessageWithArgs = defaultMessageWithArgs;
4561
}
4662

47-
public ToOutputWriter orDetail(@NotNull Supplier<MessageWithArgs> detailMessageWithArgs) {
63+
public void orDetail(@NotNull Supplier<MessageWithArgs> detailMessageWithArgs) {
64+
Objects.requireNonNull(detailMessageWithArgs);
4865
this.detailMessageWithArgs = detailMessageWithArgs;
49-
return this;
66+
write();
5067
}
5168

52-
@Override
53-
public void write() {
54-
if (CLI_OUTPUT.isDebugEnabled()) {
69+
private void write() {
70+
if (_level.intValue() <= Level.FINE.intValue()) {
5571
MessageWithArgs messageWithArgs = detailMessageWithArgs.get();
56-
CLI_OUTPUT.debug(messageWithArgs.message(), messageWithArgs.args());
72+
System.err.printf(slf4jMessageToPrintfMessage(messageWithArgs.message()), messageWithArgs.args());
73+
LOGGER.debug(messageWithArgs.message(), messageWithArgs.args());
5774
return;
5875
}
59-
if (CLI_OUTPUT.isInfoEnabled()) {
60-
MessageWithArgs messageWithArgs = defaultMessageWithArgs.get();
61-
CLI_OUTPUT.info(messageWithArgs.message(), messageWithArgs.args());
76+
MessageWithArgs messageWithArgs = defaultMessageWithArgs.get();
77+
if (_level.intValue() <= Level.INFO.intValue()) {
78+
System.err.printf(slf4jMessageToPrintfMessage(messageWithArgs.message()), messageWithArgs.args());
79+
}
80+
if (LOGGER.isInfoEnabled()) {
81+
LOGGER.info(messageWithArgs.message(), messageWithArgs.args());
6282
}
6383
}
6484
}
6585

66-
public interface ToOutputWriter {
67-
void write();
68-
}
69-
7086
public record MessageWithArgs(String message, Object... args) {
71-
7287
static MessageWithArgs create(String message, Object... args) {
7388
return new MessageWithArgs(message, args);
7489
}

‎app/src/test/java/com/diffplug/spotless/cli/SpotlessCLILoggingJavaProcessTest.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.diffplug.spotless.cli;
1717

1818
import java.util.function.Function;
19+
import java.util.regex.Pattern;
1920

2021
import org.assertj.core.api.SoftAssertions;
2122
import org.junit.jupiter.api.Test;
@@ -29,7 +30,7 @@ public class SpotlessCLILoggingJavaProcessTest extends CLIIntegrationHarness {
2930
public static final String NEEDS_REFORMATTING_STATEMENT = "needs reformatting";
3031
public static final String FILE_NAME = "Java.java";
3132
public static final String LOGFMT_KEYWORD = "timestamp=";
32-
public static final String FILE_DIFF_MARKER = "****";
33+
public static final Pattern FILE_DIFF_MARKER_PATTERN = Pattern.compile("^\\Q******\\E", Pattern.MULTILINE);
3334
public static final String LOGGER_SOURCE_NAME_FROM_SPOTLESS_CLI_PACKAGE =
3435
"source.logger.name=com.diffplug.spotless.cli";
3536
public static final String LOGGER_SOURCE_NAME_SPOTLESS_BUT_NOT_CLI_PACKAGE_PATTERN =
@@ -62,7 +63,7 @@ void defaultLogLevelGivesSomeOutput() {
6263
.contains(NEEDS_REFORMATTING_STATEMENT)
6364
.contains(FILE_NAME)
6465
.doesNotContain(LOGFMT_KEYWORD)
65-
.doesNotContain(FILE_DIFF_MARKER);
66+
.doesNotContainPattern(FILE_DIFF_MARKER_PATTERN);
6667
softly.assertThat(result.exitCode()).isEqualTo(1);
6768
});
6869
}
@@ -84,7 +85,7 @@ void quietLogLevelDoesGiveNoOutput() {
8485
.doesNotContainPattern(LOGGER_SOURCE_NAME_SPOTLESS_BUT_NOT_CLI_PACKAGE_PATTERN)
8586
.doesNotContainPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_PATTERN)
8687
.doesNotContain(LOGFMT_DEBUG_LOG_LEVEL)
87-
.doesNotContain(FILE_DIFF_MARKER);
88+
.doesNotContainPattern(FILE_DIFF_MARKER_PATTERN);
8889
softly.assertThat(result.exitCode()).isEqualTo(1);
8990
});
9091
}
@@ -101,7 +102,7 @@ void verbose1LogLevelShowsCLIInfo() {
101102
.doesNotContainPattern(LOGGER_SOURCE_NAME_SPOTLESS_BUT_NOT_CLI_PACKAGE_PATTERN)
102103
.doesNotContainPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_PATTERN)
103104
.doesNotContain(LOGFMT_DEBUG_LOG_LEVEL)
104-
.doesNotContain(FILE_DIFF_MARKER); // no diffs
105+
.doesNotContainPattern(FILE_DIFF_MARKER_PATTERN); // no diffs
105106
softly.assertThat(result.exitCode()).isEqualTo(1);
106107
});
107108
}
@@ -117,7 +118,7 @@ void verbose2LogLevelShowsCLIAndAllSpotlessOnLevelInfo() {
117118
.containsPattern(LOGGER_SOURCE_NAME_SPOTLESS_BUT_NOT_CLI_PACKAGE_PATTERN)
118119
.doesNotContainPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_PATTERN)
119120
.doesNotContain(LOGFMT_DEBUG_LOG_LEVEL)
120-
.doesNotContain(FILE_DIFF_MARKER); // no diffs
121+
.doesNotContainPattern(FILE_DIFF_MARKER_PATTERN); // no diffs
121122
softly.assertThat(result.exitCode()).isEqualTo(1);
122123
});
123124
}
@@ -133,7 +134,7 @@ void verbose3LogLevelShowsCLIAndSpotlessOnLevelDebugIncludingDiffs() {
133134
.containsPattern(LOGGER_SOURCE_NAME_SPOTLESS_BUT_NOT_CLI_PACKAGE_PATTERN)
134135
.doesNotContainPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_PATTERN)
135136
.contains(LOGFMT_DEBUG_LOG_LEVEL)
136-
.contains(FILE_DIFF_MARKER);
137+
.containsPattern(FILE_DIFF_MARKER_PATTERN);
137138
softly.assertThat(result.exitCode()).isEqualTo(1);
138139
});
139140
}
@@ -149,7 +150,7 @@ void verbose4LogLevelShowsCliAndSpotlessOnDebugAndEverythingElseOnInfo() {
149150
.containsPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_ON_INFO_LEVEL_PATTERN)
150151
.doesNotContainPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_ON_DEBUG_LEVEL_PATTERN)
151152
.contains(LOGFMT_DEBUG_LOG_LEVEL)
152-
.contains(FILE_DIFF_MARKER);
153+
.containsPattern(FILE_DIFF_MARKER_PATTERN);
153154
softly.assertThat(result.exitCode()).isEqualTo(1);
154155
});
155156
}
@@ -165,7 +166,7 @@ void verbose5LogLevelShowsCliAndSpotlessAndEverythingElseOnDebug() {
165166
.containsPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_ON_INFO_LEVEL_PATTERN)
166167
.containsPattern(LOGGER_SOURCE_NAME_OUTSIDE_SPOTLESS_ON_DEBUG_LEVEL_PATTERN)
167168
.contains(LOGFMT_DEBUG_LOG_LEVEL)
168-
.contains(FILE_DIFF_MARKER);
169+
.containsPattern(FILE_DIFF_MARKER_PATTERN);
169170
softly.assertThat(result.exitCode()).isEqualTo(1);
170171
});
171172
}

0 commit comments

Comments
 (0)