Stebalien / term

A Rust library for terminfo parsing and terminal colors.
https://stebalien.github.io/doc/term/term/
Apache License 2.0
178 stars 49 forks source link

panic with operation not supported by the terminal while libterm works fine #73

Open Dushistov opened 7 years ago

Dushistov commented 7 years ago

syntex project is port of rustc parser to be able to use it with stable compiler.

It used term crate as replace of https://github.com/rust-lang/rust/tree/master/src/libterm

And we see difference between term and libterm, the same code from: https://github.com/serde-rs/syntex/blob/master/syntex_errors/src/emitter.rs

cause panic if use term crate (0.4.5) and not panic and works as expected if use libterm.

See

https://github.com/keringar/syntex/commit/7ca0bb2e2dbae42d3983991787857730854336ef https://github.com/serde-rs/syntex/pull/121 https://github.com/rust-lang-nursery/rustfmt/issues/1180 https://github.com/serde-rs/syntex/issues/120

Any ideas why eshell handled in different ways in libterm and term?

Stebalien commented 7 years ago

Are you sure it's panicing inside the term crate? That is, is the call to unwrap inside the term crate? Sometime in the past, we changed the out-of-tree term crate to actually report an error when an unsupported attribute was used instead of just failing quietly (although I'm pretty sure the change was ported to the in-tree term crate).

Could you give me a stack trace?

Dushistov commented 7 years ago

@Stebalien

Are you sure it's panicing inside the term crate?

No, panic occures inside of syntex_errors analog of rust/src/librustc_errors.

The source of panic because of TerminfoTerminal::apply_cap from term crate return Err(::Error::NotSupported), while libterm inside rust return Ok(false),

libterm from rust is up to day:

commit a6ab049ed1db09f693df7d33046b3980f56751c1 Merge: 50b9858 3cd7f37 Author: bors bors@rust-lang.org Date: Fri May 5 03:56:34 2017 +0000

So if for example replace libterm with term for rustc, it also panics.

Any reasons return Err, instead of Ok(false)? With such API, code will be like this:

https://github.com/keringar/syntex/commit/7ca0bb2e2dbae42d3983991787857730854336ef

Stebalien commented 7 years ago

We made the change because it's too easy to accidentally ignore Ok(False) causing corrupted output when some commands succeed and others don't.

This is also why we added the is_supported method. It allows you to check if all needed commands are supported up-front. rustc uses this to choose between a "fancy" message printer and a "dumb" message printer.

Now, I'm not saying that this is a good API. However, I feel that having some commands succeed while others fail silently is unacceptable. I've tried fixing the API problems in the this create several times but, every time I've tried to improve it, I've ended up rewriting everything and then running out of free time.