lycheeverse
Fully hide progress bar if any input is `stdin`
In https://github.com/lycheeverse/lychee/pull/1914, work was done to hide the progress bar on stdin. The simple solution of removing enable_steady_tick for the progress bar. This worked well for avoiding cluttering the output while the user typed their input.
However, the initial progress bar still gets shown, which is not ideal. This commit slightly imporves the behavior by waiting for all inputs before showing the progress bar. We do this by iterating over the inputs and checking if any of them is stdin.
This way, the progress bar does no longer get shown. To start the processing, the user has to press Ctrl^D, which is customs for other Unix tools as well. Other behavior remains unchanged.
Personally, I'm okay with the current behaviour. As mentioned in https://github.com/lycheeverse/lychee/pull/1914#issuecomment-3532526933 and https://github.com/lycheeverse/lychee/issues/1736#issuecomment-3206966681 I find it unexpected to fully hide the bar if stdin is one of the inputs.
The main reason is that this might confuse users a lot. I assume the more common use case is to pipe something to lychee, rather than plain lychee -. With this PR if we now do cat README.md | lychee - the progress bar disappears without any apparent reason and there is no way for users to enable it again.
cat README.md | cargo run -- -
Oh, I hadn't thought of that. Nice catch!
Yeah, it's a bit of a tradeoff. In your recording (now archived), I got a bit irritated by the progress bar. It looked like a rendering bug, to be honest. But as you pointed out, there's no easy solution here. 😕
Or so I thought!
Maybe we should only hide the progress bar when stdin is not a TTY (i.e., it's being piped or redirected). If someone runs lychee - interactively and types into the terminal, stdin is still a TTY.
I tweaked the behavior a bit. The implementation now detects when stdin is piped or redirected and hides the progress bar only in those cases. What do you think? 🤩
Ooh I didn't know this was possible :+1:
It still feels a bit like a hack but I'm okay with it, if we are even more strict with the predicate and set stdin_input to true if stdin is the only input. (cargo run -) Because running cargo run - README.md also checks README.md. The order is non-deterministic so sometimes the file is checked while lychee also waits for stdin and sometimes it is only checked after the user finished writing the contents to stdin. If we do this then I'm okay with the PR.
Quick heads-up: When I tested this, I think I found a few issues. That means, we should test that thoroughly before merging. Don't have time to re-run the tests right now, but if someone wants to check out this branch and try it on a few inputs, that would be helpful.