search

crossterm-rs / crossterm

Cross platform terminal library rust
MIT License
3.29k stars 280 forks source link

If ioctl TIOCGWINSZ returns 0,0 try to use env vars instead. #893

Open swilde opened 6 months ago

swilde commented 6 months ago

For some terminals (for example Emacs eshell/eterm) the ioctl with TIOCGWINSZ might falsely return 0 columns and 0 rows. If this happens we now try to use LINES and COLUMNS environment variables.

Fixes #891

swilde commented 6 months ago

Any idea if there are perf concerns to checking the environment variables compare with ioctl call. (Context: Ratatui checks window_size on every frame, I'd expect many apps might do something similar).

I don't expect the operation to be expensive. In addition it only happens when ioctl fails.

joshka commented 6 months ago

I don't expect the operation to be expensive. In addition it only happens when ioctl fails.

Did a quick benchmark. On a mac, this is ~100ns, so fast enough generally to not matter. If this turned out to be a hotspot (actually measured by a profiler), it would be easy to replace with a lazy static, but there's no need to do this upfront for now.

joshka commented 6 months ago

@TimonPost this would probably be easy to merge

swilde commented 6 months ago

Don't mean to be a PITA but is there anything I could do to increase chances of getting the PR merged?

joshka commented 5 months ago

Don't mean to be a PITA but is there anything I could do to increase chances of getting the PR merged?

I've messaged @TimonPost about this on discord.

For context I'm just an interested party - not a maintainer of crossterm. Perhaps if you have some time, it might be good to get some extra eyes on the other PRs and help out from the perspective of testing / reviewing code. Does that sound like something that would work for you?

TimonPost commented 5 months ago

Personally do not have a presence if ppl writing cli applications find a very usefull use to this im fine with getting it in.