Text-based terminal input handling is broken on Linux where stty binary is not in /bin/

mabe02 / lanterna

Java library for creating text-based GUIs

GNU Lesser General Public License v3.0

2.23k stars 243 forks source link

Text-based terminal input handling is broken on Linux where stty binary is not in /bin/ #519

Open rsaarelm opened 3 years ago

rsaarelm commented 3 years ago

Setting the input mode is done with a system call to /bin/stty, but for example NixOS Linux has the stty binary in a different location. Calling /usr/bin/env stty instead of /bin/stty would fix this.

mabe02 commented 3 years ago

Yeah, hard-coding that like we do now is not great. I've updated 3.0/3.1 to take it from system properties, on master will try what you're suggesting.

pb- commented 3 years ago

@mabe02 Is 217bcd2 on track into a release any time soon? I'm having the same issue as OP on NixOS.

karelantonio commented 3 years ago

I have the sale problem in temux, android

karelantonio commented 3 years ago

Sorry. The same problem on termux, android

pb- commented 2 years ago

@mabe02 I just stumbled over the problem again. Looking a bit more carefully, it seems that the first commit (74ce744df954120e86629f8c3295858b68a17225) made it into a release, but not the second commit (https://github.com/mabe02/lanterna/commit/217bcd24566c51f444d73fcb0e7396db29206e7a). Is that on purpose or was it just missed? The trouble is that overriding alone (first commit) won't help with fixing the issue on NixOS or any other environments where only /usr/bin/env is available.

avl42 commented 2 years ago

I don't quite understand the problem...

Is there a problem with passing that "-D..." option to the java VM on NixOS?

Finding auxiliary tools via PATH should be rather frowned upon - this doesn't matter for tools that are started locally by a user, but may hurt if such tools are to be used in restricted environments.

Anyway, the decision is on @Martin Berglund @.***>

pb- commented 2 years ago

Is there a problem with passing that "-D..." option to the java VM on NixOS?

I suppose it's possible to determine and pass the actual location of stty in a wrapper script when distributing applications using lanterna.

Finding auxiliary tools via PATH should be rather frowned upon - this doesn't matter for tools that are started locally by a user, but may hurt if such tools are to be used in restricted environments. Anyway, the decision is on @martin Berglund @.***>

Interesting, I didn't know this should be frowned upon. Can you help me understand in what situations this would hurt? My understanding was it generally improves portability.

mabe02 commented 2 years ago

I do agree with @avl42 on this; using env variables is a bit old-school and may be somewhat unexpected and surprising to developers and "users" (if there are such a thing for this library :smiley: ) The JVM -D is definitely a better solution. That being said, I'm happy to keep this as on option, let me review the code again.

mabe02 commented 2 years ago

Sorry, too late in the day for me, I misunderstood the issue. Ignore my previous comment.

mabe02 commented 2 years ago

@pb- Ok, now I remember. The first commit was for 3.0 and 3.1 branches, because I didn't want to change the stty behavior there. You should still be able to customize it by setting the "com.googlecode.lanterna.terminal.UnixTerminal.sttyCommand" system property, which you can do either with the -D JVM argument or programmatically in the code (System.setProperty("com.googlecode.lanterna.terminal.UnixTerminal.sttyCommand", "/my/path/to/stty")).

The new version using env is on master branch only, by design. If you want to try that out, you'll need to clone the source and build it locally.

pb- commented 2 years ago

@mabe02 thanks for chiming in. I'm already successfully using a local build of master for development, but was wondering if there is a chance that https://github.com/mabe02/lanterna/commit/217bcd24566c51f444d73fcb0e7396db29206e7a will make it into a release eventually. As of now I see the following options for shipping a Lanterna-powered app that works on NixOS (besides other distros):

have a wrapper script that determines the location of stty and passes it via -D,
programmatically parse PATH in my process to find the location and set it as a property, or
include a custom build of lanterna in the release that has commit https://github.com/mabe02/lanterna/commit/217bcd24566c51f444d73fcb0e7396db29206e7a included.

None of these options appear ideal to me, hence my question of having it in a release :slightly_smiling_face:. If there is no such plan, I would resort to one of these options of course.