Skip to content

Commit

Permalink
Fix for AESH-474 - Support or Windows10+ Virtual Terminal Processing
Browse files Browse the repository at this point in the history
  • Loading branch information
@jfdenise
jfdenise committed Jun 11, 2018
1 parent 371a4e1 commit 781ffbf
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 4 deletions.
26 changes: 24 additions & 2 deletions readline/src/main/java/org/aesh/readline/terminal/impl/AbstractWindowsTerminal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,27 @@
import java.util.logging.Level;
import org.aesh.terminal.tty.Signal;
import org.aesh.terminal.tty.Size;
import static org.fusesource.jansi.internal.Kernel32.GetStdHandle;
import static org.fusesource.jansi.internal.Kernel32.STD_OUTPUT_HANDLE;
import static org.fusesource.jansi.internal.Kernel32.WriteConsoleW;
import org.fusesource.jansi.internal.WindowsSupport;

abstract class AbstractWindowsTerminal extends AbstractTerminal {

private static class ConsoleOutput extends OutputStream {

private static final long console = GetStdHandle(STD_OUTPUT_HANDLE);
private final int[] writtenChars = new int[1];

@Override
public void write(int b) throws IOException {
char[] chars = new char[]{(char) b};
if (WriteConsoleW(console, chars, 1, writtenChars, 0) == 0) {

This comment has been minimized.

Sign in to view

Copy link
@gnodet

gnodet Jun 12, 2018

Contributor

While the idea of using WriteConsoleW is a good one, I think you can't do it here because what you receive is a byte, not a character. Assuming bytes are characters only work for the first 128 chars mostly, everything else will certainly be messed up at display. Try to copy Æsh in the example running on Windows and it will fail.

This comment has been minimized.

Sign in to view

Copy link
@jfdenise

jfdenise Jun 12, 2018

Author Contributor

@gnodet , thank-you. I could use Character.toChars(b); and pass down the returned char array to WriteConsoleW?

This comment has been minimized.

Sign in to view

Copy link
@gnodet

gnodet Jun 12, 2018

Contributor

Well, the problem is that a single char can be encoded with 1, 2, 3, or 4 bytes, so you'd have to use buffers and a CharsetDecoder in order to deal with underflow.

This comment has been minimized.

Sign in to view

Copy link
@jfdenise

jfdenise Jun 12, 2018

Author Contributor

The ConsoleOuput receives an int, this is one of the codepoints of the String to print. Character.toChars(b) should return the right set of chars, no?

This comment has been minimized.

Sign in to view

Copy link
@gnodet

gnodet Jun 12, 2018

Contributor

No, the integers are not codepoints, but bytes.
You have 3 different things: codepoints, chars and bytes. A codepoint represents a glyph, which is represented by one or two chars, which can be represented by one to four bytes.

In TerminalConnection, you have the following:

    stdOut = new Encoder(outputEncoding(), this::write);

    private void write(byte[] data) {
        try {
            terminal.output().write(data);
        }
        catch(IOException e) {
            LOGGER.log(Level.WARNING, "Failed to write out.",e);
        }
    }

The Encoder receives codepoints, turn them into chars and then encode them into bytes.

This comment has been minimized.

Sign in to view

Copy link
@jfdenise

jfdenise Jun 12, 2018

Author Contributor

Thank-you, I missed this conversion, and the legacy (new WindowsAnsiOutputStream(new FileOutputStream(FileDescriptor.out))) was simply writing byte[] to the fdescriptor. So we are not at the right level as you said. Seems that I couldn't decode the bytes, not knowing the encoding that has been used.
If we use WriteConsoleW the Windows terminal needs to receive the char[] directly.

This comment has been minimized.

Sign in to view

Copy link
@jfdenise

jfdenise Jun 12, 2018

Author Contributor

@gnodet , I have reworked the support, could you give it an eye? jfdenise@70fb1f6
Mainly I am providing the codepoints to the terminal in case the terminal exposes an int[] consumer (something that Windows terminal does for Windows10+).

This comment has been minimized.

Sign in to view

Copy link
@gnodet

gnodet Jun 12, 2018

Contributor

That seems to break a bit the contract of the Terminal. I would suggest you could bring back the Writer on the Terminal interface instead (it was removed by efa3b27#diff-b1a04485265b44d60a131e2a98919111) and move back the encoding to the Terminal as it was initially. That said, if it works for you...

This comment has been minimized.

Sign in to view

Copy link
@jfdenise

jfdenise Jun 12, 2018

Author Contributor

@gnodet , thank-you. The encoding has been moved to TerminalConnection, it is shared by Terminals. I will keep @stalep design there and simply add this path specific to Windows10+ terminal.
Sincere thanks for your review, it saved us quite a lot of future debugging!
throw new IOException("Failed to write to console: " + WindowsSupport.getLastErrorMessage());
}
}

}
private static final int PIPE_SIZE = 1024;

protected static final int ENABLE_PROCESSED_INPUT = 0x0001;
Expand All @@ -65,17 +83,21 @@ abstract class AbstractWindowsTerminal extends AbstractTerminal {

private volatile boolean closing;

public AbstractWindowsTerminal(String name, boolean nativeSignals, SignalHandler signalHandler) throws IOException {
this(null, name, nativeSignals, signalHandler);
}

public AbstractWindowsTerminal(OutputStream output, String name, boolean nativeSignals, SignalHandler signalHandler) throws IOException {
super(name, "windows", signalHandler);
PipedInputStream input = new PipedInputStream(PIPE_SIZE);
this.slaveInputPipe = new PipedOutputStream(input);
this.input = new FilterInputStream(input) {};
this.output = output;
this.output = output == null ? new ConsoleOutput() : output;
String encoding = getConsoleEncoding();
if (encoding == null) {
encoding = Charset.defaultCharset().name();
}
this.writer = new PrintWriter(new OutputStreamWriter(output, encoding));
this.writer = new PrintWriter(new OutputStreamWriter(this.output, encoding));
// Attributes
attributes.setLocalFlag(Attributes.LocalFlag.ISIG, true);
attributes.setControlChar(Attributes.ControlChar.VINTR, ctrl('C'));
Expand Down
26 changes: 24 additions & 2 deletions readline/src/main/java/org/aesh/readline/terminal/impl/WinSysTerminal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,22 @@
import org.aesh.terminal.tty.Size;
import org.fusesource.jansi.WindowsAnsiOutputStream;
import org.fusesource.jansi.internal.Kernel32;
import static org.fusesource.jansi.internal.Kernel32.GetStdHandle;
import org.fusesource.jansi.internal.Kernel32.INPUT_RECORD;
import org.fusesource.jansi.internal.Kernel32.KEY_EVENT_RECORD;
import static org.fusesource.jansi.internal.Kernel32.STD_OUTPUT_HANDLE;
import org.fusesource.jansi.internal.WindowsSupport;

public class WinSysTerminal extends AbstractWindowsTerminal {

private static final int VIRTUAL_TERMINAL_PROCESSING = 0x0004;

public WinSysTerminal(String name, boolean nativeSignals) throws IOException {
this(name, nativeSignals, SignalHandlers.SIG_DFL);
}

public WinSysTerminal(String name, boolean nativeSignals, SignalHandler signalHandler) throws IOException {
super(new WindowsAnsiOutputStream(new FileOutputStream(FileDescriptor.out)),
name, nativeSignals, signalHandler);
super(setVTMode() ? null : new WindowsAnsiOutputStream(new FileOutputStream(FileDescriptor.out)), name, nativeSignals, signalHandler);
}

protected int getConsoleOutputCP() {
Expand Down Expand Up @@ -123,4 +126,23 @@ protected byte[] readConsoleInput() {
return sb.toString().getBytes();
}

// This allows to take benefit from Windows 10+ new features.
private static boolean setVTMode() {
long console = GetStdHandle(STD_OUTPUT_HANDLE);
int[] mode = new int[1];
if (Kernel32.GetConsoleMode(console, mode) == 0) {
// No need to go further, not supported.
return false;
}
if (Kernel32.SetConsoleMode(console, mode[0] | VIRTUAL_TERMINAL_PROCESSING) == 0) {
// No need to go further, not supported.
return false;
}

return true;
}

public static boolean isVTSupported() {
return setVTMode();
}
}

0 comments on commit 781ffbf

Please sign in to comment.