search

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

Remove `Send` bound #65

Closed hannobraun closed 7 years ago

hannobraun commented 7 years ago

As far as I can tell, the Send bound doesn't serve any purpose and removing it didn't seem to have any adverse effects.

ArtemGr commented 7 years ago

What if I'm sharing the terminal between threads? I happen to be doing just that.

hannobraun commented 7 years ago

@ArtemGr I believe your use case will not be affected at all. Send is automatically derived by the Rust compiler wherever that is possible. That means, TerminfoTerminal<T> will be Send, if T is Send.

Currently, T is explicitly required to be Send, even though TerminfoTerminal itself doesn't actually need that capability. That means, you can never use a T that is not Send, even if you don't actually need TerminfoTerminal<T> to be Send.

So, to summarize: Unless I misunderstand the subtleties of Send, this change should make my use case possible, without changing anything about yours :)

ArtemGr commented 7 years ago

Oh, cool. Maybe we're talking about the differend Sends even! I'm only sharing the https://docs.rs/term/0.4.4/term/type.StdoutTerminal.html between threads. Thanks for clearing things out, though.

ArtemGr commented 7 years ago

I wonder if it even worth it to reuse the terminal. Right now all it saves is one Box::new it seems. On the other hand, if fn stdout() will make some extra checks in the future, such as isatty as discussed in #54, then suddenly I'm saving on I/O.

TBH, I wouldn't even be sharing the terminal handler between threads if I weren't using isatty myself. Here's what I do:

let stdout = Arc::new (if unsafe {libc::isatty (1)} != 0 {term::stdout().map (|s| Mutex::new (s))} else {None});
Stebalien commented 7 years ago

Sorry for the long delay, LGTM

hannobraun commented 7 years ago

No problem. Thanks for looking into it!