on master help sign in
Only merge top-level array when merging `languages.toml` (#2215)
2c7f770Michael Davis
helix-loader/src/config.rs+1−1
@@ -19,7 +19,7 @@ pub fn user_lang_config() -> Result<toml::Value, toml::de::Error> {
19 .into_iter()
19 .into_iter()
20 .chain([default_lang_config()].into_iter())
20 .chain([default_lang_config()].into_iter())
21 .fold(toml::Value::Table(toml::value::Table::default()), |a, b| {
21 .fold(toml::Value::Table(toml::value::Table::default()), |a, b| {
22− crate::merge_toml_values(b, a)
22+ crate::merge_toml_values(b, a, true)
23 });
23 });
24
24
25 Ok(config)
25 Ok(config)
helix-loader/src/lib.rs+78−23
@@ -98,8 +98,24 @@ pub fn find_root_impl(root: Option<&str>, root_markers: &[String]) -> Vec<std::p
98 directories
98 directories
99}
99}
100
100
101−// right overrides left
101+/// Merge two TOML documents, merging values from `right` onto `left`
102+///
103+/// When an array exists in both `left` and `right`, `right`'s array is
104+/// used. When a table exists in both `left` and `right`, the merged table
105+/// consists of all keys in `left`'s table unioned with all keys in `right`
106+/// with the values of `right` being merged recursively onto values of
107+/// `left`.
108+///
109+/// `merge_toplevel_arrays` controls whether a top-level array in the TOML
110+/// document is merged instead of overridden. This is useful for TOML
111+/// documents that use a top-level array of values like the `languages.toml`,
112+/// where one usually wants to override or add to the array instead of
113+/// replacing it altogether.
102−pub fn merge_toml_values(left: toml::Value, right: toml::Value) -> toml::Value {
114+pub fn merge_toml_values(
115+ left: toml::Value,
116+ right: toml::Value,
117+ merge_toplevel_arrays: bool,
118+) -> toml::Value {
103 use toml::Value;
119 use toml::Value;
104
120
105 fn get_name(v: &Value) -> Option<&str> {
121 fn get_name(v: &Value) -> Option<&str> {
@@ -108,24 +124,35 @@ pub fn merge_toml_values(left: toml::Value, right: toml::Value) -> toml::Value {
108
124
109 match (left, right) {
125 match (left, right) {
110 (Value::Array(mut left_items), Value::Array(right_items)) => {
126 (Value::Array(mut left_items), Value::Array(right_items)) => {
127+ // The top-level arrays should be merged but nested arrays should
128+ // act as overrides. For the `languages.toml` config, this means
129+ // that you can specify a sub-set of languages in an overriding
130+ // `languages.toml` but that nested arrays like Language Server
131+ // arguments are replaced instead of merged.
132+ if merge_toplevel_arrays {
111− left_items.reserve(right_items.len());
133+ left_items.reserve(right_items.len());
112− for rvalue in right_items {
134+ for rvalue in right_items {
113− let lvalue = get_name(&rvalue)
135+ let lvalue = get_name(&rvalue)
136+ .and_then(|rname| {
114− .and_then(|rname| left_items.iter().position(|v| get_name(v) == Some(rname)))
137+ left_items.iter().position(|v| get_name(v) == Some(rname))
138+ })
115− .map(|lpos| left_items.remove(lpos));
139+ .map(|lpos| left_items.remove(lpos));
116− let mvalue = match lvalue {
140+ let mvalue = match lvalue {
117− Some(lvalue) => merge_toml_values(lvalue, rvalue),
141+ Some(lvalue) => merge_toml_values(lvalue, rvalue, false),
118− None => rvalue,
142+ None => rvalue,
119− };
143+ };
120− left_items.push(mvalue);
144+ left_items.push(mvalue);
145+ }
146+ Value::Array(left_items)
147+ } else {
148+ Value::Array(right_items)
121 }
149 }
122− Value::Array(left_items)
123 }
150 }
124 (Value::Table(mut left_map), Value::Table(right_map)) => {
151 (Value::Table(mut left_map), Value::Table(right_map)) => {
125 for (rname, rvalue) in right_map {
152 for (rname, rvalue) in right_map {
126 match left_map.remove(&rname) {
153 match left_map.remove(&rname) {
127 Some(lvalue) => {
154 Some(lvalue) => {
128− let merged_value = merge_toml_values(lvalue, rvalue);
155+ let merged_value = merge_toml_values(lvalue, rvalue, merge_toplevel_arrays);
129 left_map.insert(rname, merged_value);
156 left_map.insert(rname, merged_value);
130 }
157 }
131 None => {
158 None => {
@@ -143,23 +170,22 @@ pub fn merge_toml_values(left: toml::Value, right: toml::Value) -> toml::Value {
143#[cfg(test)]
170#[cfg(test)]
144mod merge_toml_tests {
171mod merge_toml_tests {
145 use super::merge_toml_values;
172 use super::merge_toml_values;
173+ use toml::Value;
146
174
147 #[test]
175 #[test]
148− fn language_tomls() {
176+ fn language_toml_map_merges() {
149− use toml::Value;
150−
151− const USER: &str = "
177+ const USER: &str = r#"
152 [[language]]
178 [[language]]
153− name = \"nix\"
179+ name = "nix"
154− test = \"bbb\"
180+ test = "bbb"
155− indent = { tab-width = 4, unit = \" \", test = \"aaa\" }
181+ indent = { tab-width = 4, unit = " ", test = "aaa" }
156− ";
182+ "#;
157
183
158 let base: Value = toml::from_slice(include_bytes!("../../languages.toml"))
184 let base: Value = toml::from_slice(include_bytes!("../../languages.toml"))
159 .expect("Couldn't parse built-in languages config");
185 .expect("Couldn't parse built-in languages config");
160 let user: Value = toml::from_str(USER).unwrap();
186 let user: Value = toml::from_str(USER).unwrap();
161
187
162− let merged = merge_toml_values(base, user);
188+ let merged = merge_toml_values(base, user, true);
163 let languages = merged.get("language").unwrap().as_array().unwrap();
189 let languages = merged.get("language").unwrap().as_array().unwrap();
164 let nix = languages
190 let nix = languages
165 .iter()
191 .iter()
@@ -179,4 +205,33 @@ mod merge_toml_tests {
179 // We didn't change comment-token so it should be same
205 // We didn't change comment-token so it should be same
180 assert_eq!(nix.get("comment-token").unwrap().as_str().unwrap(), "#");
206 assert_eq!(nix.get("comment-token").unwrap().as_str().unwrap(), "#");
181 }
207 }
208+
209+ #[test]
210+ fn language_toml_nested_array_merges() {
211+ const USER: &str = r#"
212+ [[language]]
213+ name = "typescript"
214+ language-server = { command = "deno", args = ["lsp"] }
215+ "#;
216+
217+ let base: Value = toml::from_slice(include_bytes!("../../languages.toml"))
218+ .expect("Couldn't parse built-in languages config");
219+ let user: Value = toml::from_str(USER).unwrap();
220+
221+ let merged = merge_toml_values(base, user, true);
222+ let languages = merged.get("language").unwrap().as_array().unwrap();
223+ let ts = languages
224+ .iter()
225+ .find(|v| v.get("name").unwrap().as_str().unwrap() == "typescript")
226+ .unwrap();
227+ assert_eq!(
228+ ts.get("language-server")
229+ .unwrap()
230+ .get("args")
231+ .unwrap()
232+ .as_array()
233+ .unwrap(),
234+ &vec![Value::String("lsp".into())]
235+ )
236+ }
182}
237}