AMythicDev / minus

An asynchronous, runtime data feedable terminal paging library for Rust
https://crates.io/crates/minus/
Apache License 2.0
317 stars 23 forks source link

Q/FR: A way to handle `minus` errors earlier with `dynamic_paging` #145

Open ilyagr opened 1 month ago

ilyagr commented 1 month ago

On Windows, minus::dynamic_paging returns an error for some terminals minus does not support. See https://github.com/martinvonz/jj/issues/4182. Our code is very similar to https://docs.rs/minus/latest/minus/index.html#threads, with one of the unwrap-s happening earlier: let pager_thread = spawn(move || dynamic_paging(pager2).unwrap());

In that issue, we'd like to handle the minus error better. Currently, we just panic with that unwrap. If we didn't panic, I see a few options, but the better ones require new minus features:

  1. It'd be nice if we could call a function from the main thread that would do some setup and make some sanity checks before starting a separate thread. Then, we could either print an error or disable the pager or do something else before anything is printed to the pager. For example, this new function could run up to and including the following line:

https://github.com/AMythicDev/minus/blob/43aa50d78a7fa1205fd7d37242d30ac0344461e8/src/core/init.rs#L120

  1. Instead of unwrap-ping the result of dynamic_paging immediately, we could notice the error when joining the thread (more similarly to the docs example I linked). We could then try printing out the text that went to minus without a pager. This would require a way to extract the text from minus, and I couldn't find such a way.

    This option has a fundamental problem, though, that we'd notice there was a problem at the very end of our program's execution. So, if writing to stdout fails, then our command will have done its work, but the user will not see the result, which is not great. In theory, this could happen already with our panic, depending on when it happens, but in practice the panic should happen quite early and hopefully prevent any changes the user should be informed about.

We could implement a version of option 2 by recording everything we send to minus ourselves, but that seems a bit awkward.

Or is there a better way to handle the error early that I missed? Thank you!

  1. Update: Another possible approach would be to have a "pager successfully started" callback. Or, more conveniently for this purpose, an "it is knows whether pager successfully started or failed" callback. Either way, the main thread would wait for it before proceeding with operations it doesn't want to do without being able to inform the user.