reubeno / brush

bash/POSIX-compatible shell implemented in Rust
MIT License
20 stars 4 forks source link

`crossterm` error (via `reedline`) causes `pexpect` et al. not to work with `brush` #230

Open reubeno opened 5 days ago

reubeno commented 5 days ago

This looks to be the same as https://github.com/nushell/reedline/issues/594.

Looks like reedline calls into crossterm to query the current cursor position; crossterm, in turn, implements that by sending a VT terminal code and waiting a limited period of time for the terminal emulator to detect the code and respond with encoded cursor info. When the duration elapses without a response, as happens with pexpect and other simple terminal "hosts", an error percolates up and brush is unable to proceed.

brush can be run with the basic input backend, but then features like tab completion can't be tested in those environments. Would be great to figure out how to make this work for testing purposes (e.g., to run bash-completion integration test suite).

39555 commented 4 days ago

Can we test with a tty? For example spawn the shell with https://docs.rs/portable-pty/0.4.0/portable_pty/ it doesn't work(

reubeno commented 4 days ago

@39555 -- I think the issue is going to be with any pty/tty where b"\x1B[6n" isn't handled. Here's the code in question in crossterm's read_position_raw():

https://github.com/crossterm-rs/crossterm/blob/b056370038416584746fd59f7576ecd218b29f8d/src/cursor/sys/unix.rs#L35C5-L35C35

Ideally it would be great if reedline could provide a degraded experience when read_position* fail. I don't know how feasible that is, though.

At worst, we could fallback to testing brush with --input-backend=basic, but the basic input backend (which doesn't use reedline) doesn't support any standard readline behavior and doesn't support tab completion.

39555 commented 4 days ago

@reubeno what about using Alacritty to test interactive usage? Than it would be the same experience. Allacritty can be used as a library without a GUI, specifically alacritty_terminal crate, there are Term struct and tty::new, it can open a pty and can process various events. I don't know will it work or not but we can try

reubeno commented 4 days ago

I'm open to us pursuing creative ideas like the one you suggest--and I also still want to see us address the underlying limitation inherited from crossterm/reedline.

As a specific motivating example, I looked into whether we could run the bash-completion project's pytest-based test suite. Thus far, bash-completion has been pretty fantastic at finding compatibility bugs in brush, and I think it would be great for us to target getting the suite fully passing. It's what caused me to file this issue, though; those tests rely on pexpect and fail out extremely early on brush due to this problem. I'd wager there could be other existing OSS projects with similar tests that could be useful to leverage, but which fall into the same category.

(As an aside, we already are using expectrl for limited interactive tests in this project, but we had to select the basic input backend for the reasons described above.)