Problem
-------
Console.systemOut is hooked up to Terminal.get, which internally calls
ProxyTerminal, which lets us deffer the wiring of terminal to
activeTerminal. This mechanism allows us to swap out the terminal
capable of standard out forwarding for sbtn.
However, as it stands this breaks the contract of being able to use
Console.systemOut with wrapped inside of `Terminal.withStreams() {...}`.
Solution
--------
Check if `activeTerminal.get` returns `null`, and if so initialize it to
the conventional `Terminal.SimpleTerminal`, which behaves as expected.
There are cases where sbt will incorrectly shutdown if the jline reader
is interrupted while filling the input buffer. To fix this we can throw
an InterruptedException instead of a ClosedException.
The repro for this was start `sbt`, input `~compile` and while sbt was
starting up, open a source file with vim using the metals bsp
integration. sbt server would end up shutting down everytime after a
single compilation iteration.
With the latest sbt code, the `last` command displayed all of the output
without line separators. This occurred because the logic for appending
bytes to System.out split the input bytes on the line separator but if
there was nothing but the line separator in the input bytes then the
result was empty.
When there are multiple console appenders for a logger, we inadvertently
evaluated the message thunk for each appender which would cause side
effects to be unexpectedely evaluated multiple times.
Fixes part of #6126.
In 85d17889b6, I attempted to fix
supershell messages getting interlaced with log lines. It turned out
that that approach didn't work with windows and was causing all of the
output to bet blown away. A better approach is to check if the bytes
we're writing contain one or more line separators. If so, we can wrap
the bytes in a string and split the string into lines. Then we can
append a ClearScreenAfterCursor before every newline.
I think the problem with windows was that the ClearScreenAfterCursor was
coming between the carraige return and the newline.
The boot server socket was not working correctly when the sbt server was
started by the thin client. This was because it is necessary for us to
create a ConsoleTerminal in order for System.out and System.err to be
properly forwarded to the clients connected over the boot server socket.
As a result, if you started a server instance of sbt with the thin
client, you wouldn't see any output util you connected to the server.
The fix is to just make sure that we create a console terminal if sbt is
run as a subprocess.
We no longer need to use the forked version of jline because they have
merged in our required changes. The latest version of jline does upgrade
jansi, however, and some of the apis we were relying on for windows were
removed so they had to be manually implemented. I verified that console
input still worked on my windows vm after this change.
In 3b09ff6af7, we stopped adding a clear
screen after curser after each log line. This inadvertently caused
supershell lines to get interlaced with log lines. This can be fixed by
writing a ClearScreenAfterCursor after every newline character that we
write to stdout.
As a bonus, I had also long noticed that supershell log lines would get
interlaced with the serverTestProj/test output and this change fixes
that as well.
On terminals with virtual io disabled, we'd spin up a thread for each
watch iteration that performed a blocking read from the terminal input
stream. This thread could not be joined which would cause the triggered
execution to be delayed by 1 second while sbt blocked trying to join
that thread. It also meant that input probably didn't work correctly
since the user would end up with many threads polling from system in.
The fix to this problem is to poll the terminal input stream if it is
unsafe to do a blocking read, which is the case for dumb terminals or if
virtual io is disabled.
With sbt 1.4.x, non-ascii utf-8 characters are not handled correctly in
the console. It was not clear from the jline 3 documentation but the
NonBlockingReader.read method is supposed to return unicode points
rather than utf8 bytes. To fix this, we can decode the input and return
the code point rather than the directy byte from the input stream.
When the sbt main loop is blocked by console, any other connected client
is prompted that they can kill the task by typing cancel. The
implementation for the console task is to write some input that will
cause the console to exit because the scala 2.12 console cannot be
safely killed with an interrupt. This input, however, was being blocked
from written to the console because the console input stream was holding
the readThread lock. We can be fix this and propagate the input to the
console we wish to terminate by synchronizing on a different lock
object. This should have no impact outside of cancelling the console
because that is the only place where we call the write method of
WriteableInputStream.
If a user runs sbt -Dsbt.ci=true with the latest code, sbt immediately
exits. This was because we were passing the SimpleTerminal into jline3
and jline 3 would end up exiting immediately. Instead we can still make
a console terminal if there is a console available. An alternative
approach would have been to use a dumb terminal with -Dsbt.ci=true, but
the dumb terminal experience is not great (tab completions don't work
for example), so I thought this was a better fix.
Tab completions did not work well in sbt 1.4.x when run with
-Dsbt.color=false. This was because we were stripping a bunch of ansi
codes, which caused some problems with the jline 3 completion engine.
Instead of stripping the ansi codes, we can set the jline max_colors
capability to 1 if color is disabled. With this change, the completions
are similar to 1.3.13 except that in jline 2 the user has to hit tab
twice to see all of the available candidates while in jline 3, the
candidates are immediately printed below the prompt.
Intellij import was broken in sbt 1.4.2 because we increased the
scenarios in which virtual io is used. The problem wasn't actually
virtual io but that we create a console terminal with jline3 and that
could cause issues reading input from System.in. This can be fixed by
only creating an instance of ConsoleTerminal if a console is actually
available for the sbt process. When hasConsole is false, the
consoleTerminalHolder will point to the SimpleTerminal whose inputStream
method just reads directly from System.in. After making this change, I
verified that intellij import worked again on windows.
We also don't want to make a console terminal if it is a dumb terminal
because jline 3 ends up having problems with the ConsoleTerminal.
The sbt console didn't work with supershell disabled because setting
that parameter was causing the terminal type to be dumb which only works
in some very specific situations. When Terminal.isAnsiSupported was
false, we were setting sbt to use a dumb terminal. We really only want
to use a dumb terminal if virtual io is off. It also doesn't necessarily
make sense to automatically disable general ansi codes even if
supershell is disabled by system property.
Scalatest will check the ansiCodesSupported value of a console appender
to decide whether or not to colorize its output. We were setting
isAnsiSupported to false by default in ci to prevent the ConsoleAppender
from adding ansi codes to the output. The only place though where we
were doing that was in adding a ClearScreenAfterCursor to the end of
each log line. This was done for supershell but there is actually other
logic in supershell processing that adds these anyway.
If a user runs with -Dsbt.log.format=false or -Dsbt.log.noformat=true,
we should disable color by default. Running with -Dsbt.color=true should
make this possible.
It is possible for downstream dependencies to print or log messages
containing ansi escape sequences and/or color codes. In older versions
of sbt, these would be printed even if the user had disabled ansi codes
or color via the sbt.log.noformat or sbt.color parameters. This commit
adds a general api to EscHelpers that strips general ansi codes and
color codes independently via flags. We can then use that api to ensure
that all bytes written to System.out are stripped of ansi escape and
color codes if the terminal properties demand this.
The motivation was that JLine 3 will prepend the prompt string with
\E[?2004h, which turns on bracketed paste mode
(https://en.wikipedia.org/wiki/ANSI_escape_code). If the sbt shell is
started with a terminal that doesn't support general ansi escape codes,
such as the jEdit shell, ?2004h gets printed to the shell. To fix this,
we can strip ansi codes from all output if the terminal doesn't support
general ansic codes. This has the additional side effect of any ansi
codes that appear in log messages or printlns that are added by non-sbt
code will be stripped. It's unlikely that this is all that common.
In addition to the JLine use case, I've noticed that utest prints
colored output during test runs. Prior to this change, the colored
output was present even when sbt was run with `-Dsbt.color=false` and
after this change, the colors are correctly stripped.
For reasons I don't fully understand, it is necessary to go in and out
of raw mode to get console input to echo on windows. This was reported
as sbt new not echoing input in #5952.
I noticed that if you run `sbt -Dsbt.log.noformat=true` there are colors
printed when using the latest sbt code. When running with that property
set, the expectation is there are no colors.
When the read methods of InputStream that take an Array[Byte] as input
are called, they are supposed to return -1 if there are no bytes
available due to an EOF. Previously it was incorrectly writing the -1 as
a byte in the array and returning 1. Now it correctly returns -1.
The condition for closing the WriteableInputStream was also incorrect.
We should only close the input stream if it returns -1 in raw mode.
Fixes#5999
This channge should turn color on by default in ci builds (see
https://github.com/sbt/sbt/issues/5269). It can still be disabled with
-Dsbt.color=false.
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.
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.
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.
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.
In canonical mode, System.in will return -1 for ctrl+d on an empty line.
The result of this behavior was that if a user entered ctrl+d during run
in a task that was reading from System.in, sbt would end up exiting
whenever the task exited. This happened because the WriteableInputStream
would close itself when it read -1 from the input stream, which it
assumed meant that the underlying input stream itself had been closed.
When the jline reader tried to read from the closed
WriteableInputStream, it would throw an exception and if the line reader
was for the console channel, it would be interpreted as the user had
inputted ctrl+d in the sbt shell which is supposed to exit sbt. This
change fixes that behavior so that sbt can continue reading input after
the run task exits.
Issue #5974 reported that ctrl+d was not working as expected in the
scala 2.13.{2,3} console. This was because we were throwing an exception
whenever ctrl+d was read instead of allowing the jline line reader to
handle it. I can't remember exactly why I added the throw here but I
validated that both the sbt shell and the scala console had the expected
behavior from the comments in #5974 after this change.
When piping an sbt task to a file, the expectation is that sbt will not
write ansi control characters and colors unless the users specifies that
with, e.g. -D.sbt.color=true. With sbt 1.4.0, all output bytes are
routed through the progress output processor which tries to ensure that
progress lines are not interleaved with log lines. The issue was that
the hasProgress flag was being set to true for the server process even
when the formatEnabledInEnv flag was set to false. This caused each log
line to have a leading clear screen before cursor ansi control code
written which would appear in the output file.
When running sbt 1.4.0 with -Dsbt.ci=true and -Dsbt.color=true, there is
no color output. This was because in this scenario, a SimpleTerminal was
used and isAnsiSupported and isColorEnabled were hardcoded to false
rather than reading the values from the system properties.
The user input is not echoed in sbt new because we were switching to raw
mode when creating the console terminal. I can't quite remember why I
was entering raw mode preemptively but it doesn't seem like the best
idea.
In windows, it is necessary for the console mode to include the
ENABLE_PROCESS_INPUT flag in order for ctrl+c to be treated as a signal
rather than a character. In jline 2, the ENABLE_PROCESS_INPUT flag was
preserved whenever the console mode was changed but in jline 3, it was
not. It is easy enough to manually set the flag after entering and
exiting raw mode and setting attributes (which are the apis that change
the console mode on windows).
Eric Peters
eugene yokota
Sebastian Alfers
Adrien Piquerez
Sam Halliday
João Ferreira
Ethan Atkins