Open AlbertoGP opened 4 years ago
Thanks for the contribution!
I was previously ambivalent about shipping a glsp
CLI, but after giving it more thought, I'm actually quite keen on the idea. It would be useful for teaching, convenient for testing, and consistent with other scripting languages. Rather than implementing it as a separate crate, it could be a [[bin]]
target of the glsp
crate, making it possible to simply type cargo install glsp
.
In the future, we could also encourage engine bindings to ship a similar CLI - for example, a glsp-ggez
CLI which works similarly to LÖVE.
Unfortunately, I'm encountering a bug which is preventing me from testing this pull request. When running on Windows 10, MSYS2 Mingw 64-bit console, the output is:
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 6, kind: Other, message: "The handle is invalid." }', src/main.rs:127:15
I get the same error when running a trivial crate which just calls linefeed::Interface::new("test")
. The error doesn't occur when running from Windows' cmd.exe
.
Would it be possible for you (or anybody else) to install MSYS2 and see whether you can replicate the bug? If the bug is consistent, I'm afraid I won't be able to take this pull request further unless the linefeed
dependency is refactored out, or the underlying bug in linefeed
is fixed.
I’ll look into it, will have to start up a Windows machine. We could switch to another CLI library instead of linefeed: I just used it because I’m familiar with it from evaluating and customizing Ketos, another Rust LISP: https://github.com/murarth/ketos I’ve had a good experience with both Ketos and linefeed, but I normally work on Linux. The Windows and MacOS machines get started only for testing my work for customers.
I can reproduce it reliably. The problem happens only when running inside the Cygwin console: when running under PowerShell or CMD.EXE as you mention, it works fine. MSYS2 “is an independent rewrite of MSYS, based on modern Cygwin (POSIX compatibility layer) and MinGW-w64”.
I’ve filed an issue with linefeed’s author: https://github.com/murarth/linefeed/issues/68
The next step is to see whether other readline-style libraries for Rust have the same issue.
This might be a widespread problem: I’m checking alternatives, and for instance in rustyline it says:
Note: Mintty (Cygwin/MinGW) is not supported
Others don’t specify that. Two of them, Terminal Wizardry and Runtastic Prompt mention only that they support Windows, the others don’t say even that.
Here are the problems with supporting Mintty in rustyline from their issue tracker:
There are at least two problems implementing the
Term
trait (mainly the input part):
- check
is_stdin_tty
- and activate
raw
mode.Under Mintty, you cannot use the Windows API anymore.
https://github.com/kkawakam/rustyline/issues/203#issuecomment-468934002
After some experimentation today, it looks like the lua
, ruby
and python
REPLs are all pretty badly broken under Mintty as well. Seems to be a known problem. They're more full-featured than bare stdin, but they don't support many of readline
's features. Even commands like PageUp and Ctrl+Left aren't reliably supported.
I still think that either finding or fixing a readline
crate is the way to go, but we definitely shouldn't be perfectionist about the quality of the REPL under Mintty. I'll be happy as long as it compiles and runs, with support for backspace, the arrow keys, Ctrl+C and Ctrl+D.
I’ve bee looking into this. The problem is that the application would need to check at run time whether it’s in Mintty, and if so use a different code path, either for each line, or duplicate the whole loop.
You could have two binaries, one for Mintty and one for everyone else, but I doubt that’s what you want.
Does it work if you launch it with winpty?
This would effectively be the same as having a custom binary for Mintty, only simpler, with a shell script with winpty glsp "$@"
.
Just tried out a minimal rustyline
program. Can confirm that it's mostly unusable under Mintty and partially broken under winpty
.
Unfortunately, it looks like winpty
isn't installed by default with MSYS2, so we can't rely on that.
I would be very averse to any workaround which requires manual intervention from the user. Spinning up a REPL is likely to be the very first thing that many new users try - making sure that it "just works" is very high priority.
MinGW is a first-class supported platform for Rust, and Mintty is the default shell for a new MinGW installation, so unfortunately we probably can't just ignore it.
I'm inclined to let this pull request block on murarth/linefeed#68 and kkawakam/rustyline#203 for the time being. It doesn't seem like this issue is fundamentally unfixable (?), and it would be a shame to waste a lot of effort on a workaround if it ends up being patched in a dependency. If both issues stay inactive for a while, I'll try to find the time to figure out a patch myself.
I agree on the importance of “just works”.
The way things are is fine for my own use, but it would be a pity to leave this hanging. I might have a way of solving it, and you could save me some time if you go through the readline-style libraries in https://crates.io/search?q=readline and find one that works in MSYS2, even (actually preferably, as it’s more likely to work) if it’s the bare minimum functionality you need.
Could you do that?
I suspect that if we find one that doesn’t support Windows, that would be the one that works on Mintty.
Funnily enough, I was testing linenoise-rust
when I received your post. (Unfortunately, it's similarly broken.)
Between us, so far we've tested GNU readline
(indirectly by running lua
), rustyline
, a linenoise
binding, and linefeed
.
promptly
seems to delegate to rustyline
, and rprompt
is extremely bare-bones.
liner
relies on termion
, which is currently failing to compile on my machine (?).
termwiz
doesn't detect any input at all.
Looks like this problem hasn't already been solved for us, sadly.
I'm confused by the fact that, even in the C ecosystem, a working readline
library for Mintty just doesn't seem to exist. It's making me a little worried that this might be an unfixable problem after all.
Yes, I suspect that if it was solvable, someone would have done it. But it involves dealing with layered hacks accumulated over decades, and at some point you just can’t make something work with all of them at the same time.
I’m still trying to find some way, just for the challenge.
Wait, does rprompt
actually work? Barebones is enough for now.
Edit: ah, no, it’s just std::io::stdin().read_line(...)
. Now that’s barebones! :-D
I’ve finished the refactoring needed to implement a back-up REPL method: if the primary (linefeed
) fails, instead of crashing the program switches to a degraded method.
I was hoping that this degraded method would be some simpler library that worked with Mintty, but since we haven’t found any yet I’ve put there a simple stdin reader. No cursor, history or anything else, just a glass teletype, but it’s better than nothing.
Additionally, now it works perfectly well inside Emacs’ shell mode, except for retrieving the command history because Emacs does it own handling of the cursor, but anything you enter through it still gets stored and can be retrieved later in a console session. I’ve also tested it in MacOS X to be sure, and it works fine as expected.
The idea is to squash the two commits into one later, but for now I thought it would be good to have them separate for further testing.
Rather than implementing it as a separate crate, it could be a
[[bin]]
target of theglsp
crate, making it possible to simply typecargo install glsp
.
Ketos, that other Rust LISP, is made like that but I changed it in my local copy because it has the problem of bringing in all the dependencies even if you just want to use it as a library. For Ketos, I tried both having the library crate inside the CLI, and the CLI inside the library, and I tend to prefer CLI inside library because it allows having several User Interfaces: we could have cli-repl, then gtk-repl, http-repl, etc. while keeping their dependencies separate. It also makes it very easy to fork an existing UI to build a new one: just duplicate the directory with a different name, in a different repo if you want just by changing the glsp dependency in Cargo.toml.
I really appreciate the work you've done so far, and I don't want to discourage you, but I just want to clarify something before you put too much effort into this:
No cursor, history or anything else, just a glass teletype, but it’s better than nothing.
I'm not certain that a raw-stdin REPL would be better than nothing. In other words, it might be a blocker to merging this pull request.
Raw stdin on Mintty is unusable, even more so than for other terminal emulators, because Mintty has a malfunctioning text cursor. The arrow keys, and PageUp/PageDown, all just move the cursor to random locations and trash the text buffer.
I don't have access to information about how many Rust game developers are using Mintty, like me. If it were 1% of GameLisp's audience, I'd be happy to merge this, but if it were 20%, it would have too much of an impact on peoples' first opinion of the language. There's a danger that the Mintty usage might be surprisingly high, because game developers are more likely to develop on Windows. I think Git Bash uses Mintty, too.
On the other hand, WSL seems to be getting more and more popular since I set up my current toolchain... so we genuinely have no way of knowing how many people might be inconvenienced by this 🙃. The only statistic I can find is that, in the 2019 Rust developer survey, 24% of all Rust development occurred on Windows. Can't extrapolate very much from that.
has the problem of bringing in all the dependencies even if you just want to use it as a library
That's unfortunate. Looks like there's an RFC that would fix this, but it's currently stalled.
I really appreciate the work you've done so far, and I don't want to discourage you, but I just want to clarify something before you put too much effort into this:
No cursor, history or anything else, just a glass teletype, but it’s better than nothing.
I'm not certain that a raw-stdin REPL would be better than nothing. In other words, it might be a blocker to merging this pull request.
Yes, I see that and I’ve been in similar situations before. The raw-stdin REPL is indeed unusable in a console, we agree it’s not a solution for that.
It is better than nothing in the sense that frontends that do their own buffer handling before sending it to stdin can access it in that way. And I think that the current message and switching to raw-stdin gives out a better impression than crashing.
As for effort, that’s no problem at all. Just wanted to be sure that we weren’t missing some easy solution, and by now it’s clear we have exhausted our leads so I’ll let it rest for now.
A little more research:
Cygwin, Mintty and MinGW recently migrated to the brand new ConPTY API on Windows 10, which should make Mintty into a more full-featured terminal emulator. This feature was disabled by default a month ago because it was "just a bit too buggy", and it's the topic of ongoing discussion. It can be experimentally re-enabled by running MSYS=enable_pcon mintty
. When enabled, it does seem to fix raw stdin, although rustyline
and GNU readline
are still broken.
Hopefully this feature will be enabled by default soon enough, in which case we'll be able to move ahead without any changes on our end. Documenting the need to update to a bleeding-edge version of Mintty will be a pain, but it's not the end of the world. It's possible to manually detect whether or not we're running in Mintty, and even detect Mintty's version, so we could potentially just print a warning message at startup.
I have found one other lead: the lua
5.3.5 REPL works well enough to be usable in Mintty, even without MSYS=enable_pcon
. This is baffling, because the full source code to that REPL is here, and it's not doing anything special. When I compile an equivalent C program myself, calling all of the same stdio or readline APIs, it's broken. Might be worth looking into.
Aha - looks like lua
is cheating. The MSYS package manager installs winpty
as a dependency, and creates a shell script which invokes winpty lua.exe
.
It might be possible for us to embed winpty
into glsp
's CLI - the API is pretty well-documented here and here. A starting point might be looking at the source code of alacritty
, or the winpty
command-line program.
Alternatively, we could try to call Windows' ConPTY API ourselves.
While we figure this out, it turns out that putting the REPL in the top workspace makes at least cargo run
just work from the root, but you can still run cargo build
from glsp/
without fetching the REPL dependencies, or use cargo build --package glsp
from the root with the same effect.
This is just a simple read-eval-print loop without hot reloading or stack trace manipulation: I just noticed #8 but decided to submit this anyway because it is already useful for me and might be for others too, until the real REPL is ready. If this isn’t approprite in-tree that’s no problem because it’s trivial to make it load the
glsp
crate from crates.io instead of the parent directory.