diff --git a/CHANGELOG.md b/CHANGELOG.md index 76023d49..c7b7c05d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Fix negative values of N not being parsed in line ranges without `=` flag value separator, see #3442 (@lmmx) - Fix broken Docker syntax preventing use of custom assets, see #3476 (@keith-hall) - Fix decorations being applied unexpectedly when piping. Now only line numbers explicitly required on the command line should be applied in auto decorations mode for `cat` compatibility. See #3496 (@keith-hall) +- `--help` now correctly reads the config file for theme information etc. See #3507 (@keith-hall) ## Other - Improve README documentation on pager options passed to less, see #3443 (@injust) diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index f8b429cb..bdfca75e 100644 --- a/src/bin/bat/app.rs +++ b/src/bin/bat/app.rs @@ -186,6 +186,20 @@ impl App { Ok(()) } + /// Build argument list with env vars and CLI args (without config file) + fn build_args_without_config() -> Vec { + let mut cli_args = wild::args_os(); + let mut args = get_args_from_env_vars(); + + // Put the zero-th CLI argument (program name) first + args.insert(0, cli_args.next().unwrap()); + + // .. and the rest at the end + cli_args.for_each(|a| args.push(a)); + + args + } + fn matches(interactive_output: bool) -> Result { // Check if we should skip config file processing for special arguments // that don't require full application setup (version, diagnostic) @@ -196,63 +210,65 @@ impl App { ) }); - // Check if help was requested - help should go through the same code path - // but be forgiving of config file errors + // Check if help was requested - help should read the config file but be + // forgiving of invalid arguments (so configured theme etc. can be used) let help_requested = wild::args_os().any(|arg| matches!(arg.to_str(), Some("-h" | "--help"))); - let args = if wild::args_os().nth(1) == Some("cache".into()) { + if wild::args_os().nth(1) == Some("cache".into()) { // Skip the config file and env vars + let args = wild::args_os().collect::>(); + return Ok(clap_app::build_app(interactive_output).get_matches_from(args)); + } - wild::args_os().collect::>() - } else if wild::args_os().any(|arg| arg == "--no-config") || should_skip_config { + if wild::args_os().any(|arg| arg == "--no-config") || should_skip_config { // Skip the arguments in bats config file when --no-config is present // or when user requests version or diagnostic information + let args = Self::build_args_without_config(); + return Ok(clap_app::build_app(interactive_output).get_matches_from(args)); + } - let mut cli_args = wild::args_os(); - let mut args = get_args_from_env_vars(); + // Build arguments with config file + let mut cli_args = wild::args_os(); - // Put the zero-th CLI argument (program name) first - args.insert(0, cli_args.next().unwrap()); - - // .. and the rest at the end - cli_args.for_each(|a| args.push(a)); - - args - } else if help_requested { - // Help goes through the normal config path but only uses env vars for themes - // to avoid failing on invalid config options - let mut cli_args = wild::args_os(); - let mut args = get_args_from_env_vars(); - - // Put the zero-th CLI argument (program name) first - args.insert(0, cli_args.next().unwrap()); - - // .. and the rest at the end (includes --help and other CLI args) - cli_args.for_each(|a| args.push(a)); - args - } else { - let mut cli_args = wild::args_os(); - - // Read arguments from bats config file - let mut args = match get_args_from_env_opts_var() { - Some(result) => result, - None => get_args_from_config_file(), - } - .map_err(|_| "Could not parse configuration file")?; - - // Selected env vars supersede config vars - args.extend(get_args_from_env_vars()); - - // Put the zero-th CLI argument (program name) first - args.insert(0, cli_args.next().unwrap()); - - // .. and the rest at the end - cli_args.for_each(|a| args.push(a)); - args + // Read arguments from bats config file + let config_args = match get_args_from_env_opts_var() { + Some(result) => result, + None => get_args_from_config_file(), }; - Ok(clap_app::build_app(interactive_output).get_matches_from(args)) + // For help, ignore config file parse errors (use empty config instead) + // For non-help, propagate the error + let mut args = if help_requested { + config_args.unwrap_or_default() + } else { + config_args.map_err(|_| "Could not parse configuration file")? + }; + + // Selected env vars supersede config vars + args.extend(get_args_from_env_vars()); + + // Put the zero-th CLI argument (program name) first + args.insert(0, cli_args.next().unwrap()); + + // .. and the rest at the end + cli_args.for_each(|a| args.push(a)); + + // For help, try parsing with config, and if clap fails (e.g., invalid + // argument in config), fall back to parsing without config file args + if help_requested { + let app = clap_app::build_app(interactive_output); + match app.try_get_matches_from(args) { + Ok(matches) => Ok(matches), + Err(_) => { + // Config has invalid arguments, fall back to just env vars + CLI args + let fallback_args = Self::build_args_without_config(); + Ok(clap_app::build_app(interactive_output).get_matches_from(fallback_args)) + } + } + } else { + Ok(clap_app::build_app(interactive_output).get_matches_from(args)) + } } pub fn config(&self, inputs: &[Input]) -> Result> { diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 8a653818..9a3d67ce 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1479,6 +1479,28 @@ fn help_works_with_invalid_config() { .stdout(predicate::str::contains("A cat(1) clone with wings")); } +#[test] +fn help_uses_valid_config() { + let tmp_dir = tempdir().expect("can create temporary directory"); + let tmp_config_path = tmp_dir.path().join("valid-config.conf"); + + // Write a valid config file with an invalid theme (so we can see a warning) + std::fs::write(&tmp_config_path, "--theme=NonExistentThemeName123") + .expect("can write config file"); + + // --help should read the config file and try to use the theme + // (we'll see a warning about unknown theme. This is the easiest way to prove the theme is read from config.) + bat_with_config() + .env("BAT_CONFIG_PATH", tmp_config_path.to_str().unwrap()) + .arg("--help") + .assert() + .success() + .stdout(predicate::str::contains( + "A cat(1) clone with syntax highlighting", + )) + .stderr(predicate::str::contains("Unknown theme")); +} + #[test] fn version_works_with_invalid_config() { let tmp_dir = tempdir().expect("can create temporary directory");