Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Send bound #65

Merged
merged 1 commit into from
Feb 7, 2017
Merged

Remove Send bound #65

merged 1 commit into from
Feb 7, 2017

Conversation

hannobraun
Copy link
Contributor

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.

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
Copy link

ArtemGr commented Nov 9, 2016

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

@hannobraun
Copy link
Contributor Author

hannobraun commented Nov 9, 2016

@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
Copy link

ArtemGr commented Nov 9, 2016

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
Copy link

ArtemGr commented Nov 10, 2016

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
Copy link
Owner

Sorry for the long delay, LGTM

@Stebalien Stebalien merged commit c31cc60 into Stebalien:master Feb 7, 2017
@hannobraun
Copy link
Contributor Author

No problem. Thanks for looking into it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants