DELTA_FEATURES appends iff prefixed with +

Thanks @bew for the suggestion. Fixes #848
dandavison · Dec 12, 2021 · fab41c3 · fab41c3
1 parent e524aba
commit fab41c3
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 26 deletions.
diff --git a/src/cli.rs b/src/cli.rs
@@ -320,10 +320,10 @@ pub struct Opt {
     pub color_only: bool,
 
     ////////////////////////////////////////////////////////////////////////////////////////////
-    #[structopt(long = "features", default_value = "", env = "DELTA_FEATURES")]
+    #[structopt(long = "features")]
     /// Name of delta features to use (space-separated). A feature is a named collection of delta
     /// options in ~/.gitconfig. See FEATURES section.
-    pub features: String,
+    pub features: Option<String>,
 
     #[structopt(long = "syntax-theme", env = "BAT_THEME")]
     /// The code syntax-highlighting theme to use. Use --show-syntax-themes to demo available

diff --git a/src/features/mod.rs b/src/features/mod.rs
@@ -103,7 +103,12 @@ pub mod tests {
         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 features: HashSet<&str> = opt.features.split_whitespace().collect();
+        let features: HashSet<&str> = opt
+            .features
+            .as_deref()
+            .unwrap_or("")
+            .split_whitespace()
+            .collect();
         for feature in builtin_features.keys() {
             assert!(features.contains(feature.as_str()))
         }
@@ -123,7 +128,8 @@ pub mod tests {
                 Some(git_config_contents),
                 Some(git_config_path)
             )
-            .features,
+            .features
+            .unwrap(),
             "navigate"
         );
 
@@ -145,7 +151,8 @@ pub mod tests {
                 Some(git_config_contents),
                 Some(git_config_path),
             )
-            .features,
+            .features
+            .unwrap(),
             "navigate raw"
         );
         assert_eq!(
@@ -154,7 +161,8 @@ pub mod tests {
                 Some(git_config_contents),
                 Some(git_config_path),
             )
-            .features,
+            .features
+            .unwrap(),
             "navigate raw"
         );
 
@@ -175,7 +183,8 @@ pub mod tests {
                 Some(git_config_contents),
                 Some(git_config_path),
             )
-            .features,
+            .features
+            .unwrap(),
             "my-feature navigate raw"
         );
 
@@ -204,7 +213,8 @@ pub mod tests {
                 Some(git_config_contents),
                 Some(git_config_path),
             )
-            .features,
+            .features
+            .unwrap(),
             "raw diff-so-fancy f e d diff-highlight c b a"
         );
 
@@ -232,7 +242,10 @@ pub mod tests {
             Some(git_config_contents),
             Some(git_config_path),
         );
-        assert_eq!(opt.features, "feature-4 feature-2 feature-3 feature-1");
+        assert_eq!(
+            opt.features.unwrap(),
+            "feature-4 feature-2 feature-3 feature-1"
+        );
 
         remove_file(git_config_path).unwrap();
     }

diff --git a/src/options/get.rs b/src/options/get.rs
@@ -79,18 +79,20 @@ pub trait GetOptionValue {
                 return Some(value);
             }
         }
-        for feature in opt.features.split_whitespace().rev() {
-            match Self::get_provenanced_value_for_feature(
-                option_name,
-                feature,
-                builtin_features,
-                opt,
-                git_config,
-            ) {
-                Some(GitConfigValue(value)) | Some(DefaultValue(value)) => {
-                    return Some(value.into());
+        if let Some(features) = &opt.features {
+            for feature in features.split_whitespace().rev() {
+                match Self::get_provenanced_value_for_feature(
+                    option_name,
+                    feature,
+                    builtin_features,
+                    opt,
+                    git_config,
+                ) {
+                    Some(GitConfigValue(value)) | Some(DefaultValue(value)) => {
+                        return Some(value.into());
+                    }
+                    None => {}
                 }
-                None => {}
             }
         }
         None
@@ -300,6 +302,44 @@ pub mod tests {
         remove_file(git_config_path).unwrap();
     }
 
+    #[test]
+    #[ignore] // FIXME
+    fn test_delta_features_env_var() {
+        let git_config_contents = b"
+[delta]
+    features = feature-from-gitconfig
+";
+        let git_config_path = "delta__test_delta_features_env_var.gitconfig";
+
+        let opt = integration_test_utils::make_options_from_args_and_git_config(
+            &[],
+            Some(git_config_contents),
+            Some(git_config_path),
+        );
+        assert_eq!(opt.features.unwrap(), "feature-from-gitconfig");
+        assert_eq!(opt.side_by_side, false);
+
+        env::set_var("DELTA_FEATURES", "side-by-side");
+        let opt = integration_test_utils::make_options_from_args_and_git_config(
+            &[],
+            Some(git_config_contents),
+            Some(git_config_path),
+        );
+        assert_eq!(opt.features.unwrap(), "side-by-side");
+        assert_eq!(opt.side_by_side, true);
+
+        env::set_var("DELTA_FEATURES", "+side-by-side");
+        let opt = integration_test_utils::make_options_from_args_and_git_config(
+            &[],
+            Some(git_config_contents),
+            Some(git_config_path),
+        );
+        assert_eq!(opt.features.unwrap(), "side-by-side feature-from-gitconfig");
+        assert_eq!(opt.side_by_side, true);
+
+        remove_file(git_config_path).unwrap();
+    }
+
     #[test]
     fn test_get_themes_from_config() {
         let git_config_contents = r#"

diff --git a/src/options/set.rs b/src/options/set.rs
@@ -89,7 +89,7 @@ pub fn set_options(
     }
 
     let features = gather_features(opt, &builtin_features, git_config);
-    opt.features = features.join(" ");
+    opt.features = Some(features.join(" "));
 
     // Set light, dark, and syntax-theme.
     set__light__dark__syntax_theme__options(opt, git_config, arg_matches, &option_names);
@@ -324,19 +324,33 @@ fn set__light__dark__syntax_theme__options(
 // [delta "d"]
 //     features = f e
 fn gather_features(
-    opt: &cli::Opt,
+    opt: &mut cli::Opt,
     builtin_features: &HashMap<String, features::BuiltinFeature>,
     git_config: &Option<GitConfig>,
 ) -> Vec<String> {
+    let from_env_var = env::get_env_var("DELTA_FEATURES");
+    let from_args = opt.features.as_deref().unwrap_or("");
+    let input_features: Vec<&str> = match from_env_var.as_deref() {
+        Some(from_env_var) if from_env_var.starts_with('+') => from_env_var[1..]
+            .split_whitespace()
+            .chain(split_feature_string(from_args))
+            .collect(),
+        Some(from_env_var) => {
+            opt.features = Some(from_env_var.to_string());
+            split_feature_string(from_env_var).collect()
+        }
+        None => split_feature_string(from_args).collect(),
+    };
+
     let mut features = VecDeque::new();
 
     // Gather features from command line.
     if let Some(git_config) = git_config {
-        for feature in split_feature_string(&opt.features) {
+        for feature in input_features {
             gather_features_recursively(feature, &mut features, builtin_features, opt, git_config);
         }
     } else {
-        for feature in split_feature_string(&opt.features) {
+        for feature in input_features {
             features.push_front(feature.to_string());
         }
     }
@@ -370,7 +384,7 @@ fn gather_features(
 
     if let Some(git_config) = git_config {
         // Gather features from [delta] section if --features was not passed.
-        if opt.features.is_empty() {
+        if opt.features.is_none() {
             if let Some(feature_string) = git_config.get::<String>("delta.features") {
                 for feature in split_feature_string(&feature_string) {
                     gather_features_recursively(
@@ -739,7 +753,11 @@ pub mod tests {
         // TODO: should set_options not be called on any feature flags?
         // assert_eq!(opt.diff_highlight, true);
         // assert_eq!(opt.diff_so_fancy, true);
-        assert!(opt.features.split_whitespace().any(|s| s == "xxxyyyzzz"));
+        assert!(opt
+            .features
+            .unwrap()
+            .split_whitespace()
+            .any(|s| s == "xxxyyyzzz"));
         assert_eq!(opt.file_added_label, "xxxyyyzzz");
         assert_eq!(opt.file_decoration_style, "black black");
         assert_eq!(opt.file_modified_label, "xxxyyyzzz");