Prevent fork nightmare with PAGER=batcat #2235
Conversation
Enselic
changed the title
|
Made commits with suggested changes, ready for further review. |
There was a problem hiding this comment.
Thanks! Much clearer code. I proposed one possible further simplification. Can you also please add an entry to CHANGELOG.md? Like this:
## Bugfixes
- Prevent fork nightmare with `PAGER=batcat`. See #2235 (@johnmatthiggins)
When that is done I think I can set this PR as Approved!
Co-authored-by: Martin Nordholts <enselic@gmail.com>
|
Added to change log and altered match expression. |
I forgot to comment on this name so I figured I'd just push a commit to take care of it.
There was a problem hiding this comment.
I forgot to comment on the arg0 name, so I pushed a commit for that. I hope you didn't mind.
This looks good to me now! Thanks a lot.
As far as I can tell our current regression tests cover this PR. For example, if we hardcode is_current_bin_pager to either true or false, tests start failing.
I didn't expect it to be, but just to make sure I ensured that startup time is not in practice affected, and it wasn't:
% hyperfine --export-markdown /dev/tty -L bat ./target/release/bat,bat-master-c8b1187 '{bat} -f --paging=always --theme="Monokai Extended" examples/simple.rs'
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
./target/release/bat -f --paging=always --theme="Monokai Extended" examples/simple.rs |
11.0 ± 1.1 | 10.1 | 21.2 | 1.00 |
bat-master-c8b1187 -f --paging=always --theme="Monokai Extended" examples/simple.rs |
11.1 ± 1.1 | 10.1 | 21.9 | 1.01 ± 0.14 |
|
About time to merge this now I think. |
Used rsamuelklatchko's suggested changes to fix recursive fork issue. Fixes issue #2023.