Rework `lessopen` implementation to use `execute` crate instead of `run_script` #2805

Anomalocaridid · 2023-12-16T23:16:11Z

I replaced the lessopen implementation to use a crate called execute instead of the currently-used run_script. This has the benefit of simplifying the code a little, but more importantly fixes a few weird issues I noticed.

The issues I noticed that these changes fix are:

batpipe not working right when using fish as a shell. Currently, bat shows the commands that batpipe prints to configure $LESSOPEN when you evaluate it in your startup config rather than the preprocessed file output.
bat not showing any output at all when using glow to preprocess markdown files.

Edit: Also appears to fix some known issues with lessopen-related integration tests.

Anomalocaridid · 2023-12-17T19:33:57Z

Marking as draft because I realized my changes cause some of the integration tests to fail. Sorry for not catching it sooner.

Anomalocaridid · 2023-12-18T04:22:22Z

I fixed the issues with the integration tests.

However, while troubleshooting them, I came across another potential concern that I am unsure how to resolve. I noticed that unlike what I assumed in my initial implementation, less does not appear to send $LESSCLOSE's output to stdout. If I change this to match less's behavior, then that would break the integration tests that test $LESSCLOSE because they rely on bat outputting $LESSCLOSE's output. I am not sure how to change them to work around this, though.

~~Also, it may be worth seeing at some point if using execute instead of run_script also fixed the issue with the randomly failing integration test.~~

Edit: Already investigated, see below.

Anomalocaridid · 2024-12-01T03:31:31Z

After some testing, as far as I can tell, the #[serial] attribute on some of the lessopen-related tests is not needed anymore. In addition, the one lessopen-related tests that was ignored because it randomly fails appears to work reliably.

Also while I was at it, I disabled the long-help and short-help tests for lessopen in order to address #2706.

keith-hall

Thanks!

…ests

Anomalocaridid · 2025-01-05T21:56:32Z

My bad, I saw it said auto-merge was enabled and it would be merged when it meets the requirements and I saw the thing saying my PR branch was behind on changes from master and I assumed that being up to date was one of the requirements.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [sharkdp/bat](https://github.com/sharkdp/bat) | minor | `v0.24.0` -> `v0.25.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>sharkdp/bat (sharkdp/bat)</summary> ### [`v0.25.0`](https://github.com/sharkdp/bat/blob/HEAD/CHANGELOG.md#v0250) [Compare Source](sharkdp/bat@v0.24.0...v0.25.0) #### Features - Set terminal title to file names when Paging is not Paging::Never [#2807](sharkdp/bat#2807) ([@Oliver-Looney](https://github.com/Oliver-Looney)) - `bat --squeeze-blank`/`bat -s` will now squeeze consecutive empty lines, see [#1441](sharkdp/bat#1441) ([@eth-p](https://github.com/eth-p)) and [#2665](sharkdp/bat#2665) ([@einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - `bat --squeeze-limit` to set the maximum number of empty consecutive when using `--squeeze-blank`, see [#1441](sharkdp/bat#1441) ([@eth-p](https://github.com/eth-p)) and [#2665](sharkdp/bat#2665) ([@einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - `PrettyPrinter::squeeze_empty_lines` to support line squeezing for bat as a library, see [#1441](sharkdp/bat#1441) ([@eth-p](https://github.com/eth-p)) and [#2665](sharkdp/bat#2665) ([@einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - Syntax highlighting for JavaScript files that start with `#!/usr/bin/env bun` [#2913](sharkdp/bat#2913) ([@sharunkumar](https://github.com/sharunkumar)) - `bat --strip-ansi={never,always,auto}` to remove ANSI escape sequences from bat's input, see [#2999](sharkdp/bat#2999) ([@eth-p](https://github.com/eth-p)) - Add or remove individual style components without replacing all styles [#2929](sharkdp/bat#2929) ([@eth-p](https://github.com/eth-p)) - Automatically choose theme based on the terminal's color scheme, see [#2896](sharkdp/bat#2896) ([@bash](https://github.com/bash)) - Add option `--binary=as-text` for printing binary content, see issue [#2974](sharkdp/bat#2974) and MR [#2976](sharkdp/bat#2976) ([@einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - Make shell completions available via `--completion <shell>`, see issue [#2057](sharkdp/bat#2057) and MR [#3126](sharkdp/bat#3126) ([@einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - Syntax highlighting for puppet code blocks within Markdown files, see [#3152](sharkdp/bat#3152) ([@liliwilson](https://github.com/liliwilson)) #### Bugfixes - Fix long file name wrapping in header, see [#2835](sharkdp/bat#2835) ([@FilipRazek](https://github.com/FilipRazek)) - Fix `NO_COLOR` support, see [#2767](sharkdp/bat#2767) ([@acuteenvy](https://github.com/acuteenvy)) - Fix handling of inputs with OSC ANSI escape sequences, see [#2541](sharkdp/bat#2541) and [#2544](sharkdp/bat#2544) ([@eth-p](https://github.com/eth-p)) - Fix handling of inputs with combined ANSI color and attribute sequences, see [#2185](sharkdp/bat#2185) and [#2856](sharkdp/bat#2856) ([@eth-p](https://github.com/eth-p)) - Fix panel width when line 10000 wraps, see [#2854](sharkdp/bat#2854) ([@eth-p](https://github.com/eth-p)) - Fix compile issue of `time` dependency caused by standard library regression [#3045](sharkdp/bat#3045) ([@cyqsimon](https://github.com/cyqsimon)) - Fix override behavior of --plain and --paging, see issue [#2731](sharkdp/bat#2731) and MR [#3108](sharkdp/bat#3108) ([@einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - Fix bugs in `$LESSOPEN` support, see [#2805](sharkdp/bat#2805) ([@Anomalocaridid](https://github.com/Anomalocaridid)) #### Other - Upgrade to Rust 2021 edition [#2748](sharkdp/bat#2748) ([@cyqsimon](https://github.com/cyqsimon)) - Refactor and cleanup build script [#2756](sharkdp/bat#2756) ([@cyqsimon](https://github.com/cyqsimon)) - Checks changelog has been written to for MRs in CI [#2766](sharkdp/bat#2766) ([@cyqsimon](https://github.com/cyqsimon)) - Use GitHub API to get correct MR submitter [#2791](sharkdp/bat#2791) ([@cyqsimon](https://github.com/cyqsimon)) - Minor benchmark script improvements [#2768](sharkdp/bat#2768) ([@cyqsimon](https://github.com/cyqsimon)) - Update Arch Linux package URL in README files [#2779](sharkdp/bat#2779) ([@brunobell](https://github.com/brunobell)) - Update and improve `zsh` completion, see [#2772](sharkdp/bat#2772) ([@okapia](https://github.com/okapia)) - More extensible syntax mapping mechanism [#2755](sharkdp/bat#2755) ([@cyqsimon](https://github.com/cyqsimon)) - Use proper Architecture for Debian packages built for musl, see [#2811](sharkdp/bat#2811) ([@Enselic](https://github.com/Enselic)) - Pull in fix for unsafe-libyaml security advisory, see [#2812](sharkdp/bat#2812) ([@dtolnay](https://github.com/dtolnay)) - Update git-version dependency to use Syn v2, see [#2816](sharkdp/bat#2816) ([@dtolnay](https://github.com/dtolnay)) - Update git2 dependency to v0.18.2, see [#2852](sharkdp/bat#2852) ([@eth-p](https://github.com/eth-p)) - Improve performance when color output disabled, see [#2397](sharkdp/bat#2397) and [#2857](sharkdp/bat#2857) ([@eth-p](https://github.com/eth-p)) - Relax syntax mapping rule restrictions to allow brace expansion [#2865](sharkdp/bat#2865) ([@cyqsimon](https://github.com/cyqsimon)) - Apply clippy fixes [#2864](sharkdp/bat#2864) ([@cyqsimon](https://github.com/cyqsimon)) - Faster startup by offloading glob matcher building to a worker thread [#2868](sharkdp/bat#2868) ([@cyqsimon](https://github.com/cyqsimon)) - Display which theme is the default one in basic output (no colors), see [#2937](sharkdp/bat#2937) ([@sblondon](https://github.com/sblondon)) - Display which theme is the default one in colored output, see [#2838](sharkdp/bat#2838) ([@sblondon](https://github.com/sblondon)) - Add aarch64-apple-darwin ("Apple Silicon") binary tarballs to releases, see [#2967](sharkdp/bat#2967) ([@someposer](https://github.com/someposer)) - Update the Lisp syntax, see [#2970](sharkdp/bat#2970) ([@ccqpein](https://github.com/ccqpein)) - Use bat's ANSI iterator during tab expansion, see [#2998](sharkdp/bat#2998) ([@eth-p](https://github.com/eth-p)) - Support 'statically linked binary' for aarch64 in 'Release' page, see [#2992](sharkdp/bat#2992) ([@tzq0301](https://github.com/tzq0301)) - Update options in shell completions and the man page of `bat`, see [#2995](sharkdp/bat#2995) ([@akinomyoga](https://github.com/akinomyoga)) - Update nix dev-dependency to v0.29.0, see [#3112](sharkdp/bat#3112) ([@decathorpe](https://github.com/decathorpe)) - Bump MSRV to [1.74](https://blog.rust-lang.org/2023/11/16/Rust-1.74.0.html), see [#3154](sharkdp/bat#3154) ([@keith-hall](https://github.com/keith-hall)) - Update clircle dependency to remove winapi transitive dependency, see [#3113](sharkdp/bat#3113) ([@niklasmohrin](https://github.com/niklasmohrin)) #### Syntaxes - `cmd-help`: scope subcommands followed by other terms, and other misc improvements, see [#2819](sharkdp/bat#2819) ([@victor-gp](https://github.com/victor-gp)) - Upgrade JQ syntax, see [#2820](sharkdp/bat#2820) ([@dependabot](https://github.com/dependabot)\[bot]) - Add syntax mapping for quadman quadlets [#2866](sharkdp/bat#2866) ([@cyqsimon](https://github.com/cyqsimon)) - Map containers .conf files to TOML syntax [#2867](sharkdp/bat#2867) ([@cyqsimon](https://github.com/cyqsimon)) - Associate `.xsh` files with `xonsh` syntax that is Python, see [#2840](sharkdp/bat#2840) ([@anki-code](https://github.com/anki-code)) - Associate JSON with Comments `.jsonc` with `json` syntax, see [#2795](sharkdp/bat#2795) ([@mxaddict](https://github.com/mxaddict)) - Associate JSON-LD `.jsonld` files with `json` syntax, see [#3037](sharkdp/bat#3037) ([@vorburger](https://github.com/vorburger)) - Associate `.textproto` files with `ProtoBuf` syntax, see [#3038](sharkdp/bat#3038) ([@vorburger](https://github.com/vorburger)) - Associate GeoJSON `.geojson` files with `json` syntax, see [#3084](sharkdp/bat#3084) ([@mvaaltola](https://github.com/mvaaltola)) - Associate `.aws/{config,credentials}`, see [#2795](sharkdp/bat#2795) ([@mxaddict](https://github.com/mxaddict)) - Associate Wireguard config `/etc/wireguard/*.conf`, see [#2874](sharkdp/bat#2874) ([@cyqsimon](https://github.com/cyqsimon)) - Add support for [CFML](https://www.adobe.com/products/coldfusion-family.html), see [#3031](sharkdp/bat#3031) ([@brenton-at-pieces](https://github.com/brenton-at-pieces)) - Map `*.mkd` files to `Markdown` syntax, see issue [#3060](sharkdp/bat#3060) and MR [#3061](sharkdp/bat#3061) ([@einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - Add syntax mapping for CITATION.cff, see [#3103](sharkdp/bat#3103) ([@Ugzuzg](https://github.com/Ugzuzg)) - Add syntax mapping for kubernetes config files [#3049](sharkdp/bat#3049) ([@cyqsimon](https://github.com/cyqsimon)) - Adds support for pipe delimiter for CSV [#3115](sharkdp/bat#3115) ([@pratik-m](https://github.com/pratik-m)) - Add syntax mapping for `/etc/pacman.conf` [#2961](sharkdp/bat#2961) ([@cyqsimon](https://github.com/cyqsimon)) - Associate `uv.lock` with `TOML` syntax, see [#3132](sharkdp/bat#3132) ([@fepegar](https://github.com/fepegar)) #### Themes - Patched/improved themes for better Manpage syntax highlighting support, see [#2994](sharkdp/bat#2994) ([@keith-hall](https://github.com/keith-hall)). #### `bat` as a library - Changes to `syntax_mapping::SyntaxMapping` [#2755](sharkdp/bat#2755) ([@cyqsimon](https://github.com/cyqsimon)) - `SyntaxMapping::get_syntax_for` is now correctly public - \[BREAKING] `SyntaxMapping::{empty,builtin}` are removed; use `SyntaxMapping::new` instead - \[BREAKING] `SyntaxMapping::mappings` is replaced by `SyntaxMapping::{builtin,custom,all}_mappings` - Make `Controller::run_with_error_handler`'s error handler `FnMut`, see [#2831](sharkdp/bat#2831) ([@rhysd](https://github.com/rhysd)) - Improve compile time by 20%, see [#2815](sharkdp/bat#2815) ([@dtolnay](https://github.com/dtolnay)) - Add `theme::theme` for choosing an appropriate theme based on the terminal's color scheme, see [#2896](sharkdp/bat#2896) ([@bash](https://github.com/bash)) - \[BREAKING] Remove `HighlightingAssets::default_theme`. Use `theme::default_theme` instead. - Add `PrettyPrinter::print_with_writer` for custom output destinations, see [#3070](sharkdp/bat#3070) ([@kojix2](https://github.com/kojix2)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Anomalocaridid force-pushed the refactor-lessopen branch from e838c8f to 36d9566 Compare December 16, 2023 23:24

Anomalocaridid marked this pull request as draft December 17, 2023 19:33

Anomalocaridid force-pushed the refactor-lessopen branch from 36d9566 to 11e4010 Compare December 18, 2023 04:03

Anomalocaridid marked this pull request as ready for review December 18, 2023 04:12

Anomalocaridid changed the title ~~Refactor lessopen to use execute crate instead of run_script~~ Rework lessopen implementation to use execute crate instead of run_script Dec 18, 2023

Anomalocaridid force-pushed the refactor-lessopen branch from 11e4010 to 450b73d Compare December 31, 2023 19:27

Anomalocaridid force-pushed the refactor-lessopen branch 2 times, most recently from 7ce06c5 to 1a1c4c3 Compare April 13, 2024 22:25

Anomalocaridid force-pushed the refactor-lessopen branch 2 times, most recently from 4c3c86d to 4355654 Compare October 31, 2024 19:31

Anomalocaridid force-pushed the refactor-lessopen branch 4 times, most recently from df1c0d4 to a163674 Compare November 28, 2024 17:30

Anomalocaridid force-pushed the refactor-lessopen branch from f9eb801 to 9ee5de3 Compare December 1, 2024 03:45

Anomalocaridid force-pushed the refactor-lessopen branch from 9ee5de3 to f77032a Compare December 9, 2024 01:53

Anomalocaridid force-pushed the refactor-lessopen branch from 44265b3 to 26274c6 Compare January 4, 2025 02:29

keith-hall approved these changes Jan 5, 2025

View reviewed changes

keith-hall enabled auto-merge January 5, 2025 21:13

Anomalocaridid added 5 commits January 5, 2025 21:36

replace run_script with execute

7bf459f

refactor lessopen implementation

de8bb79

update CHANGELOG.md

bc61d84

tests: disable lessopen for help tests

a0a090c

tests: remove serial attribute on and un-ignore applicable lessopen t…

96e4882

…ests

auto-merge was automatically disabled January 5, 2025 21:36
Head branch was pushed to by a user without write access

Anomalocaridid force-pushed the refactor-lessopen branch from 26274c6 to 96e4882 Compare January 5, 2025 21:36

keith-hall merged commit ae07586 into sharkdp:master Jan 6, 2025
24 checks passed

Anomalocaridid deleted the refactor-lessopen branch January 7, 2025 02:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework `lessopen` implementation to use `execute` crate instead of `run_script` #2805

Rework `lessopen` implementation to use `execute` crate instead of `run_script` #2805

Anomalocaridid commented Dec 16, 2023 •

edited

Loading

Anomalocaridid commented Dec 17, 2023 •

edited

Loading

Anomalocaridid commented Dec 18, 2023 •

edited

Loading

Anomalocaridid commented Dec 1, 2024

keith-hall left a comment

Anomalocaridid commented Jan 5, 2025

Rework lessopen implementation to use execute crate instead of run_script #2805

Rework lessopen implementation to use execute crate instead of run_script #2805

Conversation

Anomalocaridid commented Dec 16, 2023 • edited Loading

Anomalocaridid commented Dec 17, 2023 • edited Loading

Anomalocaridid commented Dec 18, 2023 • edited Loading

Anomalocaridid commented Dec 1, 2024

keith-hall left a comment

Choose a reason for hiding this comment

Anomalocaridid commented Jan 5, 2025

Rework `lessopen` implementation to use `execute` crate instead of `run_script` #2805

Rework `lessopen` implementation to use `execute` crate instead of `run_script` #2805

Anomalocaridid commented Dec 16, 2023 •

edited

Loading

Anomalocaridid commented Dec 17, 2023 •

edited

Loading

Anomalocaridid commented Dec 18, 2023 •

edited

Loading