-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[MNG-8535] Ability to capture stdout with maven-executor #2057
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.
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
Conversation
| try (Invoker invoker = createInvoker()) { | ||
| return invoker.invoke(parseArguments(args)); | ||
| ParserRequest.Builder parserRequestBuilder = parserRequestBuilder(args); | ||
| if (in != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't those checks useless ?
It's definitely no big deal, but
ParserRequest parserRequestBuilder = parserRequestBuilder(args)
.in(in).out(out).err(err)
.build();
|
|
||
| protected void doConfigureWithTerminal(C context, Terminal terminal) { | ||
| context.terminal = terminal; | ||
| MessageUtils.setColorEnabled(Objects.requireNonNullElseGet( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have two calls to MessageUtils.setColorEnabled ? Shouldn't only one be sufficient ?
| try { | ||
| if (r.stdoutConsumer().isPresent() | ||
| || r.stderrConsumer().isPresent()) { | ||
| System.setProperty("org.jline.terminal.output", "out"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have JLine in the classpath, right ? Else TerminalBuilder.PROP_OUTPUT would be more appropriate.
| .jansi(false) | ||
| .jna(false) | ||
| .jni(true) | ||
| .name("Maven"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may still want to have dumb(true).
For example if we are on an unsupported os/architecture combination, we'll end up with a dumb terminal, but with an annoying warning (and nothing the user can do)...
Currently JAnsi always goes for system out via Process file handle. --- https://issues.apache.org/jira/browse/MNG-8535
|
This PR is superseded by several others. Dropping this one. |
|
Resolve #9907 |
WIP
Needs jline/jline3#1160
https://issues.apache.org/jira/browse/MNG-8535