Add custom env struct to store env at init (#1025)

This allow to no longer ignore some tests (marked previously as FIXME)
by storing the env at the start of the program (Or creating a custom env
for test purpose)
This centralize almost alls calls to std::env inside one wrapper

Add a test profile to increase speed for testing (5min -> 20sec on my
machine)

clean a few code style like this:
```
if Some(value) = ...
    if value
```
to
```
if Some(true) = ...
```

Co-authored-by: William Escande <[email protected]>

Cargo.toml

@@ -63,3 +63,6 @@ features = []
 version = "0.12.4"
 default-features = false
 features = []
+
+[profile.test]
+opt-level = 2

src/cli.rs

@@ -9,6 +9,7 @@ use syntect::highlighting::Theme as SyntaxTheme;
 use syntect::parsing::SyntaxSet;
 
 use crate::config::delta_unreachable;
+use crate::env::DeltaEnv;
 use crate::git_config::{GitConfig, GitConfigEntry};
 use crate::options;
 use crate::utils;
@@ -1072,6 +1073,9 @@ pub struct Opt {
 
     #[clap(skip)]
     pub git_config_entries: HashMap<String, GitConfigEntry>,
+
+    #[clap(skip)]
+    pub env: DeltaEnv,
 }
 
 #[derive(Default, Clone, Debug)]
@@ -1120,28 +1124,40 @@ impl Default for PagingMode {
 
 impl Opt {
     pub fn from_args_and_git_config(
+        env: DeltaEnv,
         git_config: Option<GitConfig>,
         assets: HighlightingAssets,
     ) -> Self {
-        Self::from_clap_and_git_config(Self::into_app().get_matches(), git_config, assets)
+        Self::from_clap_and_git_config(env, Self::into_app().get_matches(), git_config, assets)
     }
 
-    pub fn from_iter_and_git_config<I>(iter: I, git_config: Option<GitConfig>) -> Self
+    pub fn from_iter_and_git_config<I>(
+        env: DeltaEnv,
+        iter: I,
+        git_config: Option<GitConfig>,
+    ) -> Self
     where
         I: IntoIterator,
         I::Item: Into<OsString> + Clone,
     {
         let assets = utils::bat::assets::load_highlighting_assets();
-        Self::from_clap_and_git_config(Self::into_app().get_matches_from(iter), git_config, assets)
+        Self::from_clap_and_git_config(
+            env,
+            Self::into_app().get_matches_from(iter),
+            git_config,
+            assets,
+        )
     }
 
     fn from_clap_and_git_config(
+        env: DeltaEnv,
         arg_matches: clap::ArgMatches,
         mut git_config: Option<GitConfig>,
         assets: HighlightingAssets,
     ) -> Self {
         let mut opt = Opt::from_arg_matches(&arg_matches)
             .unwrap_or_else(|_| delta_unreachable("Opt::from_arg_matches failed"));
+        opt.env = env;
         options::set::set_options(&mut opt, &mut git_config, &arg_matches, assets);
         opt.git_config = git_config;
         opt

src/config.rs

@@ -10,7 +10,6 @@ use crate::ansi;
 use crate::cli;
 use crate::color;
 use crate::delta::State;
-use crate::env;
 use crate::fatal;
 use crate::features::navigate;
 use crate::features::side_by_side::{self, ansifill, LeftRight};
@@ -151,10 +150,12 @@ impl From<cli::Opt> for Config {
 
         let wrap_config = WrapConfig::from_opt(&opt, styles["inline-hint-style"]);
 
-        let max_line_distance_for_naively_paired_lines =
-            env::get_env_var("DELTA_EXPERIMENTAL_MAX_LINE_DISTANCE_FOR_NAIVELY_PAIRED_LINES")
-                .map(|s| s.parse::<f64>().unwrap_or(0.0))
-                .unwrap_or(0.0);
+        let max_line_distance_for_naively_paired_lines = opt
+            .env
+            .experimental_max_line_distance_for_naively_paired_lines
+            .as_ref()
+            .map(|s| s.parse::<f64>().unwrap_or(0.0))
+            .unwrap_or(0.0);
 
         let commit_regex = Regex::new(&opt.commit_regex).unwrap_or_else(|_| {
             fatal(format!(
@@ -217,11 +218,11 @@ impl From<cli::Opt> for Config {
         };
 
         #[cfg(not(test))]
-        let cwd_of_delta_process = std::env::current_dir().ok();
+        let cwd_of_delta_process = opt.env.current_dir;
         #[cfg(test)]
         let cwd_of_delta_process = Some(utils::path::fake_delta_cwd_for_tests());
 
-        let cwd_relative_to_repo_root = std::env::var("GIT_PREFIX").ok();
+        let cwd_relative_to_repo_root = opt.env.git_prefix;
 
         let cwd_of_user_shell_process = utils::path::cwd_of_user_shell_process(
             cwd_of_delta_process.as_ref(),

src/env.rs

@@ -1,24 +1,73 @@
-#[cfg(not(test))]
 use std::env;
 
-/// If `name` is set and, after trimming whitespace, is not empty string, then return that trimmed
-/// string. Else None.
-pub fn get_env_var(_name: &str) -> Option<String> {
-    #[cfg(not(test))]
-    match env::var(_name).unwrap_or_else(|_| "".to_string()).trim() {
-        "" => None,
-        non_empty_string => Some(non_empty_string.to_string()),
+const COLORTERM: &str = "COLORTERM";
+const BAT_THEME: &str = "BAT_THEME";
+const GIT_CONFIG_PARAMETERS: &str = "GIT_CONFIG_PARAMETERS";
+const GIT_PREFIX: &str = "GIT_PREFIX";
+const DELTA_FEATURES: &str = "DELTA_FEATURES";
+const DELTA_NAVIGATE: &str = "DELTA_NAVIGATE";
+const DELTA_EXPERIMENTAL_MAX_LINE_DISTANCE_FOR_NAIVELY_PAIRED_LINES: &str =
+    "DELTA_EXPERIMENTAL_MAX_LINE_DISTANCE_FOR_NAIVELY_PAIRED_LINES";
+const DELTA_PAGER: &str = "DELTA_PAGER";
+const BAT_PAGER: &str = "BAT_PAGER";
+const PAGER: &str = "PAGER";
+
+#[derive(Default, Clone)]
+pub struct DeltaEnv {
+    pub bat_theme: Option<String>,
+    pub colorterm: Option<String>,
+    pub current_dir: Option<std::path::PathBuf>,
+    pub experimental_max_line_distance_for_naively_paired_lines: Option<String>,
+    pub features: Option<String>,
+    pub git_config_parameters: Option<String>,
+    pub git_prefix: Option<String>,
+    pub navigate: Option<String>,
+    pub pagers: (Option<String>, Option<String>, Option<String>),
+}
+
+impl DeltaEnv {
+    /// Create a structure with current environment variable
+    pub fn init() -> Self {
+        let bat_theme = env::var(BAT_THEME).ok();
+        let colorterm = env::var(COLORTERM).ok();
+        let experimental_max_line_distance_for_naively_paired_lines =
+            env::var(DELTA_EXPERIMENTAL_MAX_LINE_DISTANCE_FOR_NAIVELY_PAIRED_LINES).ok();
+        let features = env::var(DELTA_FEATURES).ok();
+        let git_config_parameters = env::var(GIT_CONFIG_PARAMETERS).ok();
+        let git_prefix = env::var(GIT_PREFIX).ok();
+        let navigate = env::var(DELTA_NAVIGATE).ok();
+
+        let current_dir = env::current_dir().ok();
+        let pagers = (
+            env::var(DELTA_PAGER).ok(),
+            env::var(BAT_PAGER).ok(),
+            env::var(PAGER).ok(),
+        );
+
+        Self {
+            bat_theme,
+            colorterm,
+            current_dir,
+            experimental_max_line_distance_for_naively_paired_lines,
+            features,
+            git_config_parameters,
+            git_prefix,
+            navigate,
+            pagers,
+        }
     }
-    #[cfg(test)]
-    None
 }
 
-/// If `name` is set to any value at all (including "") then return true; else false.
-pub fn get_boolean_env_var(_name: &str) -> bool {
-    #[cfg(not(test))]
-    {
-        env::var(_name).ok().is_some()
+#[cfg(test)]
+pub mod tests {
+    use super::DeltaEnv;
+    use std::env;
+
+    #[test]
+    fn test_env_parsing() {
+        let feature = "Awesome Feature";
+        env::set_var("DELTA_FEATURES", feature);
+        let env = DeltaEnv::init();
+        assert_eq!(env.features, Some(feature.into()));
     }
-    #[cfg(test)]
-    false
 }

src/features/mod.rs

@@ -94,6 +94,7 @@ pub mod tests {
     use std::fs::remove_file;
 
     use crate::cli;
+    use crate::env::DeltaEnv;
     use crate::features::make_builtin_features;
     use crate::tests::integration_test_utils::make_options_from_args_and_git_config;
 
@@ -102,7 +103,7 @@ pub mod tests {
         let builtin_features = make_builtin_features();
         let mut args = vec!["delta".to_string()];
         args.extend(builtin_features.keys().map(|s| format!("--{}", s)));
-        let opt = cli::Opt::from_iter_and_git_config(args, None);
+        let opt = cli::Opt::from_iter_and_git_config(DeltaEnv::default(), args, None);
         let features: HashSet<&str> = opt
             .features
             .as_deref()

src/git_config/mod.rs

@@ -2,9 +2,9 @@ mod git_config_entry;
 
 pub use git_config_entry::{GitConfigEntry, GitRemoteRepo};
 
+use crate::env::DeltaEnv;
 use regex::Regex;
 use std::collections::HashMap;
-use std::env;
 #[cfg(test)]
 use std::path::Path;
 
@@ -37,11 +37,11 @@ impl Clone for GitConfig {
 
 impl GitConfig {
     #[cfg(not(test))]
-    pub fn try_create() -> Option<Self> {
+    pub fn try_create(env: &DeltaEnv) -> Option<Self> {
         use crate::fatal;
 
-        let repo = match std::env::current_dir() {
-            Ok(dir) => git2::Repository::discover(dir).ok(),
+        let repo = match &env.current_dir {
+            Some(dir) => git2::Repository::discover(dir).ok(),
             _ => None,
         };
         let config = match &repo {
@@ -55,7 +55,7 @@ impl GitConfig {
                 });
                 Some(Self {
                     config,
-                    config_from_env_var: parse_config_from_env_var(),
+                    config_from_env_var: parse_config_from_env_var(env),
                     repo,
                     enabled: true,
                 })
@@ -65,16 +65,16 @@ impl GitConfig {
     }
 
     #[cfg(test)]
-    pub fn try_create() -> Option<Self> {
+    pub fn try_create(_env: &DeltaEnv) -> Option<Self> {
         unreachable!("GitConfig::try_create() is not available when testing");
     }
 
     #[cfg(test)]
-    pub fn from_path(path: &Path, honor_env_var: bool) -> Self {
+    pub fn from_path(env: &DeltaEnv, path: &Path, honor_env_var: bool) -> Self {
         Self {
             config: git2::Config::open(path).unwrap(),
             config_from_env_var: if honor_env_var {
-                parse_config_from_env_var()
+                parse_config_from_env_var(env)
             } else {
                 HashMap::new()
             },
@@ -96,9 +96,9 @@ impl GitConfig {
     }
 }
 
-fn parse_config_from_env_var() -> HashMap<String, String> {
-    if let Ok(s) = env::var("GIT_CONFIG_PARAMETERS") {
-        parse_config_from_env_var_value(&s)
+fn parse_config_from_env_var(env: &DeltaEnv) -> HashMap<String, String> {
+    if let Some(s) = &env.git_config_parameters {
+        parse_config_from_env_var_value(s)
     } else {
         HashMap::new()
     }

src/main.rs

@@ -86,7 +86,12 @@ fn main() -> std::io::Result<()> {
 // arguments and without standard input; 2 is used to report a real problem.
 fn run_app() -> std::io::Result<i32> {
     let assets = utils::bat::assets::load_highlighting_assets();
-    let opt = cli::Opt::from_args_and_git_config(git_config::GitConfig::try_create(), assets);
+    let env = env::DeltaEnv::init();
+    let opt = cli::Opt::from_args_and_git_config(
+        env.clone(),
+        git_config::GitConfig::try_create(&env),
+        assets,
+    );
 
     let subcommand_result = if opt.list_languages {
         Some(list_languages())
@@ -128,7 +133,7 @@ fn run_app() -> std::io::Result<i32> {
     }
 
     let mut output_type =
-        OutputType::from_mode(config.paging_mode, config.pager.clone(), &config).unwrap();
+        OutputType::from_mode(&env, config.paging_mode, config.pager.clone(), &config).unwrap();
     let mut writer = output_type.handle().unwrap();
 
     match (config.minus_file.as_ref(), config.plus_file.as_ref()) {