Skip to content

SOLR-18281 Make PackageManager testable by printing to console through ToolRuntime #4502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
janhoy merged 2 commits into from
Jun 7, 2026
+80 −79
Merged

SOLR-18281 Make PackageManager testable by printing to console through ToolRuntime #4502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
907540b
SOLR-18281 Make PackageManager testable by printing to console throug…
janhoy Jun 6, 2026
4d74844
SOLR-18281: Remove PackageUtils.print() and route remaining output th…
janhoy Jun 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions solr/core/src/java/org/apache/solr/cli/CLIUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ private CLIUtils() {}

public static String YELLOW = "\u001B[33m";

public static String RESET = "\u001B[0m";

@dsmiley dsmiley Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are these codes? I have no clue here

@dsmiley dsmiley Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the implication of my question is we need a comment with a URL

@janhoy janhoy Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are ANSI terminal codes, a standard way to provide colored terminal output. For some reason Package manager uses colored output 🙈

@epugh epugh Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we did the last pass of the CLI, this jumped out at me as something to rethink in the future. Maybe as part of the picocli migration we drop this (or double down and embrace it everywhere!).

@janhoy janhoy Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. I'm also sure there exists a java library for TUI (Terminal User Interface) hanlding, if we want to properlty handle color, user input etc.

But I think perhaps we instead should stick to plain ASCII output as you say... Let's remember to cycle back to this..


private static final long MAX_WAIT_FOR_CORE_LOAD_NANOS =
TimeUnit.NANOSECONDS.convert(1, TimeUnit.MINUTES);

Expand Down
4 changes: 1 addition & 3 deletions solr/core/src/java/org/apache/solr/cli/SolrCLI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,8 @@ public static void printRed(Object message) {
}

public static void print(String color, Object message) {
String RESET = "\u001B[0m";

if (color != null) {
CLIO.out(color + message + RESET);
CLIO.out(color + message + CLIUtils.RESET);
} else {
CLIO.out(String.valueOf(message));
}
Expand Down
10 changes: 10 additions & 0 deletions solr/core/src/java/org/apache/solr/cli/ToolRuntime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ public abstract class ToolRuntime {

public abstract void println(String message);

/** Print an error message, highlighted in red on terminals. */
public void printError(String message) {
println(CLIUtils.RED + message + CLIUtils.RESET);
}

/** Print a success message, highlighted in green on terminals. */
public void printSuccess(String message) {
println(CLIUtils.GREEN + message + CLIUtils.RESET);
}

/** Invokes {@link System#exit(int)} to force the JVM to immediately quit. */
@SuppressForbidden(reason = "That's the only method in CLI code where we allow to exit the JVM")
public void exit(int status) {
Expand Down
Loading
Loading