Use termios from termios crate

[nospam3089]

The termios interface from the nix crate is lacking a thought through design. Severely broken on at least one platform. Please see #2071 for details.

The termios crate stays closer to the C API and thus both works with illumos and reduces the risk of bugs on other platforms.

[gwenn]

As far as possible, we try to reduce the number of dependencies (see #613, #614, #615)
And are you sure that correctly supported platforms by termios is greater than nix ones.
I mean that termios may work on illumos but not on ios or ... ?

[nospam3089]

I agree reducing dependencies in general is a desirable goal. For the three linked issues above, the reduction of dependencies could be done without any negative effects. As for this PR however, I'd say the possible choice of keeping quantity of dependencies low, conflicts with the hopefully existing goal of increasing the quality of the crate.

To my understanding, it is highly unlikely that the termios crate would introduce breakage on platforms where nix is working. (Except for the easily fixable potential need to add some definition(s) to src/os/, which I suppose would become apparent if releasing a PR run to the CI.) I base that claim on reading the crate descriptions, the API:s and enough of the source codes to understand their differences.

Lets start with one sentence from each crate description:

nix

This is done by wrapping the libc functionality with types/abstractions that enforce legal/safe usage.

termios

The safe bindings are a small wrapper around the raw C functions, which converts integer return values to std::io::Result to indicate success or failure.

The key point here of course being that nix attempts to make higher level abstractions, while termios stays low-level.

While nix::sys::termios does talk about these abstractions in terms of safety, the implementation (by truncating the flags) fails to honor that documented requirements of the C API to avoid undefined behavior triumphs type safety. As noted in nix-rust/nix#2071, the undefined behavior always gets triggered on illumos, without any possibility in the API to avoid it.

As the termios implementation merely passes the wrapped C struct unmodified, there are less moving parts and thus less risk. There's a point to be made that termios::tcsetattr() technically should probably be an unsafe function. With it too being able to trigger undefined behavior with invalid input. Considering the low-level expectation, and since termios::Termios documents every reasonable way to interface the API, I'm tempted to say that detail is best ignored.

Another, more opinionated, factor to consider is whether nix is basically a bit of a hit'n'run repository? One where people add functionality for their platform, without staying around to do comprehensive quality assurance. With 370 unique committer names in the log (of 2890 commits) and a call for maintainers opened since 2½ years, I'm of the opinion nix has bitten off more than it chew and my feeling is that it will likely always expose buggy functionality on non-mainstream platforms. (This is not my only problematic experience of that crate, but I'll spare you digressing too deep into the land of unrelated stuff.)

For comparison termios has seven unique committers over 70 commits, which seems like a ratio somewhere between a-one-man-show and too-many-chefs. It also has an outspoken goal to be stable and dcuddeback/termios-rs#12 motivates why the crate was not deprecated even though nix added an interface for the same C API.

I realize nix is used for other things in rustyline, and that solving nix-rust/nix#2071 would be to prefer (and would gladly do what I can to resolve it). All things considered, I still think merging this PR would be reasonable. One could always revert in case the current situation improves.

Alternatively, I could rewrite this PR to make the termios crate a conditional dependency only on illumos, if that would be preferred? That would clearly result in messier code and would not had been my first choice as a maintainer. But it isn't ideal that this crate is fully broken on illumos, is it?

Yet another theoretically possible way forward would of course be for rustyline to use termios from libc directly (which already is a dependency). I'm reluctant to suggest it, but I could update the PR in such a way if desired?

[gwenn]

Ok, let's try and see.
(please fix rustfmt issue)

[nospam3089]

Thanks!

A new branch have been pushed, fixing the rustfmt issue.

Some thinking also went into platform compability, and one should probably consider the risk of previously working platforms breaking. As you pointed out, e.g. ios would break since termios v0.3.3 lacks knowledge about it. Hence I've posted dcuddeback/termios-rs#36 which, if merged, could make v0.3.4 of termios support pretty much any posix like system out there. 🤞

[miraclx]

Missed this. Raw mode leaks. Resolved in #724.

Suggested change

	tcgetattr(self.tty_in, &mut termios)?;
	tcsetattr(self.tty_in, termios::TCSADRAIN, &mut termios)?;