fusesource / jansi

Jansi is a small java library that allows you to use ANSI escape sequences to format your console output which works even on windows.
http://fusesource.github.io/jansi/
Apache License 2.0
1.12k stars 139 forks source link

Rework fix of terminal width support on MINGW (fixes #290) #291

Open rosti-il opened 4 months ago

rosti-il commented 2 months ago

You removed support for differentiating stdout and stderr, is there any reason for that ?

There are two main reasons:

  1. It is very unusual for a Java process to have two different terminals with two different or even equal widths for stdout and stderr.
  2. The tty.exe of MSYS2/MINGW is a trivial program. Almost all that it does is calling for the ttyname() function and then printing the returned C string to the stdout. If you really need to support different terminals for stdout and stderr the right thing to do is adding that functionality into the tty.exe (to be called for example as tty.exe -e) in the Coreutils project and this is a really trivial change.

The MingwSupport.getRedirect() method in the original code is an unnecessarily dirty hack that in most cases simply won't work, because most users will not bother to configure --add-opens java.base/java.lang=ALL-UNNAMED for all their Java software that uses JANSI and in some future version of Java that --add-opens option might simply be not supported anymore because the transition period to the strong encapsulation of the Java Platform Module System (JPMS) will be over.

gnodet commented 2 months ago

You removed support for differentiating stdout and stderr, is there any reason for that ?

There are two main reasons:

  1. It is very unusual for a Java process to have two different terminals with two different or even equal widths for stdout and stderr.

Supporting both stdout and stderr is important imho. Just try

> java -jar jansi-2.4.2-SNAPSHOT.jar | grep  terminal
isatty(STDOUT_FILENO): 0, System.out is *NOT* a terminal
isatty(STDERR_FILENO): 1, System.err is a terminal

That's a pretty common use case for java apps running in terminals.

  1. The tty.exe of MSYS2/MINGW is a trivial program. Almost all that it does is calling for the ttyname() function and then printing the returned C string to the stdout. If you really need to support different terminals for stdout and stderr the right thing to do is adding that functionality into the tty.exe (to be called for example as tty.exe -e) in the Coreutils project and this is a really trivial change.

Well, it calls ttyname on the standard input. So your code won't work if the input comes from a pipe either.

> echo foo | java -jar jansi-2.4.2-SNAPSHOT.jar 

The output should still be usable and report the correct width.

The MingwSupport.getRedirect() method in the original code is an unnecessarily dirty hack that in most cases simply won't work, because most users will not bother to configure --add-opens java.base/java.lang=ALL-UNNAMED for all their Java software that uses JANSI and in some future version of Java that --add-opens option might simply be not supported anymore because the transition period to the strong encapsulation of the Java Platform Module System (JPMS) will be over.

Then we could go to JNI ?

rosti-il commented 2 months ago

You're right, I didn't think about pipes and redirections. So the first reason is not relevant but the second still is. I mean tty.exe could be changed to support stdout and stderr in addition to the already supported stdin.

Another any maybe the best option is indeed JNI. The ttyname() and ttyname_r() libc functions of MSYS2 are available in the msys-2.0.dll. In Cygwing I believe they are available in the cygwin1.dll. Both DLLs reside in a directory that is included into PATH, meaning JANSI doesn't need to know the exact location of these DLLs. So, the already existing jansi_ttyname.c just need to support those DLLs in MSYS2 and Cygwin environments. Maybe another existing JNI code could support those DLLs as well, so JANSI could be used without running both tty.exe and stty.exe?