Fix more color/ansi issues #5998
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In general, could we also try to address #5269? |
|
I verified that colors now appear on travis without having to set any system properties. |
It is currently the case that all tagged log messages going through a ConsoleAppender have color codes added and then subsequently stripped out. This didn't work well in combination with -Dsbt.color=true and -Dsbt.ci=true because the -Dsbt.color=true was causing ConsoleAppender.formatEnabledInEnv to be set to true which caused ansi codes to be enabled. Travis CI supports color codes but not other ansi escape sequences (this is also often true of embedded shells like in intellij or jedit). This commit reworks the ConsoleAppender so that we only add colors if useFormat is enable dand only add ClearScreenAfterCursor if ansi codes are supported. It also reworks the ConsoleAppender.write method so that if useFormat is true but ansiCodesSupported is false that we only remove the non color related ansi escape sequences so that colors remain.
The ConsoleAppender formatEnabledInEnv field was being used both as an indicator that ansi codes were supported and that color codes are enabled. There are cases in which general ansi codes are not supported but color codes are and these use cases need to be handled separately. To make things more explicit, this commit adds isColorEnabled and isAnsiSupported to the Terminal companion object so that we can be more specific about what the requirements are (general ansi escape codes or just colors). There are a few cases in ConsoleAppender itself where formatEnabledInEnv was used to set flags for both color and ansi codes. When that is the case, we use Terminal.isAnsiSupported because when that is true, colors should at least work but there are terminals that support color but not general ansi escape codes.
Running sbt with -Dsbt.supershell=true and -Dsbt.color=true doesn't work well because the isAnsiSupported flag is set to false when -Dsbt.color=false. This causes the processing that ensures that supershell lines do not get interleaved with normal log lines to be skipped. To fix this, we can enable ansi codes when supershell is explicitly enabled.
When using the DumbTerminal, no input is successfully read. I must not have actually tested this when I added it.
Disabling virtual io prevents the thin client from working. It is possible that a user may manually start a server with -Dsbt.color=false but still want to connect a thin client.
This channge should turn color on by default in ci builds (see #5269). It can still be disabled with -Dsbt.color=false.
|
FTR, the colors have reappeared in sbt's ci in the build that dogfoods sbt with a publishlocal: https://travis-ci.org/github/sbt/sbt/jobs/737809980 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes #5995. It also does some additional refactoring to ensure that color can be enabled independently from general ansi codes. I tested that color codes and newlines were displayed on travis ci in github.com/swoval/swoval if I published a local version of sbt with these changes in the ci build. Using 1.4.1 reproduced the issue in #5995. I also tested manually that the sbt interactive shell worked ok with different variations of -Dsbt.color, -Dsbt.log.format and -Dsbt.supershell.