Skip to content

Commit

Permalink
Use updated output with smithy build and diff
Browse files Browse the repository at this point in the history
Smithy diff and build now uses the pretty validation output and
color theming options.

Because Styles were copied and repated multiple times, I introduced
a ColorTheme class to store reusable styles. This isn't a swappable
color scheme system, but could be evolved into one in the future if
needed.

For Smithy Diff:

The old and new models are both validated, but only build-failing
errors are shown. When the original model fails, a label is added
before ERROR/DANGER to show it's on the OLD model. When the new
model fails, it shows a NEW label.

When running the diff with two valid models, events have a label
of DIFF. The events shown here use the --severity option and default
to WARNING when not set.

For Smithy Build:

Each projection output is contained in a titled section using a
colored title to match whether the projection passed or failed.
Any events encountered during the projection are emitted using the
updated styling and have a prefix label using the projection name.
  • Loading branch information
mtdowling committed Apr 3, 2023
1 parent 885066e commit f00caa9
Show file tree
Hide file tree
Showing 22 changed files with 471 additions and 163 deletions.
4 changes: 4 additions & 0 deletions config/spotbugs/filter.xml
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@
<Class name="software.amazon.smithy.cli.commands.WarmupCommand"/>
<Bug pattern="DM_DEFAULT_ENCODING"/>
</Match>
<Match>
<Class name="software.amazon.smithy.cli.DiffCommandTest"/>
<Bug pattern="DM_DEFAULT_ENCODING"/>
</Match>

<!-- SecurityManager is deprecated and scheduled for removal, so this isn't a good check. -->
<Match>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.cli;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.UncheckedIOException;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.utils.ListUtils;

public class DiffCommandTest {
@Test
public void passingDiffEventsExitZero() {
IntegUtils.withTempDir("diff", dir -> {
Path a = dir.resolve("a.smithy");
writeFile(a, "$version: \"2.0\"\nnamespace example\nstring A\n");

RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--old", a.toString(), "--new", a.toString()));
assertThat(result.getExitCode(), equalTo(0));
});
}

@Test
public void showsLabelForOldModelEvents() {
IntegUtils.withTempDir("diff", dir -> {
Path a = dir.resolve("a.smithy");
writeFile(a, "$version: \"2.0\"\nnamespace example\n@aaaaaa\nstring A\n");

RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--old", a.toString(), "--new", a.toString()));
assertThat(result.getExitCode(), equalTo(1));
assertThat(result.getOutput(), containsString("── OLD ERROR ──"));
});
}

@Test
public void showsLabelForNewModelEvents() {
IntegUtils.withTempDir("diff", dir -> {
Path a = dir.resolve("a.smithy");
writeFile(a, "$version: \"2.0\"\nnamespace example\nstring A\n");

Path b = dir.resolve("b.smithy");
writeFile(b, "$version: \"2.0\"\nnamespace example\n@aaaaaa\nstring A\n");

RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--old", a.toString(), "--new", b.toString()));
assertThat(result.getExitCode(), equalTo(1));
assertThat(result.getOutput(), containsString("── NEW ERROR ──"));
});
}

@Test
public void showsLabelForDiffEvents() {
IntegUtils.withTempDir("diff", dir -> {
Path a = dir.resolve("a.smithy");
writeFile(a, "$version: \"2.0\"\nnamespace example\nstring A\n");

Path b = dir.resolve("b.smithy");
writeFile(b, "$version: \"2.0\"\nnamespace example\nstring A\nstring B\n"); // Added B.

RunResult result = IntegUtils.run(dir, ListUtils.of(
"diff",
"--old", a.toString(),
"--new", b.toString(),
"--severity", "NOTE")); // Note that this is required since the default severity is WARNING.
assertThat(result.getExitCode(), equalTo(0));
assertThat(result.getOutput(), containsString("── DIFF NOTE ──"));
});
}

private void writeFile(Path path, String contents) {
try {
FileWriter fileWriter = new FileWriter(path.toString());
PrintWriter printWriter = new PrintWriter(fileWriter);
printWriter.print(contents);
printWriter.close();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private static List<String> createSmithyCommand(List<String> args) {
throw new RuntimeException("No SMITHY_BINARY location was set. Did you build the Smithy jlink CLI?");
}

private static void withTempDir(String name, Consumer<Path> consumer) {
static void withTempDir(String name, Consumer<Path> consumer) {
try {
Path path = Files.createTempDirectory(name.replace("/", "_"));
try {
Expand Down
6 changes: 4 additions & 2 deletions smithy-cli/src/main/java/software/amazon/smithy/cli/Cli.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ public int run(String[] args) {
Command.Env env = new Command.Env(colorFormatter, stdoutPrinter, stderrPrinter, classLoader);
return command.execute(arguments, env);
} catch (Exception e) {
stdoutPrinter.flush();
stderrPrinter.flush();
printException(e, standardOptions.stackTrace());
throw CliError.wrap(e);
} finally {
Expand Down Expand Up @@ -112,13 +114,13 @@ public void stderr(CliPrinter stderrPrinter) {
private void printException(Throwable e, boolean stacktrace) {
try (ColorBuffer buffer = ColorBuffer.of(colorFormatter, stderrPrinter)) {
if (!stacktrace) {
colorFormatter.println(stderrPrinter, e.getMessage(), Style.RED);
colorFormatter.println(stderrPrinter, e.getMessage(), ColorTheme.ERROR);
} else {
StringWriter writer = new StringWriter();
e.printStackTrace(new PrintWriter(writer));
String result = writer.toString();
int positionOfName = result.indexOf(':');
buffer.print(result.substring(0, positionOfName), Style.RED, Style.UNDERLINE);
buffer.print(result.substring(0, positionOfName), ColorTheme.ERROR);
buffer.println(result.substring(positionOfName));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
/**
* Handles text output of the CLI.
*/
@FunctionalInterface
public interface CliPrinter extends Appendable, Flushable {

@Override
Expand All @@ -38,12 +37,7 @@ default CliPrinter append(CharSequence csq) {
}

@Override
default CliPrinter append(CharSequence csq, int start, int end) {
for (int i = start; i < end; i++) {
append(csq.charAt(i));
}
return this;
}
CliPrinter append(CharSequence csq, int start, int end);

/**
* Prints text to the writer and appends a new line.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ public static ColorBuffer of(ColorFormatter colors, Appendable sink) {
*/
public static ColorBuffer of(ColorFormatter colors, CliPrinter sink) {
StringBuilder buffer = new StringBuilder();
return new ColorBuffer(colors, buffer, s -> {
sink.append(s.toString());
});
return new ColorBuffer(colors, buffer, s -> sink.append(s.toString()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.cli;

/**
* Standardizes on colors across commands.
*/
public final class ColorTheme {

public static final Style EM_UNDERLINE = Style.of(Style.BRIGHT_WHITE, Style.UNDERLINE);
public static final Style DEPRECATED = Style.of(Style.BG_YELLOW, Style.BLACK);

public static final Style MUTED = Style.of(Style.BRIGHT_BLACK);
public static final Style EVENT_SHAPE_ID = Style.of(Style.BRIGHT_MAGENTA);
public static final Style LITERAL = Style.of(Style.CYAN);

public static final Style ERROR_TITLE = Style.of(Style.BG_RED, Style.BLACK);
public static final Style ERROR = Style.of(Style.RED);

public static final Style DANGER_TITLE = Style.of(Style.BG_MAGENTA, Style.BLACK);
public static final Style DANGER = Style.of(Style.MAGENTA);

public static final Style WARNING_TITLE = Style.of(Style.BG_YELLOW, Style.BLACK);
public static final Style WARNING = Style.of(Style.YELLOW);

public static final Style NOTE_TITLE = Style.of(Style.BG_CYAN, Style.BLACK);
public static final Style NOTE = Style.of(Style.CYAN);

public static final Style SUPPRESSED_TITLE = Style.of(Style.BG_GREEN, Style.BLACK);
public static final Style SUPPRESSED = Style.of(Style.GREEN);

public static final Style SUCCESS = Style.of(Style.GREEN);

public static final Style DIFF_TITLE = Style.of(Style.BG_BRIGHT_BLACK, Style.WHITE);
public static final Style DIFF_EVENT_TITLE = Style.of(Style.BG_BRIGHT_BLUE, Style.BLACK);

private ColorTheme() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public ClassLoader classLoader() {
return classLoader == null ? getClass().getClassLoader() : classLoader;
}

public void flush() {
stderr.flush();
stdout.flush();
}

public Env withClassLoader(ClassLoader classLoader) {
return classLoader == this.classLoader ? this : new Env(colors, stdout, stderr, classLoader);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void print(ColorFormatter colors, CliPrinter printer) {
LineWrapper builder = new LineWrapper(maxWidth);

builder.appendWithinLine("Usage: ")
.appendWithinLine(colors.style(name, Style.BRIGHT_WHITE, Style.UNDERLINE))
.appendWithinLine(colors.style(name, ColorTheme.EM_UNDERLINE))
.space();

// Calculate the column manually to account for possible styles interfering with the current column number.
Expand Down Expand Up @@ -168,13 +168,13 @@ public void print(ColorFormatter colors, CliPrinter printer) {

private void writeArgHelp(ColorFormatter colors, LineWrapper builder, Arg arg) {
if (arg.longName != null) {
builder.appendWithinLine(colors.style(arg.longName, Style.YELLOW));
builder.appendWithinLine(colors.style(arg.longName, ColorTheme.LITERAL));
if (arg.shortName != null) {
builder.appendWithinLine(", ");
}
}
if (arg.shortName != null) {
builder.appendWithinLine(colors.style(arg.shortName, Style.YELLOW));
builder.appendWithinLine(colors.style(arg.shortName, ColorTheme.LITERAL));
}
if (arg.exampleValue != null) {
builder.space().appendWithinLine(arg.exampleValue);
Expand Down Expand Up @@ -213,13 +213,13 @@ String toShortArgs(ColorFormatter colors) {
StringBuilder builder = new StringBuilder();
builder.append('[');
if (longName != null) {
builder.append(colors.style(longName, Style.YELLOW));
builder.append(colors.style(longName, ColorTheme.LITERAL));
if (shortName != null) {
builder.append(" | ");
}
}
if (shortName != null) {
builder.append(colors.style(shortName, Style.YELLOW));
builder.append(colors.style(shortName, ColorTheme.LITERAL));
}
if (exampleValue != null) {
builder.append(' ').append(exampleValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ public void publish(LogRecord record) {
if (isLoggable(record)) {
String formatted = getFormatter().format(record);
if (record.getLevel().equals(Level.SEVERE)) {
colors.println(printer, formatted, Style.RED);
colors.println(printer, formatted, ColorTheme.ERROR);
} else if (record.getLevel().equals(Level.WARNING)) {
colors.println(printer, formatted, Style.YELLOW);
colors.println(printer, formatted, ColorTheme.WARNING);
} else {
printer.println(formatted);
}
Expand Down
32 changes: 32 additions & 0 deletions smithy-cli/src/main/java/software/amazon/smithy/cli/Style.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

package software.amazon.smithy.cli;

import java.util.StringJoiner;

/**
* Colors and styles for use with {@link AnsiColorFormatter} or {@link CliPrinter}.
*
Expand Down Expand Up @@ -67,15 +69,45 @@ public interface Style {

String getAnsiColorCode();

static Style of(Style... styles) {
if (styles.length == 1) {
return styles[0];
} else {
return new Multi(styles);
}
}

final class Constant implements Style {
private final String ansiColorCode;

public Constant(String ansiColorCode) {
this.ansiColorCode = ansiColorCode;
}

@Override
public String getAnsiColorCode() {
return ansiColorCode;
}
}

final class Multi implements Style {
private final String ansiStyle;

public Multi(Style... styles) {
if (styles.length == 1) {
ansiStyle = styles[0].getAnsiColorCode();
} else {
StringJoiner joiner = new StringJoiner(";");
for (Style style : styles) {
joiner.add(style.getAnsiColorCode());
}
ansiStyle = joiner.toString();
}
}

@Override
public String getAnsiColorCode() {
return ansiStyle;
}
}
}
Loading

0 comments on commit f00caa9

Please sign in to comment.