feat: make output pipeable with -n, non-auto styles#3438
feat: make output pipeable with -n, non-auto styles#3438keith-hall merged 2 commits intosharkdp:masterfrom
-n, non-auto styles#3438Conversation
c68b8bd to
fabd105
Compare
-n, non-auto styles
ee3ecc1 to
50f39f0
Compare
-n, non-auto styles-n, non-auto styles
f02b7a8 to
2a95a12
Compare
|
I've disabled these two tests on Windows as they are failing the CI (snapshots have one fewer terminal width char?) on 2 of the 3 Windows architectures (so if I changed it to per platform it would still not work) 🤦♂️
Click to show full log` ` |
Isn't that what the bat --decorations=always -nr :4 Cargo.toml | cat
1 [package]
2 authors = ["David Peter <mail@david-peter.de>"]
3 categories = ["command-line-utilities"]
4 description = "A cat(1) clone with wings." |
I meant
This is ultimately about nicer default behaviour for piping, as #2983 was, and keeping to the original design/better ergonomics. |
|
Lines 15 to 17 in a730eaa Lines 131 to 135 in a730eaa It makes bat's behavior more complex. The rule is no longer "always act like cat |
|
Ah I hadn’t seen those man page sections before.
I'd say it changes the behaviour rather than making it more complex.
Yes, that's exactly the point - this was the requested change in #2935 (agreed to be worked on in 2024, did not land in #2983).
I think this phrasing is unnecessarily verbose. The simpler way to describe it is:
Two clarifications:
Based on the man page you quoted, I thought this was already the intended functionality: that the auto style adapts to piping, while explicit styles like default or numbers would work regardless.
To me, intuitive = simple. If someone explicitly requests a style with I find it undesirable for bat to behave like cat when piped, and my impression from the issue tracker is that others agree. I understood from #2935 and the discussion on the previous PR that this behavior change was desired. If there's still disagreement about whether this should change at all, I'm not sure how to proceed. I took it I was implementing a requested feature. The key principle: when someone explicitly requests a style, honour it. Default behaviour (no flags) is preserved for backwards compatibility. I'll update the man page sections to reflect this behavior change: @@ -1,3 +1,4 @@
Whenever the output of {{PROJECT_EXECUTABLE}} goes to a non-interactive terminal, i.e. when the
output is piped into another process or into a file, {{PROJECT_EXECUTABLE}} will act as a drop-in
-replacement for cat(1) and fall back to printing the plain file contents.
+replacement for cat(1) and fall back to printing the plain file contents,
+unless an explicit style is requested.@@ -2,5 +2,6 @@
.IP
Specify when to use the decorations that have been specified via '\-\-style'. The
-automatic mode only enables decorations if an interactive terminal is detected. Possible
-values: *auto*, never, always.
+automatic mode only enables decorations if an interactive terminal is detected. The
+always mode will show decorations even when piping output. Possible
+values: *auto*, never, always. |
-n, non-auto styles-n, non-auto styles
b916592 to
42322b8
Compare
I then looked for other docs with the same substring to edit likewise louis 🌟 ~/dev/bat $ rg -l 'Specify when to use the dec'
src/bin/bat/clap_app.rs
assets/manual/bat.1.in
doc/long-help.txt
tests/syntax-tests/highlighted/Manpage/bat-0.16.man
tests/syntax-tests/source/Manpage/bat-0.16.man
The README also mentions cat-like behaviour in two places (file concatenation and xclip examples), but I'd leave these as is since they still work as shown. The modifier "except if a style is set" could be added, but I think this is sufficiently intuitive/already documented that it would be unnecessary. 👍 Ready for re-review, taking back out of draft |
keith-hall
left a comment
There was a problem hiding this comment.
Nice, thanks for your hard work on this one! Looks great!
|
Until now, bat behaved predictably: it showed decorations only in interactive The proposed change breaks this for some flags. Users would see decorations in I got accustomed to interactive viewing and have learned over the years that BAT_OPTS='--style=header,grid' bat README.md | wc -lIf line numbers in piped output is the major reason, users can set the |
|
Thanks for the feedback, to address your points: The change only affects behaviour when users explicitly pass style flags like The principle: if someone explicitly asks for a style, honour it. This matches how other Unix tools work. On On The logic and compatibility were validated in previous reviews. The implementation is consistent with both the documentation and existing Note Rebased on master branch without changes DemosDiff DemoBehaviour is unchanged: louis 🌟 ~/dev/bat $ bat --diff README.md
───────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
│ File: README.md
───────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
911 │
912 │ ## Project goals and alternatives
913 ~ │ hello
914 │ `bat` tries to achieve the following goals:
915 │
───────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
louis 🌟 ~/dev/bat $ bat --diff README.md | wc -l
5
louis 🌟 ~/dev/bat $ ./target/release/bat --diff README.md | wc -l
5Cat DemoComparing louis 🌟 ~/dev/bat $ echo "hello" | cat | wc -c
6
louis 🌟 ~/dev/bat $ echo "hello" | cat -n | wc -c
13
louis 🌟 ~/dev/bat $ echo "hello" | bat | wc -c
6
louis 🌟 ~/dev/bat $ echo "hello" | bat -n | wc -c
6With this PR, louis 🌟 ~/dev/bat $ echo "hello" | ./target/release/bat -n | wc -c
11Tail DemoHeaders are preserved when piped (similar to bat's header/grid decorations): louis 🌟 ~/dev/bat $ tail -n999 <(echo "foo") <(echo "bar")
==> /dev/fd/63 <==
foo
==> /dev/fd/62 <==
bar
louis 🌟 ~/dev/bat $ tail -n999 <(echo "foo") <(echo "bar") | cat
==> /dev/fd/63 <==
foo
==> /dev/fd/62 <==
bar |
|
@LangLangBart thanks for sharing your concerns. I understand that these changes may break behavior you are used to. But, as we have had a number of people open issues about it, and the wording in the documentation generally suggesting it should work the way it does in this PR, along with the |
-n, non-auto styles-n, non-auto styles
|
Sorry @keith-hall I was reviewing bug reports in here this morning and did not spot this 🤦♂️ Taking a look now |
That issue seems to be addressed already, not sure what the ask is here @keith-hall but happy to think about any further accomodations. Were there any other reports or just this one so far? That I personally think aliasing cat with a style is a bit radical, and I think it’s fair to expect that to not be smooth sailing! It’s much more common if you look at people’s shared dotfiles for them to do it with |
Just this one so far that I have seen. I guess I just want us to be prepared in case more come in. I completely agree about aliasing I was wondering whether it would be safer to only fix Probably we can leave things as they are for now. Thanks for checking common usage 👍 |
I do understand that aliasing cat is bound to break sometimes but in this case I don't see why it is considered radical though. The way I see it the decorations behave in a confusing way now. I always assumed not carrying decorations was deliberate choice to avoid issues when piped to something else but if that is not the case then may be we should unify behaviour in the above case as well. |
is @sharkdp still around ? |
Perhaps "radical" was too strong - I meant that using bat as a "drop-in replacement" (as quoted in the man page) refers to unconfigured behaviour, whilst custom style configurations are a different use case. The PR preserves default (unconfigured) behaviour; extending that guarantee to all possible style configurations would require different design choices. |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [sharkdp/bat](https://github.com/sharkdp/bat) | minor | `v0.25.0` -> `v0.26.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.26.0`](https://github.com/sharkdp/bat/blob/HEAD/CHANGELOG.md#v0260) [Compare Source](sharkdp/bat@v0.25.0...v0.26.0) #### Features - Add build for windows/ARM64 platform. [#​3190](sharkdp/bat#3190) ([@​alcroito](https://github.com/alcroito)) - Add paging to `--list-themes`, see MR [#​3239](sharkdp/bat#3239) ([@​einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - Support negative relative line ranges, e.g. `bat -r :-10` / `bat -r='-10:'`, see [#​3068](sharkdp/bat#3068) ([@​ajesipow](https://github.com/ajesipow)) - Support context in line ranges, e.g. `bat -r 30::5` / `bat -r 30:40:5`, see [#​3345](sharkdp/bat#3345) ([@​cavanaug](https://github.com/cavanaug)) - Add built-in 'minus' pager, e.g. `bat --pager=builtin` see MR [#​3402](sharkdp/bat#3402) ([@​academician](https://github.com/academician)) #### Bugfixes - Fix UTF-8 BOM not being stripped for syntax detection, see [#​3314](sharkdp/bat#3314) ([@​krikera](https://github.com/krikera)) - Fix `BAT_THEME_DARK` and `BAT_THEME_LIGHT` being ignored, see issue [#​3171](sharkdp/bat#3171) and MR [#​3168](sharkdp/bat#3168) ([@​bash](https://github.com/bash)) - Prevent `--list-themes` from outputting default theme info to stdout when it is piped, see [#​3189](sharkdp/bat#3189) ([@​einfachIrgendwer0815](https://github.com/einfachIrgendwer0815)) - Rename some submodules to fix Dependabot submodule updates, see issue [#​3198](sharkdp/bat#3198) and MR [#​3201](sharkdp/bat#3201) ([@​victor-gp](https://github.com/victor-gp)) - Make highlight tests fail when new syntaxes don't have fixtures MR [#​3255](sharkdp/bat#3255) ([@​dan-hipschman](https://github.com/dan-hipschman)) - Fix crash for multibyte characters in file path, see issue [#​3230](sharkdp/bat#3230) and MR [#​3245](sharkdp/bat#3245) ([@​HSM95](https://github.com/HSM95)) - Add missing mappings for various bash/zsh files, see MR [#​3262](sharkdp/bat#3262) ([@​AdamGaskins](https://github.com/AdamGaskins)) - Send all bat errors to stderr by default, see [#​3336](sharkdp/bat#3336) ([@​JerryImMouse](https://github.com/JerryImMouse)) - Make --map-syntax target case insensitive to match --language, see [#​3206](sharkdp/bat#3206) ([@​keith-hall](https://github.com/keith-hall)) - Correctly determine the end of the line in UTF16LE/BE input [#​3369](sharkdp/bat#3369) ([@​keith-hall](https://github.com/keith-hall)) - `--style=changes` no longer prints a two-space indent when the file is unmodified, see issue [#​2710](sharkdp/bat#2710) and MR [#​3406](sharkdp/bat#3406) ([@​jyn514](https://github.com/jyn514)) - Add missing shell completions, see [#​3411](sharkdp/bat#3411) ([@​keith-hall](https://github.com/keith-hall)) - Execute help/version/diagnostic commands even with invalid config/arguments present, see [#​3414](sharkdp/bat#3414) ([@​keith-hall](https://github.com/keith-hall)) - Fixed line numbers (`-n`) and style components not printing when piping output, see issue [#​2935](sharkdp/bat#2935) and MR [#​3438](sharkdp/bat#3438) ([@​lmmx](https://github.com/lmmx)) #### Other - Update base16 README links to community driven base16 work [#​2871](sharkdp/bat#2871) ([@​JamyGolden](https://github.com/JamyGolden)) - Work around build failures when building `bat` from vendored sources [#​3179](sharkdp/bat#3179) ([@​dtolnay](https://github.com/dtolnay)) - CICD: Stop building for x86\_64-pc-windows-gnu which fails [#​3261](sharkdp/bat#3261) (Enselic) - CICD: replace windows-2019 runners with windows-2025 [#​3339](sharkdp/bat#3339) ([@​cyqsimon](https://github.com/cyqsimon)) - Build script: replace string-based codegen with quote-based codegen [#​3340](sharkdp/bat#3340) ([@​cyqsimon](https://github.com/cyqsimon)) - Improve code coverage of `--list-languages` parameter [#​2942](sharkdp/bat#2942) ([@​sblondon](https://github.com/sblondon)) - Only start offload worker thread when there's more than 1 core [#​2956](sharkdp/bat#2956) ([@​cyqsimon](https://github.com/cyqsimon)) - Update terminal-colorsaurus (the library used for dark/light detection) to 1.0, see [#​3347](sharkdp/bat#3347) ([@​bash](https://github.com/bash)) - Update console dependency to 0.16, see [#​3351](sharkdp/bat#3351) ([@​musicinmybrain](https://github.com/musicinmybrain)) - Fixed some typos [#​3244](sharkdp/bat#3244) ([@​ssbarnea](https://github.com/ssbarnea)) - Update onig\_sys dependency to 69.9.1 to fix a gcc build failure [#​3400](sharkdp/bat#3400) ([@​CosmicHorrorDev](https://github.com/CosmicHorrorDev)) - Add a cargo feature (`vendored-libgit2`) to build with vendored libgit2 version without depending on the system's one [#​3426](sharkdp/bat#3426) ([@​0x61nas](https://github.com/0x61nas)) - Update syntect dependency to v5.3.0 to fix a few minor bugs, see [#​3410](sharkdp/bat#3410) ([@​keith-hall](https://github.com/keith-hall)) #### Syntaxes - Add syntax mapping for `paru` configuration files [#​3182](sharkdp/bat#3182) ([@​cyqsimon](https://github.com/cyqsimon)) - Add support for [Idris 2 programming language](https://www.idris-lang.org/) [#​3150](sharkdp/bat#3150) ([@​buzden](https://github.com/buzden)) - Add syntax mapping for `nix`'s '`flake.lock` lockfiles [#​3196](sharkdp/bat#3196) ([@​odilf](https://github.com/odilf)) - Improvements to CSV/TSV highlighting, with autodetection of delimiter and support for TSV files, see [#​3186](sharkdp/bat#3186) ([@​keith-](https://github.com/keith-) - Improve (Sys)log error highlighting, see [#​3205](sharkdp/bat#3205) ([@​keith-hall](https://github.com/keith-hall)) - Map `ndjson` extension to JSON syntax, see [#​3209](sharkdp/bat#3209) ([@​keith-hall](https://github.com/keith-hall)) - Map files with `csproj`, `vbproj`, `props` and `targets` extensions to XML syntax, see [#​3213](sharkdp/bat#3213) ([@​keith-hall](https://github.com/keith-hall)) - Add debsources syntax to highlight `/etc/apt/sources.list` files, see [#​3215](sharkdp/bat#3215) ([@​keith-hall](https://github.com/keith-hall)) - Add syntax definition and test file for GDScript highlighting, see [#​3236](sharkdp/bat#3236) ([@​chetanjangir0](https://github.com/chetanjangir0)) - Add syntax test file for Odin highlighting, see [#​3241](sharkdp/bat#3241) ([@​chetanjangir0](https://github.com/chetanjangir0)) - Update quadlet syntax mapping rules to cover quadlets in subdirectories [#​3299](sharkdp/bat#3299) ([@​cyqsimon](https://github.com/cyqsimon)) - Add syntax Typst [#​3300](sharkdp/bat#3300) ([@​cskeeters](https://github.com/cskeeters)) - Map `.mill` files to Scala syntax for Mill build tool configuration files [#​3311](sharkdp/bat#3311) ([@​krikera](https://github.com/krikera)) - Add syntax highlighting for VHDL, see [#​3337](sharkdp/bat#3337) ([@​JerryImMouse](https://github.com/JerryImMouse)) - Add syntax mapping for certbot certificate configuration [#​3338](sharkdp/bat#3338) ([@​cyqsimon](https://github.com/cyqsimon)) - Update Lean syntax from Lean 3 to Lean 4 [#​3322](sharkdp/bat#3322) ([@​YDX-2147483647](https://github.com/YDX-2147483647)) - Map `.flatpakref` and `.flatpakrepo` files to INI syntax [#​3353](sharkdp/bat#3353) ([@​Ferenc-](https://github.com/Ferenc-)) - Update hosts syntax [#​3368](sharkdp/bat#3368) ([@​keith-hall](https://github.com/keith-hall)) - Map `.kshrc` files to Bash syntax [#​3364](sharkdp/bat#3364) ([@​ritoban23](https://github.com/ritoban23)) - Map `/var/log/dmesg` files to Syslog syntax [#​3412](sharkdp/bat#3412) ([@​keith-hall](https://github.com/keith-hall)) - Add syntax definition and test file for Go modules(`go.mod` and `go.sum`) highlighting, see [#​3424](sharkdp/bat#3424) ([@​DarkMatter-999](https://github.com/DarkMatter-999)) - Syntax highlighting for typescript code blocks within Markdown files, see [#​3435](sharkdp/bat#3435) ([@​MuntasirSZN](https://github.com/MuntasirSZN)) #### Themes - Add Catppuccin, see [#​3317](sharkdp/bat#3317) ([@​SchweGELBin](https://github.com/SchweGELBin)) - Updated Catppuccin, see [#​3333](sharkdp/bat#3333) ([@​SchweGELBin](https://github.com/SchweGELBin)) - Updated gruvbox, see [#​3372](sharkdp/bat#3372) ([@​Nicholas42](https://github.com/Nicholas42)) - Updated GitHub theme, see [#​3382](sharkdp/bat#3382) ([@​CosmicHorrorDev](https://github.com/CosmicHorrorDev)) - Updated ANSI theme to highlight JSON object keys differently from values, see [#​3413](sharkdp/bat#3413) ([@​keith-hall](https://github.com/keith-hall)) </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. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xNTEuMSIsInVwZGF0ZWRJblZlciI6IjQxLjE1Mi45IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Background
I recently noticed I cannot pipe
bat -noutput anywhere (e.g. cat, other Unix tools, the clipboard via xclip) and get line numbers likenl, it just looks likecatoutput.I have come across these duplicates, there may be other reports of the same thing:
--styledocumentation is misleading #3316I found there was an abandoned PR with the same discussion (#2983) and someone proposed a new approach, but it never got extended to a full solution.
Since that PR, lots of the machinery it proposed was added (passing style components through in the
loop_throughcase which occurs when piped), which made this easier this time round.I've made sure to add tests and to preserve the standard behaviour (the
--style=autocase, which appears to be the CLI default, not to be confused with--style=defaultwhich is set explicitly and behaves likeautobut the docs highlight that auto should be expected to pipe as plain)Description
Gate the use of the SimplePrinter onplaintooloop_throughvariable) on:numberflag being passedstyleflag being passed withnumbersin the value, or anything exceptautoBugs fixed
-n/--numbershould pipe--style=numbersshould pipe--style=header,grid,numbersshould pipe--style=defaultshould pipe--style=autoshould continue to not pipe (retained existing behaviour, not a bug fix)Test cases
-n/--numberflag--styleequivalently (as--style=numbers)--style=header,grid,numbers)Integration tests added:
Click to show demos of existing bat behaviour
Demo
No flags (implicit style=auto)
bat Cargo.toml -r :2 | catBefore:
After: (Unchanged)
Plain
bat Cargo.toml -p -r :4 | catBefore:
After: (Unchanged)
Line numbers only
bat Cargo.toml -n -r :4 | catorbat Cargo.toml --style=numbers -r :4 | catBefore: (As for plain)
After:
Full style (explicit default)
bat Cargo.toml --style=default -r :4 | catBefore: (As for plain)
After: