Calling lock() on Stdout causes Picker::from_query_stdio() to fail unhelpfully

[mcclure]

I have a program based on sample code from the tui-textarea widget for Ratatui. I attempted to add ratatui-image to this program and got a failure (for example see: https://github.com/mcclure/tui-textarea , branch "orb-demo", commit 7e7341ba). When run, Picker::from_query_stdio().unwrap() would fail because from_query_stdio returned Err(Timeout(Timeout)). This confused me and I wasn't sure how to proceed (a function which fails instantly with error "Timeout" was confusing to me). After some experimentation and comparing with the ratatui-image samples, I found this was caused by the original tui-textarea code calling .lock() on io::stdout(). I don't fully understand why it does this; I assume it's so that printlns later in the program don't splatter nonsense while ratatui is in tui mode. I found I was able to introduce the problem into ratatui-image samples by adding a .lock() call on the stdout object, and I found I was able to fix the problem in my own code by either removing the .lock() or moving the call to Picker::from_query_stdio() to the start of the program, before the .lock().

Because there is sample code in the wild that suggests locking stdio before initializing crossterm, and all ratatui-image sample code initializes the picker after initializing crossterm, I think this should be considered a problem in ratatui-image. I can think of two ways to mitigate it:

• The hard way: The reason tui-textarea locks stdio (I think) is to reserve its use by crossterm. ratatui-image integrates with crossterm, so perhaps if ratatui-image detects crossterm is initialized it could obtain the locked stdio handle from crossterm to do the picker query then.
• The easy way: Add notes to either Errors::Timeout, from_query_stdio, or both, explaining that a Timeout error can also mean that stdio is locked as per normal ratatui practice. When I first saw the error I did check the docs, so this would have saved me time.

I am willing to write a docs PR myself if it would help.

[mcclure]

An update: As mentioned, I found I could avoid this problem by either removing .lock() or moving the from_query_stdio above the .lock(). I chose the second solution, because it seems .lock()-ing stdio is doing something beneficial. However I now notice the documentation for from_query_stdio says:

WARNING: this method should be called after entering alternate screen but before reading terminal events.

I cannot do this. The way I enter the alternate screen is crossterm::execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?; So stdout must be passed to execute before entering the alternate screen, and stdout must?¹ be locked before passing it to crossterm_execute, but your suggestion is that I call picker.from_query_stdio after crossterm_execute, and I cannot call from_query_stdio after lock. So in short I'm very confused, and any guidance for what I "ought" to do here is welcome.

¹ I do find I can call stdout::lock() after calling the above crossterm::execute, but I don't know if this will actually do what I want.

[benjajaja]

WARNING: this method should be called after entering alternate screen but before reading terminal events.

I think this is outdated or plain wrong. In my latest dog-food-eating at mdfried, I simply call from_query_stdio before any crossterm/ratatui stdio stuff.

Still, from_query_stdio is very easy to call at the wrong time. Perhaps the general advice / docs should say, "initialize picker.from_query_stdio before manipulating stdin or stdout.

The timeout errors could also be improved probably, to something like "no response from stdin" (or was it stdout?). In general though, the timeouts are leagues better than just halting forever - potentially on some other machine.

You are welcome to make some changes!

[mcclure]

@benjajaja , if i make changes it will probably be to the documentation, but before I do that—

""no response from stdin" (or was it stdout?)"

Could you clarify what you mean by this? It would help in knowing how the docs should be changed.

[benjajaja]

I just pushed 39d62f5 which changes the error from Timeout(Timeout) to No response from stdin.

To clarify: we send a query to stdout, and then read back the reply from stdin (with a timeout to avoid blocking forever). So the guidelines should be: 1) both stdout and stdin should not be locked by anything, 2) call from_query_stdio before doing anything else, and 3) if there is incoming data from stdin (e.g. pipe) then it must be read / cleared before.