From 35d2693630a4ec29a654704bc4be47badb8d6070 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= <blaz@mxxn.io>
Date: Wed, 20 Apr 2022 17:09:03 +0900
Subject: [PATCH] Revert "override nested arrays when merging TOML (#2145)"

Looks like there's some follow-up issues

This reverts commit c8cfd0b1a0da071618a9efc71ac5735d6147a0ca.
---
 helix-loader/src/config.rs |   2 +-
 helix-loader/src/lib.rs    | 101 +++++++++----------------------------
 2 files changed, 24 insertions(+), 79 deletions(-)

diff --git a/helix-loader/src/config.rs b/helix-loader/src/config.rs
index 242af197..5dc2d6b6 100644
--- a/helix-loader/src/config.rs
+++ b/helix-loader/src/config.rs
@@ -19,7 +19,7 @@ pub fn user_lang_config() -> Result<toml::Value, toml::de::Error> {
         .into_iter()
         .chain([default_lang_config()].into_iter())
         .fold(toml::Value::Table(toml::value::Table::default()), |a, b| {
-            crate::merge_toml_values(b, a, false)
+            crate::merge_toml_values(b, a)
         });
 
     Ok(config)
diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs
index 767bff7a..de2951f8 100644
--- a/helix-loader/src/lib.rs
+++ b/helix-loader/src/lib.rs
@@ -98,24 +98,8 @@ pub fn find_root_impl(root: Option<&str>, root_markers: &[String]) -> Vec<std::p
     directories
 }
 
-/// Merge two TOML documents, merging values from `right` onto `left`
-///
-/// When an array exists in both `left` and `right`, `right`'s array is
-/// used. When a table exists in both `left` and `right`, the merged table
-/// consists of all keys in `left`'s table unioned with all keys in `right`
-/// with the values of `right` being merged recursively onto values of
-/// `left`.
-///
-/// `merge_toplevel_arrays` controls whether a top-level array in the TOML
-/// document is merged instead of overridden. This is useful for TOML
-/// documents that use a top-level array of values like the `languages.toml`,
-/// where one usually wants to override or add to the array instead of
-/// replacing it altogether.
-pub fn merge_toml_values(
-    left: toml::Value,
-    right: toml::Value,
-    merge_toplevel_arrays: bool,
-) -> toml::Value {
+// right overrides left
+pub fn merge_toml_values(left: toml::Value, right: toml::Value) -> toml::Value {
     use toml::Value;
 
     fn get_name(v: &Value) -> Option<&str> {
@@ -124,35 +108,24 @@ pub fn merge_toml_values(
 
     match (left, right) {
         (Value::Array(mut left_items), Value::Array(right_items)) => {
-            // The top-level arrays should be merged but nested arrays should
-            // act as overrides. For the `languages.toml` config, this means
-            // that you can specify a sub-set of languages in an overriding
-            // `languages.toml` but that nested arrays like Language Server
-            // arguments are replaced instead of merged.
-            if merge_toplevel_arrays {
-                left_items.reserve(right_items.len());
-                for rvalue in right_items {
-                    let lvalue = get_name(&rvalue)
-                        .and_then(|rname| {
-                            left_items.iter().position(|v| get_name(v) == Some(rname))
-                        })
-                        .map(|lpos| left_items.remove(lpos));
-                    let mvalue = match lvalue {
-                        Some(lvalue) => merge_toml_values(lvalue, rvalue, false),
-                        None => rvalue,
-                    };
-                    left_items.push(mvalue);
-                }
-                Value::Array(left_items)
-            } else {
-                Value::Array(right_items)
+            left_items.reserve(right_items.len());
+            for rvalue in right_items {
+                let lvalue = get_name(&rvalue)
+                    .and_then(|rname| left_items.iter().position(|v| get_name(v) == Some(rname)))
+                    .map(|lpos| left_items.remove(lpos));
+                let mvalue = match lvalue {
+                    Some(lvalue) => merge_toml_values(lvalue, rvalue),
+                    None => rvalue,
+                };
+                left_items.push(mvalue);
             }
+            Value::Array(left_items)
         }
         (Value::Table(mut left_map), Value::Table(right_map)) => {
             for (rname, rvalue) in right_map {
                 match left_map.remove(&rname) {
                     Some(lvalue) => {
-                        let merged_value = merge_toml_values(lvalue, rvalue, merge_toplevel_arrays);
+                        let merged_value = merge_toml_values(lvalue, rvalue);
                         left_map.insert(rname, merged_value);
                     }
                     None => {
@@ -170,22 +143,23 @@ pub fn merge_toml_values(
 #[cfg(test)]
 mod merge_toml_tests {
     use super::merge_toml_values;
-    use toml::Value;
 
     #[test]
-    fn language_toml_map_merges() {
-        const USER: &str = r#"
+    fn language_tomls() {
+        use toml::Value;
+
+        const USER: &str = "
         [[language]]
-        name = "nix"
-        test = "bbb"
-        indent = { tab-width = 4, unit = "    ", test = "aaa" }
-        "#;
+        name = \"nix\"
+        test = \"bbb\"
+        indent = { tab-width = 4, unit = \"    \", test = \"aaa\" }
+        ";
 
         let base: Value = toml::from_slice(include_bytes!("../../languages.toml"))
             .expect("Couldn't parse built-in languages config");
         let user: Value = toml::from_str(USER).unwrap();
 
-        let merged = merge_toml_values(base, user, true);
+        let merged = merge_toml_values(base, user);
         let languages = merged.get("language").unwrap().as_array().unwrap();
         let nix = languages
             .iter()
@@ -205,33 +179,4 @@ mod merge_toml_tests {
         // We didn't change comment-token so it should be same
         assert_eq!(nix.get("comment-token").unwrap().as_str().unwrap(), "#");
     }
-
-    #[test]
-    fn language_toml_nested_array_merges() {
-        const USER: &str = r#"
-        [[language]]
-        name = "typescript"
-        language-server = { command = "deno", args = ["lsp"] }
-        "#;
-
-        let base: Value = toml::from_slice(include_bytes!("../../languages.toml"))
-            .expect("Couldn't parse built-in languages config");
-        let user: Value = toml::from_str(USER).unwrap();
-
-        let merged = merge_toml_values(base, user, true);
-        let languages = merged.get("language").unwrap().as_array().unwrap();
-        let ts = languages
-            .iter()
-            .find(|v| v.get("name").unwrap().as_str().unwrap() == "typescript")
-            .unwrap();
-        assert_eq!(
-            ts.get("language-server")
-                .unwrap()
-                .get("args")
-                .unwrap()
-                .as_array()
-                .unwrap(),
-            &vec![Value::String("lsp".into())]
-        )
-    }
 }