On Mon, Aug 28, 2023 at 1:07 AM Piotr P. Karwasz <piotr.karw...@gmail.com> wrote: > > Hi Gary, > > On Thu, 22 Jun 2023 at 12:05, Gary Gregory <garydgreg...@gmail.com> wrote: > > But is it enabled in cmd.exe? Windows Terminal is a separate download, at > > least on Windows 10. > > > > Gary > > > > On Thu, Jun 22, 2023, 03:55 ppkarwasz (via GitHub) <g...@apache.org> wrote: > > > ppkarwasz opened a new issue, #1529: > > > URL: https://github.com/apache/logging-log4j2/issues/1529 > > > > > > Since microsoft/terminal#2824 enabled ANSI escape sequence processing > > > on all newer version of Windows 10, we could probably drop our JANSI > > > support. > > > > > > At the moment JANSI support is broken anyway, since the upgrade to > > > JANSI 2.x. > > I have rechecked this fact and you are right `cmd.exe` by default does > not accept ANSI escape sequences. > > Due to this and our lack of support for Jansi 2.x, I have drafted a > new proposal, by creating an `AnsiSupport` class[1] and three > subclasses to abstract the two operations we require from Jansi: > > 1. If the user sets `direct` to true in the console appender, we'll > use `AnsiConsole.out()/err()` directly (I have checked the source code > and these streams bypass `System.out/err`). This should be the > preferred way to use Jansi, since in 2.x they introduced a lot of > features: in an MSYS2/Cygwin terminal on Windows and on all UNIX-es > Jansi only translates the colors if required; on `cmd.exe` it switches > virtual terminal emulation on and only if it fails falls back to > emulating control sequences, > 2. If `direct="false"` (default) and the user wants to use > `System.out/err`, we'll wrap it in a Jansi `AnsiOutputStream`. For > Jansi 1.x this is straightforward, but in Jansi 2.x they removed the > wrapping method, so we need to copy some (ASL licensed) code from > Jansi to have the same behavior. It is messy and I would gladly drop > it, if the default value of `direct` wasn't false. > > My proposition is to: > * keep these classes as they are in `2.x` (modulo name change or > switching from an abstract class to an interface), > * split the two subclasses that use Jansi into a `log4j-core-jansi` > module in `3.x` and detect their presence using plugins or > ServiceLoader. The module would have a non-optional dependency on > `jansi` and I think we could drop support for Jansi 1.x here. > > What do you think?
Piotr, Thank you for digging in and writing up this plan. And it sounds like a nice plan :-) I'm most interested in getting Jansi 2 support in 2.x. Your 3.x plan seems fine from your description, I've not looked at this code in a long time! ;-) Gary > > Piotr > > PS: I think in `3.x` we should switch the default mode of the console > appender to `direct`, which gives a considerable performance boost. > > [1] > https://github.com/ppkarwasz/logging-log4j2/blob/jansi/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/internal/AnsiSupport.java