From 4f5e3ebbd1106c5f8e418b5743e2166576c9f0a8 Mon Sep 17 00:00:00 2001 From: Thomas Otto Date: Mon, 18 Oct 2021 17:15:58 +0200 Subject: Use fatal() to exit with errorcode 2 (#739) * Fix two typos * Dismantle two Pyramids of Doom Use question mark operator instead * Use fatal() to exit with errorcode 2 --- src/bat_utils/output.rs | 6 +++--- src/cli.rs | 2 +- src/color.rs | 5 ++--- src/config.rs | 17 ++++++----------- src/git_config/mod.rs | 6 +++--- src/handlers/blame.rs | 43 +++++++++++++++++++----------------------- src/handlers/diff_stat.rs | 24 ++++++++++------------- src/main.rs | 2 ++ src/options/rewrite.rs | 20 ++++++++------------ src/options/set.rs | 19 +++++++------------ src/parse_style.rs | 21 ++++++++------------- src/subcommands/show_config.rs | 2 +- 12 files changed, 70 insertions(+), 97 deletions(-) (limited to 'src') diff --git a/src/bat_utils/output.rs b/src/bat_utils/output.rs index 51819787..a5251a0c 100644 --- a/src/bat_utils/output.rs +++ b/src/bat_utils/output.rs @@ -10,6 +10,7 @@ use std::process::{Child, Command, Stdio}; use super::less::retrieve_less_version; use crate::config; +use crate::fatal; use crate::features::navigate; #[derive(Debug, Clone, Copy, PartialEq)] @@ -177,14 +178,13 @@ fn _make_process_from_less_path( fn _make_process_from_pager_path(pager_path: PathBuf, args: &[String]) -> Option { if pager_path.file_stem() == Some(&OsString::from("delta")) { - eprintln!( + fatal( "\ It looks like you have set delta as the value of $PAGER. \ This would result in a non-terminating recursion. \ delta is not an appropriate value for $PAGER \ -(but it is an appropriate value for $GIT_PAGER)." +(but it is an appropriate value for $GIT_PAGER).", ); - std::process::exit(1); } if let Ok(pager_path) = grep_cli::resolve_binary(pager_path) { let mut p = Command::new(&pager_path); diff --git a/src/cli.rs b/src/cli.rs index 1281bd9a..12db6e2a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -443,7 +443,7 @@ pub struct Opt { /// Default language used for syntax highlighting when this cannot be /// inferred from a filename. It will typically make sense to set this in - /// per-repository git config ().git/config) + /// per-repository git config (.git/config) #[structopt(long = "default-language")] pub default_language: Option, diff --git a/src/color.rs b/src/color.rs index e7c3f62f..dfa86fa7 100644 --- a/src/color.rs +++ b/src/color.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::process; use std::str::FromStr; use ansi_term::Color; @@ -7,6 +6,7 @@ use lazy_static::lazy_static; use syntect::highlighting::Color as SyntectColor; use crate::bat_utils::terminal::to_ansi_color; +use crate::fatal; use crate::syntect_utils; pub fn parse_color(s: &str, true_color: bool) -> Option { @@ -14,8 +14,7 @@ pub fn parse_color(s: &str, true_color: bool) -> Option { return None; } let die = || { - eprintln!("Invalid color or style attribute: {}", s); - process::exit(1); + fatal(format!("Invalid color or style attribute: {}", s)); }; let syntect_color = if s.starts_with('#') { SyntectColor::from_str(s).unwrap_or_else(|_| die()) diff --git a/src/config.rs b/src/config.rs index 4074551b..15860a80 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; use std::path::PathBuf; -use std::process; use regex::Regex; use structopt::clap; @@ -185,23 +184,21 @@ impl From for Config { .unwrap_or(0.0); let commit_regex = Regex::new(&opt.commit_regex).unwrap_or_else(|_| { - eprintln!( + fatal(format!( "Invalid commit-regex: {}. \ The value must be a valid Rust regular expression. \ See https://docs.rs/regex.", opt.commit_regex - ); - process::exit(1); + )); }); let tokenization_regex = Regex::new(&opt.tokenization_regex).unwrap_or_else(|_| { - eprintln!( + fatal(format!( "Invalid word-diff-regex: {}. \ The value must be a valid Rust regular expression. \ See https://docs.rs/regex.", opt.tokenization_regex - ); - process::exit(1); + )); }); let side_by_side_data = side_by_side::SideBySideData::new_sbs( @@ -613,13 +610,11 @@ pub fn user_supplied_option(option: &str, arg_matches: &clap::ArgMatches) -> boo } pub fn delta_unreachable(message: &str) -> ! { - let error_exit_code = 2; // This is also stored in Config. - eprintln!( + fatal(format!( "{} This should not be possible. \ Please report the bug at https://github.com/dandavison/delta/issues.", message - ); - process::exit(error_exit_code); + )); } #[cfg(test)] diff --git a/src/git_config/mod.rs b/src/git_config/mod.rs index 1b2ea0a6..9b9f11ff 100644 --- a/src/git_config/mod.rs +++ b/src/git_config/mod.rs @@ -7,7 +7,8 @@ use std::collections::HashMap; use std::env; #[cfg(test)] use std::path::Path; -use std::process; + +use crate::fatal; use lazy_static::lazy_static; @@ -31,8 +32,7 @@ impl GitConfig { match config { Some(mut config) => { let config = config.snapshot().unwrap_or_else(|err| { - eprintln!("Failed to read git config: {}", err); - process::exit(1) + fatal(format!("Failed to read git config: {}", err)); }); Some(Self { config, diff --git a/src/handlers/blame.rs b/src/handlers/blame.rs index 2c1b2222..31fb1315 100644 --- a/src/handlers/blame.rs +++ b/src/handlers/blame.rs @@ -109,30 +109,25 @@ $ } pub fn parse_git_blame_line<'a>(line: &'a str, timestamp_format: &str) -> Option> { - if let Some(caps) = BLAME_LINE_REGEX.captures(line) { - let commit = caps.get(1).unwrap().as_str(); - let author = caps.get(2).unwrap().as_str(); - let timestamp = caps.get(3).unwrap().as_str(); - if let Ok(time) = DateTime::parse_from_str(timestamp, timestamp_format) { - let line_number_str = caps.get(4).unwrap().as_str(); - if let Ok(line_number) = line_number_str.parse::() { - let code = caps.get(5).unwrap().as_str(); - Some(BlameLine { - commit, - author, - time, - line_number, - code, - }) - } else { - None - } - } else { - None - } - } else { - None - } + let caps = BLAME_LINE_REGEX.captures(line)?; + + let commit = caps.get(1).unwrap().as_str(); + let author = caps.get(2).unwrap().as_str(); + let timestamp = caps.get(3).unwrap().as_str(); + + let time = DateTime::parse_from_str(timestamp, timestamp_format).ok()?; + + let line_number = caps.get(4).unwrap().as_str().parse::().ok()?; + + let code = caps.get(5).unwrap().as_str(); + + Some(BlameLine { + commit, + author, + time, + line_number, + code, + }) } lazy_static! { diff --git a/src/handlers/diff_stat.rs b/src/handlers/diff_stat.rs index 39a2130f..d8e9c847 100644 --- a/src/handlers/diff_stat.rs +++ b/src/handlers/diff_stat.rs @@ -46,20 +46,16 @@ pub fn relativize_path_in_diff_stat_line( cwd_relative_to_repo_root: &str, diff_stat_align_width: usize, ) -> Option { - if let Some(caps) = DIFF_STAT_LINE_REGEX.captures(line) { - let path_relative_to_repo_root = caps.get(1).unwrap().as_str(); - if let Some(relative_path) = - pathdiff::diff_paths(path_relative_to_repo_root, cwd_relative_to_repo_root) - { - if let Some(relative_path) = relative_path.to_str() { - let suffix = caps.get(2).unwrap().as_str(); - let pad_width = diff_stat_align_width.saturating_sub(relative_path.len()); - let padding = " ".repeat(pad_width); - return Some(format!(" {}{}{}", relative_path, padding, suffix)); - } - } - } - None + let caps = DIFF_STAT_LINE_REGEX.captures(line)?; + let path_relative_to_repo_root = caps.get(1).unwrap().as_str(); + + let relative_path = + pathdiff::diff_paths(path_relative_to_repo_root, cwd_relative_to_repo_root)?; + let relative_path = relative_path.to_str()?; + let suffix = caps.get(2).unwrap().as_str(); + let pad_width = diff_stat_align_width.saturating_sub(relative_path.len()); + let padding = " ".repeat(pad_width); + Some(format!(" {}{}{}", relative_path, padding, suffix)) } #[cfg(test)] diff --git a/src/main.rs b/src/main.rs index 22f0c7aa..0955f90d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -45,6 +45,8 @@ where #[cfg(not(test))] { eprintln!("{}", errmsg); + // As in Config::error_exit_code: use 2 for error + // because diff uses 0 and 1 for non-error. process::exit(2); } #[cfg(test)] diff --git a/src/options/rewrite.rs b/src/options/rewrite.rs index 1c4759e6..11bd0639 100644 --- a/src/options/rewrite.rs +++ b/src/options/rewrite.rs @@ -2,12 +2,11 @@ /// 1. Express deprecated usages in the new non-deprecated form /// 2. Implement options such as --raw which are defined to be equivalent to some set of /// other options. -use std::process; - use structopt::clap; use crate::cli; use crate::config::user_supplied_option; +use crate::fatal; pub fn apply_rewrite_rules(opt: &mut cli::Opt, arg_matches: &clap::ArgMatches) { rewrite_style_strings_to_honor_deprecated_minus_plus_options(opt); @@ -102,10 +101,9 @@ fn rewrite_options_to_implement_deprecated_hunk_style_option(opt: &mut cli::Opt) // apparently been left at its default value. let hunk_header_decoration_default = "blue box"; if opt.hunk_header_decoration_style != hunk_header_decoration_default { - eprintln!( - "Deprecated option --hunk-style cannot be used with --hunk-header-decoration-style. \ - Use --hunk-header-decoration-style."); - process::exit(1); + fatal( + "Deprecated option --hunk-style cannot be used with --hunk-header-decoration-style. \ + Use --hunk-header-decoration-style."); } match opt.deprecated_hunk_style.as_deref().map(str::to_lowercase) { Some(attr) if attr == "plain" => opt.hunk_header_decoration_style = "".to_string(), @@ -163,25 +161,23 @@ fn _get_rewritten_minus_plus_style_string( ))) } (_, (_, Some(_))) => { - eprintln!( + fatal(format!( "--{name}-color cannot be used with --{name}-style. \ Use --{name}-style=\"fg bg attr1 attr2 ...\" to set \ foreground color, background color, and style attributes. \ --{name}-color can only be used to set the background color. \ (It is still available for backwards-compatibility.)", name = element_name, - ); - process::exit(1); + )); } (_, (Some(_), None)) => { - eprintln!( + fatal(format!( "Deprecated option --highlight-removed cannot be used with \ --{name}-style. Use --{name}-style=\"fg bg attr1 attr2 ...\" \ to set foreground color, background color, and style \ attributes.", name = element_name, - ); - process::exit(1); + )); } } } diff --git a/src/options/set.rs b/src/options/set.rs index 18c10118..c8a700e2 100644 --- a/src/options/set.rs +++ b/src/options/set.rs @@ -1,6 +1,5 @@ use std::collections::{HashMap, HashSet, VecDeque}; use std::convert::TryInto; -use std::process; use std::result::Result; use std::str::FromStr; @@ -230,8 +229,7 @@ fn set__light__dark__syntax_theme__options( ) { let validate_light_and_dark = |opt: &cli::Opt| { if opt.light && opt.dark { - eprintln!("--light and --dark cannot be used together."); - process::exit(1); + fatal("--light and --dark cannot be used together."); } }; let empty_builtin_features = HashMap::new(); @@ -498,11 +496,10 @@ impl FromStr for cli::InspectRawLines { "true" => Ok(Self::True), "false" => Ok(Self::False), _ => { - eprintln!( + fatal(format!( r#"Invalid value for inspect-raw-lines option: {}. Valid values are "true", and "false"."#, s - ); - process::exit(1); + )); } } } @@ -514,11 +511,10 @@ fn parse_paging_mode(paging_mode_string: &str) -> PagingMode { "never" => PagingMode::Never, "auto" => PagingMode::QuitIfOneScreen, _ => { - eprintln!( + fatal(format!( "Invalid value for --paging option: {} (valid values are \"always\", \"never\", and \"auto\")", paging_mode_string - ); - process::exit(1); + )); } } } @@ -610,11 +606,10 @@ fn set_true_color(opt: &mut cli::Opt) { "never" => false, "auto" => is_truecolor_terminal(), _ => { - eprintln!( + fatal(format!( "Invalid value for --true-color option: {} (valid values are \"always\", \"never\", and \"auto\")", opt.true_color - ); - process::exit(1); + )); } }; } diff --git a/src/parse_style.rs b/src/parse_style.rs index 1c20c981..49299fa9 100644 --- a/src/parse_style.rs +++ b/src/parse_style.rs @@ -1,9 +1,8 @@ -use std::process; - use bitflags::bitflags; use crate::color; use crate::config::delta_unreachable; +use crate::fatal; use crate::style::{DecorationStyle, Style}; impl Style { @@ -136,12 +135,10 @@ impl DecorationStyle { let (style, is_omitted, is_raw, is_syntax_highlighted) = parse_ansi_term_style(&style_string, None, true_color); if is_raw { - eprintln!("'raw' may not be used in a decoration style."); - process::exit(1); + fatal("'raw' may not be used in a decoration style."); }; if is_syntax_highlighted { - eprintln!("'syntax' may not be used in a decoration style."); - process::exit(1); + fatal("'syntax' may not be used in a decoration style."); }; #[allow(non_snake_case)] let (BOX, UL, OL, EMPTY) = ( @@ -256,12 +253,11 @@ fn parse_ansi_term_style( seen_foreground = true; } else if !seen_background { if word == "syntax" { - eprintln!( + fatal( "You have used the special color 'syntax' as a background color \ - (second color in a style string). It may only be used as a foreground \ - color (first color in a style string)." + (second color in a style string). It may only be used as a foreground \ + color (first color in a style string).", ); - process::exit(1); } else if word == "auto" { background_is_auto = true; style.background = default.and_then(|s| s.ansi_term_style.background); @@ -270,11 +266,10 @@ fn parse_ansi_term_style( } seen_background = true; } else { - eprintln!( + fatal(format!( "Invalid style string: {}. See the STYLES section of delta --help.", s - ); - process::exit(1); + )); } } if foreground_is_auto && background_is_auto { diff --git a/src/subcommands/show_config.rs b/src/subcommands/show_config.rs index 254c5e30..5442ae1f 100644 --- a/src/subcommands/show_config.rs +++ b/src/subcommands/show_config.rs @@ -100,7 +100,7 @@ pub fn show_config(config: &config::Config, writer: &mut dyn Write) -> std::io:: writer, " max-line-distance = {max_line_distance} max-line-length = {max_line_length} - line-fill-method = {line_fill_method} + line-fill-method = {line_fill_method} navigate = {navigate} navigate-regexp = {navigate_regexp} pager = {pager} -- cgit v1.2.3