Exit quietly on broken pipe in auxiliary output paths

The main output path has exited with status 0 on EPIPE since #232, but
auxiliary outputs still used println!/print!/io::stdout().flush().unwrap()
and panicked (exit code 101) when the reader of a piped stdout went away
early, e.g. 'bat --completion bash | head -n0'.

Route them through the existing convention instead: write!/writeln! to
io::stdout() and '?' so that Error::Io(BrokenPipe) reaches
default_error_handler, which exits with status 0. This covers
--completion, --config-file, --diagnostic (bugreport's print() becomes
format() + writeln!), --generate-config-file, the 'cache' subcommand
progress messages, the OSC title sequence of --set-terminal-title, and
the 'built without feature' notices.

Closes #4034
This commit is contained in:
Mathjk
2026-10-02 02:23:08 +08:00
parent 4608fc959a
commit a9b112b6fe
6 changed files with 133 additions and 50 deletions
+1
View File
@@ -25,6 +25,7 @@
- Syntax highlighting for Python files using uv as script runner in shebang #3689 (@janlarres)
## Bugfixes
- Exit quietly with status 0 instead of panicking when stdout is closed early (broken pipe) on auxiliary output paths such as `--completion`, `--config-file`, `--diagnostic`, `--generate-config-file`, `--set-terminal-title` and `cache --build`/`--clear`, matching the main output path since #232. Closes #4034 (@Mathjk)
- Track strikethrough (SGR 9 / 29) in the ANSI style tracker so it is re-emitted like bold, dim, italic, and underline, and is no longer dropped after the first highlighted token when displaying ANSI input with syntax highlighting, see #4015 (@mikehasa)
- Allow boolean flags to be given more than once, so that a flag set in the config file can also be passed on the command line without erroring, see #3912 (@logarithmone1128)
- Use parsed CLI arguments to detect number flags, see #3908 (@cuishuang)
+25 -15
View File
@@ -1,4 +1,5 @@
use std::convert::TryInto;
use std::io::{self, Write};
use std::path::Path;
use syntect::highlighting::ThemeSet;
@@ -24,7 +25,7 @@ pub fn build(
let acknowledgements = build_acknowledgements(source_dir, include_acknowledgements)?;
print_unlinked_contexts(&syntax_set);
print_unlinked_contexts(&syntax_set)?;
write_assets(
&theme_set,
@@ -46,16 +47,18 @@ fn build_theme_set(source_dir: &Path, include_integrated_assets: bool) -> Result
if theme_dir.exists() {
let res = theme_set.add_from_folder(&theme_dir);
if let Err(err) = res {
println!(
writeln!(
io::stdout(),
"Failed to load one or more themes from '{}' (reason: '{err}')",
theme_dir.to_string_lossy(),
);
)?;
}
} else {
println!(
writeln!(
io::stdout(),
"No themes were found in '{}', using the default set",
theme_dir.to_string_lossy()
);
)?;
}
theme_set.try_into()
@@ -78,23 +81,25 @@ fn build_syntax_set_builder(
if syntax_dir.exists() {
syntax_set_builder.add_from_folder(syntax_dir, true)?;
} else {
println!(
writeln!(
io::stdout(),
"No syntaxes were found in '{}', using the default set.",
syntax_dir.to_string_lossy()
);
)?;
}
Ok(syntax_set_builder)
}
fn print_unlinked_contexts(syntax_set: &SyntaxSet) {
fn print_unlinked_contexts(syntax_set: &SyntaxSet) -> Result<()> {
let missing_contexts = syntax_set.find_unlinked_contexts();
if !missing_contexts.is_empty() {
println!("Some referenced contexts could not be found!");
writeln!(io::stdout(), "Some referenced contexts could not be found!")?;
for context in missing_contexts {
println!("- {context}");
writeln!(io::stdout(), "- {context}")?;
}
}
Ok(())
}
fn write_assets(
@@ -127,12 +132,13 @@ fn write_assets(
)?;
}
print!(
write!(
io::stdout(),
"Writing metadata to folder {} ... ",
target_dir.to_string_lossy()
);
)?;
crate::assets_metadata::AssetsMetadata::new(current_version).save_to_folder(target_dir)?;
println!("okay");
writeln!(io::stdout(), "okay")?;
Ok(())
}
@@ -161,10 +167,14 @@ fn asset_to_cache<T: serde::Serialize>(
description: &str,
compressed: bool,
) -> Result<()> {
print!("Writing {description} to {} ... ", path.to_string_lossy());
write!(
io::stdout(),
"Writing {description} to {} ... ",
path.to_string_lossy()
)?;
let contents = asset_to_contents(asset, description, compressed)?;
std::fs::write(path, &contents[..])
.map_err(|_| format!("Could not save {description} to {}", path.to_string_lossy()))?;
println!("okay");
writeln!(io::stdout(), "okay")?;
Ok(())
}
+15 -10
View File
@@ -1,5 +1,5 @@
use std::fs;
use std::io;
use std::io::{self, Write};
use std::path::Path;
use std::path::PathBuf;
@@ -9,10 +9,11 @@ use bat::assets::HighlightingAssets;
use bat::assets_metadata::AssetsMetadata;
use bat::error::*;
pub fn clear_assets(cache_dir: &Path) {
clear_asset(cache_dir.join("themes.bin"), "theme set cache");
clear_asset(cache_dir.join("syntaxes.bin"), "syntax set cache");
clear_asset(cache_dir.join("metadata.yaml"), "metadata file");
pub fn clear_assets(cache_dir: &Path) -> Result<()> {
clear_asset(cache_dir.join("themes.bin"), "theme set cache")?;
clear_asset(cache_dir.join("syntaxes.bin"), "syntax set cache")?;
clear_asset(cache_dir.join("metadata.yaml"), "metadata file")?;
Ok(())
}
pub fn assets_from_cache_or_binary(
@@ -43,15 +44,19 @@ pub fn assets_from_cache_or_binary(
Ok(custom_assets.unwrap_or_else(HighlightingAssets::from_binary))
}
fn clear_asset(path: PathBuf, description: &str) {
print!("Clearing {description} ... ");
fn clear_asset(path: PathBuf, description: &str) -> Result<()> {
write!(io::stdout(), "Clearing {description} ... ")?;
match fs::remove_file(&path) {
Err(err) if err.kind() == io::ErrorKind::NotFound => {
println!("skipped (not present)");
writeln!(io::stdout(), "skipped (not present)")?;
}
Err(err) => {
println!("could not remove the cache file {path:?}: {err}");
writeln!(
io::stdout(),
"could not remove the cache file {path:?}: {err}"
)?;
}
Ok(_) => println!("okay"),
Ok(_) => writeln!(io::stdout(), "okay")?,
}
Ok(())
}
+7 -5
View File
@@ -32,12 +32,13 @@ pub fn config_file() -> PathBuf {
pub fn generate_config_file() -> bat::error::Result<()> {
let config_file = config_file();
if config_file.is_file() {
println!(
writeln!(
io::stdout(),
"A config file already exists at: {}",
config_file.to_string_lossy()
);
)?;
print!("Overwrite? (y/N): ");
write!(io::stdout(), "Overwrite? (y/N): ")?;
io::stdout().flush()?;
let mut decision = String::new();
io::stdin().read_line(&mut decision)?;
@@ -93,10 +94,11 @@ pub fn generate_config_file() -> bat::error::Result<()> {
)
})?;
println!(
writeln!(
io::stdout(),
"Success! Config file written to {}",
config_file.to_string_lossy()
);
)?;
Ok(())
}
+23 -17
View File
@@ -74,9 +74,9 @@ fn run_cache_subcommand(
#[cfg(feature = "build-assets")]
build_assets(matches, config_dir, cache_dir)?;
#[cfg(not(feature = "build-assets"))]
println!("bat has been built without the 'build-assets' feature. The 'cache --build' option is not available.");
writeln!(io::stdout(), "bat has been built without the 'build-assets' feature. The 'cache --build' option is not available.")?;
} else if matches.get_flag("clear") {
clear_assets(cache_dir);
clear_assets(cache_dir)?;
}
Ok(())
@@ -262,13 +262,17 @@ pub fn list_themes(
Ok(())
}
fn set_terminal_title_to(new_terminal_title: String) {
fn set_terminal_title_to(new_terminal_title: String) -> Result<()> {
let osc_command_for_setting_terminal_title = "\x1b]0;";
let osc_end_command = "\x07";
// Prevent BEL/ESC/C1 bytes in the title from terminating or nesting the OSC.
let safe_title = bat::sanitize_for_terminal(&new_terminal_title);
print!("{osc_command_for_setting_terminal_title}{safe_title}{osc_end_command}");
io::stdout().flush().unwrap();
write!(
io::stdout(),
"{osc_command_for_setting_terminal_title}{safe_title}{osc_end_command}"
)?;
io::stdout().flush()?;
Ok(())
}
fn get_new_terminal_title(inputs: &Vec<Input>) -> String {
@@ -286,13 +290,13 @@ fn run_controller(inputs: Vec<Input>, config: &Config, cache_dir: &Path) -> Resu
let assets = assets_from_cache_or_binary(config.use_custom_assets, cache_dir)?;
let controller = Controller::new(config, &assets);
if config.paging_mode != PagingMode::Never && config.set_terminal_title {
set_terminal_title_to(get_new_terminal_title(&inputs));
set_terminal_title_to(get_new_terminal_title(&inputs))?;
}
controller.run(inputs, None)
}
#[cfg(feature = "bugreport")]
fn invoke_bugreport(app: &App, cache_dir: &Path) {
fn invoke_bugreport(app: &App, cache_dir: &Path) -> Result<()> {
use bugreport::{bugreport, collector::*, format::Markdown, report::ReportEntry};
struct ColorSchemeCollector;
@@ -370,7 +374,8 @@ fn invoke_bugreport(app: &App, cache_dir: &Path) {
))
};
report.print::<Markdown>();
writeln!(io::stdout(), "{}", report.format::<Markdown>())?;
Ok(())
}
/// Returns `Err(..)` upon fatal errors. Otherwise, returns `Ok(true)` on full success and
@@ -382,21 +387,22 @@ fn run() -> Result<bool> {
if app.matches.get_flag("diagnostic") {
#[cfg(feature = "bugreport")]
invoke_bugreport(&app, cache_dir);
invoke_bugreport(&app, cache_dir)?;
#[cfg(not(feature = "bugreport"))]
println!("bat has been built without the 'bugreport' feature. The '--diagnostic' option is not available.");
writeln!(io::stdout(), "bat has been built without the 'bugreport' feature. The '--diagnostic' option is not available.")?;
return Ok(true);
}
#[cfg(feature = "application")]
if let Some(shell) = app.matches.get_one::<String>("completion") {
match shell.as_str() {
"bash" => println!("{}", completions::BASH_COMPLETION),
"fish" => println!("{}", completions::FISH_COMPLETION),
"ps1" => println!("{}", completions::PS1_COMPLETION),
"zsh" => println!("{}", completions::ZSH_COMPLETION),
let completion = match shell.as_str() {
"bash" => completions::BASH_COMPLETION,
"fish" => completions::FISH_COMPLETION,
"ps1" => completions::PS1_COMPLETION,
"zsh" => completions::ZSH_COMPLETION,
_ => unreachable!("No completion for shell '{shell}' available."),
}
};
writeln!(io::stdout(), "{completion}")?;
return Ok(true);
}
@@ -435,7 +441,7 @@ fn run() -> Result<bool> {
list_themes(&config, config_dir, cache_dir, app.theme_options())?;
Ok(true)
} else if app.matches.get_flag("config-file") {
println!("{}", config_file().to_string_lossy());
writeln!(io::stdout(), "{}", config_file().to_string_lossy())?;
Ok(true)
} else if app.matches.get_flag("generate-config-file") {
generate_config_file()?;
+62 -3
View File
@@ -25,10 +25,8 @@ mod unix {
use unix::*;
mod utils;
use utils::command::{bat, bat_with_config};
use utils::command::{bat, bat_raw_command, bat_raw_command_with_config, bat_with_config};
#[cfg(unix)]
use utils::command::bat_raw_command;
use utils::mocked_pagers;
const EXAMPLES_DIR: &str = "tests/examples";
@@ -60,6 +58,67 @@ fn stdin() {
.stdout("foo\nbar\n");
}
/// Runs `bat` with `args` while its stdout is a pipe whose read end has
/// already been closed, so the very first write fails with `BrokenPipe`
/// (EPIPE on Unix, `ERROR_NO_DATA` on Windows).
fn assert_quiet_exit_when_stdout_is_closed(command: &mut std::process::Command, args: &[&str]) {
use std::process::Stdio;
let (pipe_reader, pipe_writer) = std::io::pipe().expect("could not create a pipe");
drop(pipe_reader);
let output = command
.args(args)
.stdin(Stdio::null())
.stdout(pipe_writer)
.stderr(Stdio::piped())
.spawn()
.and_then(|child| child.wait_with_output())
.expect("could not run bat");
assert!(
output.status.success(),
"bat {args:?} should exit quietly when stdout is closed, status: {}, stderr: {}",
output.status,
String::from_utf8_lossy(&output.stderr),
);
assert!(
output.stderr.is_empty(),
"bat {args:?} should not print to stderr when stdout is closed, stderr: {}",
String::from_utf8_lossy(&output.stderr),
);
}
#[test]
fn auxiliary_output_does_not_panic_on_closed_stdout() {
// Auxiliary outputs used to be written via `println!`, which panics when
// the reader of a piped stdout is already gone (e.g. `bat --config-file |
// head -n 0`). The main output path has exited quietly on `BrokenPipe`
// since #232; auxiliary outputs should behave the same way.
assert_quiet_exit_when_stdout_is_closed(&mut bat_raw_command(), &["--config-file"]);
assert_quiet_exit_when_stdout_is_closed(&mut bat_raw_command(), &["--diagnostic"]);
// The `cache` subcommand is only enabled when the working directory does
// not contain a file named `cache` (see `cache_clear`), and it conflicts
// with top-level flags like `--no-config`. Run it from a temporary
// directory that doubles as `BAT_CACHE_PATH`, leaving the real cache alone.
let temp_dir = tempdir().expect("could not create a temporary directory");
let mut command = bat_raw_command_with_config();
command
.current_dir(temp_dir.path())
.env("BAT_CONFIG_PATH", temp_dir.path().join("config"))
.env("BAT_CACHE_PATH", temp_dir.path());
assert_quiet_exit_when_stdout_is_closed(&mut command, &["cache", "--clear"]);
}
#[cfg(feature = "application")]
#[test]
fn completion_does_not_panic_on_closed_stdout() {
for shell in ["bash", "fish", "ps1", "zsh"] {
assert_quiet_exit_when_stdout_is_closed(&mut bat_raw_command(), &["--completion", shell]);
}
}
#[test]
fn concatenate() {
bat()