Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LineReader.readLine() botches lines if stdin is redirected #899

Closed
pavelrappo opened this issue Nov 7, 2023 · 6 comments
Closed

LineReader.readLine() botches lines if stdin is redirected #899

pavelrappo opened this issue Nov 7, 2023 · 6 comments

Comments

@pavelrappo
Copy link

Is this (see title) expected? For concrete examples, see this bug against OpenJDK, where System.console() is implemented using JLine: https://bugs.openjdk.org/browse/JDK-8319545

@gnodet
Copy link
Member

gnodet commented Nov 7, 2023

I may be wrong, but JLine aims to follow the terminal settings. On unix systems, the input is controlled by the input flags returned by stty -a, for me:

iflags: -istrip -icrnl -inlcr -igncr ixon -ixoff ixany imaxbel iutf8
	-ignbrk brkint -inpck -ignpar -parmrk

Those are explained at https://man7.org/linux/man-pages/man3/termios.3.html

I don't think there's an easy way to simply handle \r, \n and \r\n in an exact similar way at the moment. It should be possible to add a flag to the LineReader to toggle this behaviour.

gnodet added a commit to gnodet/jline3 that referenced this issue Nov 7, 2023
@gnodet
Copy link
Member

gnodet commented Nov 7, 2023

With the PR #900, it should be possible normalize EOL behaviour by using the following lines just after the terminal is created:

            Attributes attributes = terminal.getAttributes();
            attributes.setInputFlag(InputFlag.INORMEOL, true);
            terminal.setAttributes(attributes);

@pavelrappo could you double check if that would work for you ?

@pavelrappo
Copy link
Author

With the PR #900, it should be possible normalize EOL behaviour by using the following after the terminal is created:

            Attributes attributes = terminal.getAttributes();
            attributes.setInputFlag(InputFlag.INORMEOL, true);
            terminal.setAttributes(attributes);

@pavelrappo could you double check if that would work for you ?

Thanks for the suggestion. A colleague of mine, @naotoj, might look into this soon. I just wanted to understand if the behaviour I described in that OpenJDK bug was expected.

Even if, for whatever reason, your suggestion doesn't pan out, as a non-expert, I assume that OpenJDK could always check if JLine is actually dealing with redirection rather than TTY. If it does, we could simulate Console.readLine() by wrapping System.in or some java.io.Reader which represents stdin into java.io.BufferedReader, which actually provides readLine().

@gnodet
Copy link
Member

gnodet commented Nov 7, 2023

With the PR #900, it should be possible normalize EOL behaviour by using the following after the terminal is created:

            Attributes attributes = terminal.getAttributes();
            attributes.setInputFlag(InputFlag.INORMEOL, true);
            terminal.setAttributes(attributes);

@pavelrappo could you double check if that would work for you ?

Thanks for the suggestion. A colleague of mine, @naotoj, might look into this soon. I just wanted to understand if the behaviour I described in that OpenJDK bug was expected.

Even if, for whatever reason, your suggestion doesn't pan out, as a non-expert, I assume that OpenJDK could always check if JLine is actually dealing with redirection rather than TTY. If it does, we could simulate Console.readLine() by wrapping System.in or some java.io.Reader which represents stdin into java.io.BufferedReader, which actually provides readLine().

That won't work, as JLine kinda ignores the given streams when building a system terminal and goes straight to the file descriptors.

However, the JDK assumption that all line endings should be treated the same is, imho, a bit weird. That's a bit specific to the BufferedReader, but the PrintWriter for example uses System.lineSeparator() which is obviously platform dependant.

@pavelrappo
Copy link
Author

pavelrappo commented Nov 7, 2023

With the PR #900, it should be possible normalize EOL behaviour by using the following after the terminal is created:

            Attributes attributes = terminal.getAttributes();
            attributes.setInputFlag(InputFlag.INORMEOL, true);
            terminal.setAttributes(attributes);

@pavelrappo could you double check if that would work for you ?

Thanks for the suggestion. A colleague of mine, @naotoj, might look into this soon. I just wanted to understand if the behaviour I described in that OpenJDK bug was expected.
Even if, for whatever reason, your suggestion doesn't pan out, as a non-expert, I assume that OpenJDK could always check if JLine is actually dealing with redirection rather than TTY. If it does, we could simulate Console.readLine() by wrapping System.in or some java.io.Reader which represents stdin into java.io.BufferedReader, which actually provides readLine().

That won't work, as JLine kinda ignores the given streams when building a system terminal and goes straight to the file descriptors.

Why not? Perhaps I was unclear. What I meant was this: if Console.isTerminal(), which merely wraps POSIX isatty in not-yet-released JDK 22, returns false, we can bypass JLine entirely and go straight to System.in for everything that Console has to offer.

However, the JDK assumption that all line endings should be treated the same is, imho, a bit weird. That's a bit specific to the BufferedReader, but the PrintWriter for example uses System.lineSeparator() which is obviously platform dependant.

I don't know this for sure, so the following is my speculation. Input is different from output. BufferedReader is input and PrintWriter is output. Input, such as BufferedReader, String.lines(), java.nio.file.Files.readAllLines(java.nio.file.Path, java.nio.charset.Charset), etc., behave similarly WRT line separators. They bring data produced on an arbitrary system to Java runtime. So they must be prepared for everything.

If you output data from Java runtime, you might want to do it according to your or other's system customs. Hence, BufferedWriter does not have writeLine, String does not have joinLines and Files does not have writeAllLines. Instead, a programmer has to explicitly specify separators, in write, join and write accordingly.

PrintStream/PrintWriter are convenient output facilities, which, I assume, are typically used for stdout.

@gnodet
Copy link
Member

gnodet commented Nov 7, 2023

With the PR #900, it should be possible normalize EOL behaviour by using the following after the terminal is created:

            Attributes attributes = terminal.getAttributes();
            attributes.setInputFlag(InputFlag.INORMEOL, true);
            terminal.setAttributes(attributes);

@pavelrappo could you double check if that would work for you ?

Thanks for the suggestion. A colleague of mine, @naotoj, might look into this soon. I just wanted to understand if the behaviour I described in that OpenJDK bug was expected.
Even if, for whatever reason, your suggestion doesn't pan out, as a non-expert, I assume that OpenJDK could always check if JLine is actually dealing with redirection rather than TTY. If it does, we could simulate Console.readLine() by wrapping System.in or some java.io.Reader which represents stdin into java.io.BufferedReader, which actually provides readLine().

That won't work, as JLine kinda ignores the given streams when building a system terminal and goes straight to the file descriptors.

Why not? Perhaps I was unclear. What I meant was this: if Console.isTerminal(), which merely wraps POSIX isatty in not-yet-released JDK 22, returns false, we can bypass JLine entirely and go straight to System.in for everything that Console has to offer.

Not sure to understand. I thought the problem was when using the JLine console, i.e. mainly when isatty returns true. My understanding is that, given how JLine closely follows the terminal's behaviour, the EOL handling is the same whether the console is a real console or a fake one (i.e. wether isatty returns true or false). I agree you could bypass JLine when not running inside a real terminal, but what I was saying earlier, is that you won't be able to wrap the input stream when a real console is active, as JLine will create the InputStream on the file descriptor directly.

@gnodet gnodet closed this as completed in d733739 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants