Skip to content
New issue

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

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

Already on GitHub? # to your account

Upgrading from 0.10.x to 0.12.x break input #295

Closed
sophiajt opened this issue Oct 27, 2019 · 16 comments
Closed

Upgrading from 0.10.x to 0.12.x break input #295

sophiajt opened this issue Oct 27, 2019 · 16 comments

Comments

@sophiajt
Copy link
Contributor

Hi all,

We use crossterm in nushell in a few different places, and have generally been pretty happy with it. Recently, I tried to update from 0.10.x to 0.12.x. While the API changes were easy to fix, the result was a system that no longer worked.

Does someone have a minute to look over https://github.com/nushell/nushell/blob/master/src/fuzzysearch.rs and let me know why it doesn't work in 0.12.x (ignoring the slight API differences)

Here are some of the issues we see with 0.12.x when we try it:

  • Hitting enter on a selection doesn't work
  • After fuzzy search runs, input is eaten about every other character, so you can't type in the nushell prompt correctly anymore
@TimonPost
Copy link
Member

Mmm, we will look into this. We did a complete rewrite of input unix module. So yea there might me differences although our tests haven't shown any problems with it yet.

@vmedea
Copy link

vmedea commented Oct 27, 2019

Enter and Tab are now passed as separate Key enum items instead of characters.
This fixed it for Cursive: gyscos/cursive#395

@TimonPost
Copy link
Member

Ah yes that's it @jonathandturner, thanks for the search!

@sophiajt
Copy link
Contributor Author

Thanks @vmedea - the keys work now.

The last issue is that it seems like crossterm continues to monitor the input after this code finishes. With 0.10.x, if you did sync inputs, crossterm would stop trying to read the input after the code left. But, as best as I can figure right now it seems that some of the input characters continue to get eaten after this code runs.

@sophiajt
Copy link
Contributor Author

Is there a way to tell crossterm to stop taking input, or at least pause taking input?

@TimonPost
Copy link
Member

@zrzka did you build any mechanism to stop reading after all channels are dropped?

@zrzka
Copy link
Contributor

zrzka commented Oct 28, 2019

@TimonPost nope, the reading thread is still there even if you drop all (a)sync readers. I didn't do it because there's a huge input refactoring PR and it didn't make sense to spend so much time on it. Currently, there're couple of options for the crate users:

  • live with it,
  • we can introduce simple fn which will stop the reading thread no matter how many readers you have instantiated and release crossterm 0.12.2 with this change only,
  • wait for the input PR to be finished, which will take some time, because I consider it as highly experimental now.

@TimonPost
Copy link
Member

We can implement Arc for a type that's wraps the Receiver. We share this type and clone it for every consumer. Then we implement Drop for this type. This Drop is called when all references to Arc are dropped. Here we can stop the reader because there are no listeners.

@TimonPost
Copy link
Member

Closing this now, because the problem has been solved. Either use the old crossterm or wait for the input PR. It will be fixed in the near future.

@sophiajt
Copy link
Contributor Author

We'll stick with 0.10.x. We'd like to be able to upgrade but won't be able to until this gets fixed.

@TimonPost
Copy link
Member

Unfortunate, it will probably fixed in 0.14. 0.13 is going to be released this or next week. We are already working on the new input module so there is work in progress. But expect that to be released in about two or three weeks. (just estimating)

@TimonPost
Copy link
Member

@jonathandturner, we did a patch with #307. This might be related to your issue if you were on Windows. An AsyncReader can be closed and is closed on drop of AsyncReader.

@Canop
Copy link
Contributor

Canop commented Nov 4, 2019

@jonathandturner I use read_sync in broot. I ported my application to crossterm 0.13 and no character is eaten, so I'm sure it's feasible.

If you're interested my implementation of an event reader is thread+channel based: https://github.com/Canop/termimad/blob/master/src/events/event_source.rs#L28

EDIT: I reproduce this problem when broot isn't closed (and thus the new thread of the sync reader isn't dropped even while the sync reader is): one key over two is lost.

@TimonPost
Copy link
Member

A temp fix is merged #308

@Canop
Copy link
Contributor

Canop commented Nov 4, 2019

Thanks @zrzka and @TimonPost for having been so fast in publishing a fix!

@sigmaSd
Copy link
Contributor

sigmaSd commented Dec 27, 2019

@jonathandturner this appears to be fixed in 0.14

december1981 pushed a commit to december1981/crossterm that referenced this issue Oct 26, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants