diff options
author | Dan Davison <dandavison7@gmail.com> | 2020-06-22 11:07:09 -0400 |
---|---|---|
committer | Dan Davison <dandavison7@gmail.com> | 2020-06-22 12:05:37 -0400 |
commit | 83e020d43e143852aebe1c64e15edd2ba4b13756 (patch) | |
tree | 57d523665c16077c6ad6839f561d2b66d79c6a7c | |
parent | f635d87938a0b3d74e90e8e0e9aadb65e7d3bb93 (diff) |
Refactor: separate Opt construction from Config construction
-rw-r--r-- | src/cli.rs | 33 | ||||
-rw-r--r-- | src/config.rs | 119 | ||||
-rw-r--r-- | src/features/diff_highlight.rs | 57 | ||||
-rw-r--r-- | src/features/diff_so_fancy.rs | 88 | ||||
-rw-r--r-- | src/features/mod.rs | 106 | ||||
-rw-r--r-- | src/features/navigate.rs | 18 | ||||
-rw-r--r-- | src/main.rs | 27 | ||||
-rw-r--r-- | src/tests/integration_test_utils.rs | 17 |
8 files changed, 195 insertions, 270 deletions
@@ -1,7 +1,13 @@ +#[cfg(test)] +use std::ffi::OsString; use std::path::PathBuf; use structopt::clap::AppSettings::{ColorAlways, ColoredHelp, DeriveDisplayOrder}; -use structopt::StructOpt; +use structopt::{clap, StructOpt}; + +use crate::git_config::GitConfig; +use crate::rewrite_options; +use crate::set_options; #[derive(StructOpt, Clone, Debug, PartialEq)] #[structopt( @@ -473,3 +479,28 @@ pub struct Opt { /// Deprecated: use --syntax-theme. pub deprecated_theme: Option<String>, } + +impl Opt { + pub fn from_args_and_git_config(git_config: &mut Option<GitConfig>) -> Self { + Self::from_clap_and_git_config(Self::clap().get_matches(), git_config) + } + + #[cfg(test)] + pub fn from_iter_and_git_config<I>(iter: I, git_config: &mut Option<GitConfig>) -> Self + where + I: IntoIterator, + I::Item: Into<OsString> + Clone, + { + Self::from_clap_and_git_config(Self::clap().get_matches_from(iter), git_config) + } + + fn from_clap_and_git_config( + arg_matches: clap::ArgMatches, + git_config: &mut Option<GitConfig>, + ) -> Self { + let mut opt = Opt::from_clap(&arg_matches); + set_options::set_options(&mut opt, git_config, &arg_matches); + rewrite_options::apply_rewrite_rules(&mut opt, &arg_matches); + opt + } +} diff --git a/src/config.rs b/src/config.rs index e05d7b8c..3ee7c85a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,7 +4,7 @@ use std::process; use console::Term; use regex::Regex; -use structopt::{clap, StructOpt}; +use structopt::clap; use syntect::highlighting::Style as SyntectStyle; use syntect::highlighting::Theme as SyntaxTheme; use syntect::parsing::SyntaxSet; @@ -15,9 +15,6 @@ use crate::cli; use crate::color; use crate::delta::State; use crate::env; -use crate::git_config::GitConfig; -use crate::rewrite_options; -use crate::set_options; use crate::style::Style; use crate::syntax_theme; @@ -77,20 +74,6 @@ pub struct Config { } impl Config { - pub fn from_args(args: &[&str], git_config: &mut Option<GitConfig>) -> Self { - Self::from_arg_matches(cli::Opt::clap().get_matches_from(args), git_config) - } - - pub fn from_arg_matches( - arg_matches: clap::ArgMatches, - git_config: &mut Option<GitConfig>, - ) -> Self { - let mut opt = cli::Opt::from_clap(&arg_matches); - set_options::set_options(&mut opt, git_config, &arg_matches); - rewrite_options::apply_rewrite_rules(&mut opt, &arg_matches); - Self::from(opt) - } - pub fn get_style(&self, state: &State) -> &Style { match state { State::CommitMeta => &self.commit_style, @@ -101,56 +84,6 @@ impl Config { } } -fn _check_validity(opt: &cli::Opt, assets: &HighlightingAssets) { - if opt.light && opt.dark { - eprintln!("--light and --dark cannot be used together."); - process::exit(1); - } - if let Some(ref syntax_theme) = opt.syntax_theme { - if !syntax_theme::is_no_syntax_highlighting_theme_name(&syntax_theme) { - if !assets.theme_set.themes.contains_key(syntax_theme.as_str()) { - return; - } - let is_light_syntax_theme = syntax_theme::is_light_theme(&syntax_theme); - if is_light_syntax_theme && opt.dark { - eprintln!( - "{} is a light syntax theme, but you supplied --dark. \ - If you use --syntax-theme, you do not need to supply --light or --dark.", - syntax_theme - ); - process::exit(1); - } else if !is_light_syntax_theme && opt.light { - eprintln!( - "{} is a dark syntax theme, but you supplied --light. \ - If you use --syntax-theme, you do not need to supply --light or --dark.", - syntax_theme - ); - process::exit(1); - } - } - } -} - -/// Did the user supply `option` on the command line? -pub fn user_supplied_option(option: &str, arg_matches: &clap::ArgMatches) -> bool { - arg_matches.occurrences_of(option) > 0 -} - -pub fn unreachable(message: &str) -> ! { - eprintln!( - "{} This should not be possible. \ - Please report the bug at https://github.com/dandavison/delta/issues.", - message - ); - process::exit(1); -} - -fn is_truecolor_terminal() -> bool { - env::get_env_var("COLORTERM") - .map(|colorterm| colorterm == "truecolor" || colorterm == "24bit") - .unwrap_or(false) -} - impl From<cli::Opt> for Config { fn from(opt: cli::Opt) -> Self { let assets = HighlightingAssets::new(); @@ -539,6 +472,56 @@ pub fn make_navigate_regexp(config: &Config) -> String { ) } +fn _check_validity(opt: &cli::Opt, assets: &HighlightingAssets) { + if opt.light && opt.dark { + eprintln!("--light and --dark cannot be used together."); + process::exit(1); + } + if let Some(ref syntax_theme) = opt.syntax_theme { + if !syntax_theme::is_no_syntax_highlighting_theme_name(&syntax_theme) { + if !assets.theme_set.themes.contains_key(syntax_theme.as_str()) { + return; + } + let is_light_syntax_theme = syntax_theme::is_light_theme(&syntax_theme); + if is_light_syntax_theme && opt.dark { + eprintln!( + "{} is a light syntax theme, but you supplied --dark. \ + If you use --syntax-theme, you do not need to supply --light or --dark.", + syntax_theme + ); + process::exit(1); + } else if !is_light_syntax_theme && opt.light { + eprintln!( + "{} is a dark syntax theme, but you supplied --light. \ + If you use --syntax-theme, you do not need to supply --light or --dark.", + syntax_theme + ); + process::exit(1); + } + } + } +} + +/// Did the user supply `option` on the command line? +pub fn user_supplied_option(option: &str, arg_matches: &clap::ArgMatches) -> bool { + arg_matches.occurrences_of(option) > 0 +} + +pub fn unreachable(message: &str) -> ! { + eprintln!( + "{} This should not be possible. \ + Please report the bug at https://github.com/dandavison/delta/issues.", + message + ); + process::exit(1); +} + +fn is_truecolor_terminal() -> bool { + env::get_env_var("COLORTERM") + .map(|colorterm| colorterm == "truecolor" || colorterm == "24bit") + .unwrap_or(false) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/features/diff_highlight.rs b/src/features/diff_highlight.rs index 35b71c4b..fa4cba1e 100644 --- a/src/features/diff_highlight.rs +++ b/src/features/diff_highlight.rs @@ -59,27 +59,14 @@ mod test_utils { #[test] fn test_diff_highlight_defaults() { - let config = features::tests::make_config(&["--features", "diff-highlight"], None, None); - - assert_eq!(config.minus_style, features::tests::make_style("red")); - assert_eq!( - config.minus_non_emph_style, - features::tests::make_style("red") - ); - assert_eq!( - config.minus_emph_style, - features::tests::make_emph_style("red reverse") - ); - assert_eq!(config.zero_style, features::tests::make_style("")); - assert_eq!(config.plus_style, features::tests::make_style("green")); - assert_eq!( - config.plus_non_emph_style, - features::tests::make_style("green") - ); - assert_eq!( - config.plus_emph_style, - features::tests::make_emph_style("green reverse") - ); + let opt = features::tests::make_options(&["--features", "diff-highlight"], None, None); + assert_eq!(opt.minus_style, "red"); + assert_eq!(opt.minus_non_emph_style, "red"); + assert_eq!(opt.minus_emph_style, "red reverse"); + assert_eq!(opt.zero_style, "normal"); + assert_eq!(opt.plus_style, "green"); + assert_eq!(opt.plus_non_emph_style, "green"); + assert_eq!(opt.plus_emph_style, "green reverse"); } #[test] @@ -97,31 +84,19 @@ mod test_utils { "; let git_config_path = "delta__test_diff_highlight.gitconfig"; - let config = features::tests::make_config( + let opt = features::tests::make_options( &["--features", "diff-highlight"], Some(git_config_contents), Some(git_config_path), ); - assert_eq!(config.minus_style, features::tests::make_style("red bold")); - assert_eq!( - config.minus_non_emph_style, - features::tests::make_style("ul red bold") - ); - assert_eq!( - config.minus_emph_style, - features::tests::make_emph_style("red bold 52") - ); - assert_eq!(config.zero_style, features::tests::make_style("")); - assert_eq!(config.plus_style, features::tests::make_style("green bold")); - assert_eq!( - config.plus_non_emph_style, - features::tests::make_style("ul green bold") - ); - assert_eq!( - config.plus_emph_style, - features::tests::make_emph_style("green bold 22") - ); + assert_eq!(opt.minus_style, "red bold"); + assert_eq!(opt.minus_non_emph_style, "ul red bold"); + assert_eq!(opt.minus_emph_style, "red bold 52"); + assert_eq!(opt.zero_style, "normal"); + assert_eq!(opt.plus_style, "green bold"); + assert_eq!(opt.plus_non_emph_style, "ul green bold"); + assert_eq!(opt.plus_emph_style, "green bold 22"); remove_file(git_config_path).unwrap(); } diff --git a/src/features/diff_so_fancy.rs b/src/features/diff_so_fancy.rs index 8b7d0299..bba2b07c 100644 --- a/src/features/diff_so_fancy.rs +++ b/src/features/diff_so_fancy.rs @@ -64,34 +64,16 @@ pub mod tests { #[test] fn test_diff_so_fancy_defaults() { - let config = features::tests::make_config(&["--features", "diff-so-fancy"], None, None); + let opt = features::tests::make_options(&["--features", "diff-so-fancy"], None, None); - assert_eq!( - config.commit_style.ansi_term_style, - features::tests::make_style("bold yellow").ansi_term_style - ); - assert_eq!( - config.commit_style.decoration_style, - features::tests::make_decoration_style("none") - ); + assert_eq!(opt.commit_style, "bold yellow"); + assert_eq!(opt.commit_decoration_style, "none"); - assert_eq!( - config.file_style.ansi_term_style, - features::tests::make_style("11").ansi_term_style - ); - assert_eq!( - config.file_style.decoration_style, - features::tests::make_decoration_style("bold yellow ul ol") - ); + assert_eq!(opt.file_style, "11"); + assert_eq!(opt.file_decoration_style, "bold yellow ul ol"); - assert_eq!( - config.hunk_header_style.ansi_term_style, - features::tests::make_style("bold syntax").ansi_term_style - ); - assert_eq!( - config.hunk_header_style.decoration_style, - features::tests::make_decoration_style("magenta box") - ); + assert_eq!(opt.hunk_header_style, "bold syntax"); + assert_eq!(opt.hunk_header_decoration_style, "magenta box"); } #[test] @@ -107,36 +89,18 @@ pub mod tests { "; let git_config_path = "delta__test_diff_so_fancy.gitconfig"; - let config = features::tests::make_config( + let opt = features::tests::make_options( &["--features", "diff-so-fancy some-other-feature"], Some(git_config_contents), Some(git_config_path), ); - assert_eq!( - config.commit_style.ansi_term_style, - features::tests::make_style("yellow bold").ansi_term_style - ); - assert_eq!( - config.file_style.ansi_term_style, - features::tests::make_style("11").ansi_term_style - ); - assert_eq!( - config.hunk_header_style.ansi_term_style, - features::tests::make_style("magenta bold").ansi_term_style - ); - assert_eq!( - config.commit_style.decoration_style, - features::tests::make_decoration_style("none") - ); - assert_eq!( - config.file_style.decoration_style, - features::tests::make_decoration_style("yellow bold ul ol") - ); - assert_eq!( - config.hunk_header_style.decoration_style, - features::tests::make_decoration_style("magenta box") - ); + assert_eq!(opt.commit_style, "bold yellow"); + assert_eq!(opt.file_style, "11"); + assert_eq!(opt.hunk_header_style, "magenta bold"); + assert_eq!(opt.commit_decoration_style, "none"); + assert_eq!(opt.file_decoration_style, "bold yellow ul ol"); + assert_eq!(opt.hunk_header_decoration_style, "magenta box"); remove_file(git_config_path).unwrap(); } @@ -159,37 +123,25 @@ pub mod tests { "; let git_config_path = "delta__test_diff_so_fancy_obeys_feature_precedence_rules.gitconfig"; - let config = features::tests::make_config( + let opt = features::tests::make_options( &["--features", "decorations diff-so-fancy"], Some(git_config_contents), Some(git_config_path), ); - assert_eq!( - config.file_style.ansi_term_style, - features::tests::make_style("11").ansi_term_style - ); + assert_eq!(opt.file_style, "11"); - assert_eq!( - config.file_style.decoration_style, - features::tests::make_decoration_style("yellow bold ul ol") - ); + assert_eq!(opt.file_decoration_style, "bold yellow ul ol"); - let config = features::tests::make_config( + let opt = features::tests::make_options( &["--features", "diff-so-fancy decorations"], Some(git_config_contents), Some(git_config_path), ); - assert_eq!( - config.file_style.ansi_term_style, - features::tests::make_style("ul bold 19").ansi_term_style - ); + assert_eq!(opt.file_style, "bold 19 ul"); - assert_eq!( - config.file_style.decoration_style, - features::tests::make_decoration_style("none") - ); + assert_eq!(opt.file_decoration_style, "none"); remove_file(git_config_path).unwrap(); } diff --git a/src/features/mod.rs b/src/features/mod.rs index 6a80539f..e907d037 100644 --- a/src/features/mod.rs +++ b/src/features/mod.rs @@ -85,11 +85,9 @@ pub mod tests { use structopt::{clap, StructOpt}; use crate::cli; - use crate::config; use crate::features::make_builtin_features; use crate::git_config::GitConfig; use crate::set_options::set_options; - use crate::style::{DecorationStyle, Style}; #[test] fn test_builtin_features_have_flags() { @@ -134,29 +132,29 @@ pub mod tests { // First check that it doesn't default to blue, because that's going to be used to signal // that gitconfig has set the style. - assert_ne!(make_config(&[], None, None).minus_style, make_style("blue")); + assert_ne!(make_options(&[], None, None).minus_style, "blue"); // Check that --minus-style is honored as we expect. assert_eq!( - make_config(&["--minus-style", "red"], None, None).minus_style, - make_style("red") + make_options(&["--minus-style", "red"], None, None).minus_style, + "red" ); // Check that gitconfig does not override a command line argument assert_eq!( - make_config( + make_options( &["--minus-style", "red"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("red") + "red" ); // Finally, check that gitconfig is honored when not overridden by a command line argument. assert_eq!( - make_config(&[], Some(git_config_contents), Some(git_config_path)).minus_style, - make_style("blue") + make_options(&[], Some(git_config_contents), Some(git_config_path)).minus_style, + "blue" ); remove_file(git_config_path).unwrap(); @@ -174,13 +172,13 @@ pub mod tests { let git_config_path = "delta__test_feature.gitconfig"; assert_eq!( - make_config( + make_options( &["--features", "my-feature"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("green") + "green" ); remove_file(git_config_path).unwrap(); } @@ -198,19 +196,19 @@ pub mod tests { // Without --features the main section takes effect assert_eq!( - make_config(&[], Some(git_config_contents), Some(git_config_path)).minus_style, - make_style("blue") + make_options(&[], Some(git_config_contents), Some(git_config_path)).minus_style, + "blue" ); // Event with --features the main section overrides the feature. assert_eq!( - make_config( + make_options( &["--features", "my-feature-1"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("blue") + "blue" ); remove_file(git_config_path).unwrap(); } @@ -230,23 +228,23 @@ pub mod tests { let git_config_path = "delta__test_multiple_features.gitconfig"; assert_eq!( - make_config( + make_options( &["--features", "my-feature-1 my-feature-2"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("yellow") + "yellow" ); assert_eq!( - make_config( + make_options( &["--features", "my-feature-2 my-feature-1"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("green") + "green" ); remove_file(git_config_path).unwrap(); @@ -263,22 +261,22 @@ pub mod tests { "; let git_config_path = "delta__test_invalid_features.gitconfig"; - let default = make_config(&[], None, None).minus_style; - assert_ne!(default, make_style("green")); - assert_ne!(default, make_style("yellow")); + let default = make_options(&[], None, None).minus_style; + assert_ne!(default, "green"); + assert_ne!(default, "yellow"); assert_eq!( - make_config( + make_options( &["--features", "my-feature-1"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("green") + "green" ); assert_eq!( - make_config( + make_options( &["--features", "my-feature-x"], Some(git_config_contents), Some(git_config_path), @@ -288,23 +286,23 @@ pub mod tests { ); assert_eq!( - make_config( + make_options( &["--features", "my-feature-1 my-feature-x"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("green") + "green" ); assert_eq!( - make_config( + make_options( &["--features", "my-feature-x my-feature-2 my-feature-x"], Some(git_config_contents), Some(git_config_path), ) .minus_style, - make_style("yellow") + "yellow" ); remove_file(git_config_path).unwrap(); @@ -320,26 +318,26 @@ pub mod tests { // Git config disabled: hard-coded delta default wins assert_eq!( - make_config(&[], None, None).whitespace_error_style, - make_style("magenta reverse") + make_options(&[], None, None).whitespace_error_style, + "magenta reverse" ); // Unspecified by user: color.diff.whitespace wins assert_eq!( - make_config(&[], Some(git_config_contents), Some(git_config_path)) + make_options(&[], Some(git_config_contents), Some(git_config_path)) .whitespace_error_style, - make_style("yellow dim ul magenta") + "yellow dim ul magenta" ); // Command line argument wins assert_eq!( - make_config( + make_options( &["--whitespace-error-style", "red reverse"], Some(git_config_contents), Some(git_config_path) ) .whitespace_error_style, - make_style("reverse red") + "red reverse" ); let git_config_contents = b" @@ -355,20 +353,20 @@ pub mod tests { // Command line argument wins assert_eq!( - make_config( + make_options( &["--whitespace-error-style", "red reverse"], Some(git_config_contents), Some(git_config_path) ) .whitespace_error_style, - make_style("reverse red") + "red reverse" ); // No command line argument or features; main [delta] section wins assert_eq!( - make_config(&[], Some(git_config_contents), Some(git_config_path)) + make_options(&[], Some(git_config_contents), Some(git_config_path)) .whitespace_error_style, - make_style("blue reverse") + "blue reverse" ); // Feature contains key, but main [delta] section still wins. @@ -380,34 +378,18 @@ pub mod tests { // // In this situation, the value from the feature is overridden. assert_eq!( - make_config( + make_options( &["--features", "my-whitespace-error-style-feature"], Some(git_config_contents), Some(git_config_path) ) .whitespace_error_style, - make_style("reverse blue") + "blue reverse" ); remove_file(git_config_path).unwrap(); } - pub fn make_style(s: &str) -> Style { - _make_style(s, false) - } - - pub fn make_emph_style(s: &str) -> Style { - _make_style(s, true) - } - - fn _make_style(s: &str, is_emph: bool) -> Style { - Style::from_str(s, None, None, None, true, is_emph) - } - - pub fn make_decoration_style(s: &str) -> DecorationStyle { - DecorationStyle::from_str(s, true) - } - fn make_git_config(contents: &[u8], path: &str) -> GitConfig { let path = Path::new(path); let mut file = File::create(path).unwrap(); @@ -415,21 +397,21 @@ pub mod tests { GitConfig::from_path(&path) } - pub fn make_config( + pub fn make_options( args: &[&str], git_config_contents: Option<&[u8]>, - path: Option<&str>, - ) -> config::Config { + git_config_path: Option<&str>, + ) -> cli::Opt { let args: Vec<&str> = itertools::chain( &["/dev/null", "/dev/null", "--24-bit-color", "always"], args, ) .map(|s| *s) .collect(); - let mut git_config = match (git_config_contents, path) { + let mut git_config = match (git_config_contents, git_config_path) { (Some(contents), Some(path)) => Some(make_git_config(contents, path)), _ => None, }; - config::Config::from_args(&args, &mut git_config) + cli::Opt::from_iter_and_git_config(args, &mut git_config) } } diff --git a/src/features/navigate.rs b/src/features/navigate.rs index 4c0a71d8..001c8bc5 100644 --- a/src/features/navigate.rs +++ b/src/features/navigate.rs @@ -32,18 +32,18 @@ mod tests { "; let git_config_path = "delta__test_navigate_with_overriden_key_in_main_section.gitconfig"; - assert_eq!(features::tests::make_config(&[], None, None).file_modified_label, ""); + assert_eq!(features::tests::make_options(&[], None, None).file_modified_label, ""); assert_eq!( - features::tests::make_config(&["--features", "navigate"], None, None) + features::tests::make_options(&["--features", "navigate"], None, None) .file_modified_label, "Δ" ); assert_eq!( - features::tests::make_config(&["--navigate"], None, None).file_modified_label, + features::tests::make_options(&["--navigate"], None, None).file_modified_label, "Δ" ); assert_eq!( - features::tests::make_config(&[], Some(git_config_contents), Some(git_config_path)) + features::tests::make_options(&[], Some(git_config_contents), Some(git_config_path)) .file_modified_label, "modified: " ); @@ -64,16 +64,16 @@ mod tests { "delta__test_navigate_with_overriden_key_in_custom_navigate_section.gitconfig"; assert_eq!( - features::tests::make_config(&[], None, None).file_modified_label, + features::tests::make_options(&[], None, None).file_modified_label, "" ); assert_eq!( - features::tests::make_config(&["--features", "navigate"], None, None) + features::tests::make_options(&["--features", "navigate"], None, None) .file_modified_label, "Δ" ); assert_eq!( - features::tests::make_config(&[], Some(git_config_contents), Some(git_config_path)) + features::tests::make_options(&[], Some(git_config_contents), Some(git_config_path)) .file_modified_label, "modified: " ); @@ -91,12 +91,12 @@ mod tests { let git_config_path = "delta__test_navigate_activated_by_custom_feature.gitconfig"; assert_eq!( - features::tests::make_config(&[], Some(git_config_contents), Some(git_config_path)) + features::tests::make_options(&[], Some(git_config_contents), Some(git_config_path)) .file_modified_label, "" ); assert_eq!( - features::tests::make_config( + features::tests::make_options( &["--features", "my-navigate-feature"], Some(git_config_contents), Some(git_config_path) diff --git a/src/main.rs b/src/main.rs index 9d152c41..33010852 100644 --- a/src/main.rs +++ b/src/main.rs @@ -49,10 +49,8 @@ mod errors { } fn main() -> std::io::Result<()> { - let config = config::Config::from_arg_matches( - cli::Opt::clap().get_matches(), - &mut git_config::GitConfig::try_create(), - ); + let opt = cli::Opt::from_args_and_git_config(&mut git_config::GitConfig::try_create()); + let config = config::Config::from(opt); if config.list_languages { |