Refactor 'helix-loader::merge_toml_values' to use a 'merge-depth' instead of 'merge_toplevel_arrays' (#3080)

- This ensures that other values than just the arrays are overridden, like nested objects, where it makes sense
- merge_depth is set to 3 so that top-level language features are merged (like 'scope'), but everything deeper is overridden with the user-config
This commit is contained in:
Philipp Mildenberger 2022-07-26 02:40:38 +02:00 committed by GitHub
parent b7fa9ba601
commit 235237ddc4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 19 deletions

View file

@ -19,7 +19,23 @@ pub fn user_lang_config() -> Result<toml::Value, toml::de::Error> {
.into_iter() .into_iter()
.chain([default_lang_config()].into_iter()) .chain([default_lang_config()].into_iter())
.fold(toml::Value::Table(toml::value::Table::default()), |a, b| { .fold(toml::Value::Table(toml::value::Table::default()), |a, b| {
crate::merge_toml_values(b, a, true) // combines for example
// b:
// [[language]]
// name = "toml"
// language-server = { command = "taplo", args = ["lsp", "stdio"] }
//
// a:
// [[language]]
// language-server = { command = "/usr/bin/taplo" }
//
// into:
// [[language]]
// name = "toml"
// language-server = { command = "/usr/bin/taplo" }
//
// thus it overrides the third depth-level of b with values of a if they exist, but otherwise merges their values
crate::merge_toml_values(b, a, 3)
}); });
Ok(config) Ok(config)

View file

@ -113,11 +113,7 @@ pub fn find_root_impl(root: Option<&str>, root_markers: &[String]) -> Vec<std::p
/// documents that use a top-level array of values like the `languages.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 /// where one usually wants to override or add to the array instead of
/// replacing it altogether. /// replacing it altogether.
pub fn merge_toml_values( pub fn merge_toml_values(left: toml::Value, right: toml::Value, merge_depth: usize) -> toml::Value {
left: toml::Value,
right: toml::Value,
merge_toplevel_arrays: bool,
) -> toml::Value {
use toml::Value; use toml::Value;
fn get_name(v: &Value) -> Option<&str> { fn get_name(v: &Value) -> Option<&str> {
@ -131,7 +127,7 @@ pub fn merge_toml_values(
// that you can specify a sub-set of languages in an overriding // that you can specify a sub-set of languages in an overriding
// `languages.toml` but that nested arrays like Language Server // `languages.toml` but that nested arrays like Language Server
// arguments are replaced instead of merged. // arguments are replaced instead of merged.
if merge_toplevel_arrays { if merge_depth > 0 {
left_items.reserve(right_items.len()); left_items.reserve(right_items.len());
for rvalue in right_items { for rvalue in right_items {
let lvalue = get_name(&rvalue) let lvalue = get_name(&rvalue)
@ -140,7 +136,7 @@ pub fn merge_toml_values(
}) })
.map(|lpos| left_items.remove(lpos)); .map(|lpos| left_items.remove(lpos));
let mvalue = match lvalue { let mvalue = match lvalue {
Some(lvalue) => merge_toml_values(lvalue, rvalue, false), Some(lvalue) => merge_toml_values(lvalue, rvalue, merge_depth - 1),
None => rvalue, None => rvalue,
}; };
left_items.push(mvalue); left_items.push(mvalue);
@ -151,10 +147,11 @@ pub fn merge_toml_values(
} }
} }
(Value::Table(mut left_map), Value::Table(right_map)) => { (Value::Table(mut left_map), Value::Table(right_map)) => {
if merge_depth > 0 {
for (rname, rvalue) in right_map { for (rname, rvalue) in right_map {
match left_map.remove(&rname) { match left_map.remove(&rname) {
Some(lvalue) => { Some(lvalue) => {
let merged_value = merge_toml_values(lvalue, rvalue, merge_toplevel_arrays); let merged_value = merge_toml_values(lvalue, rvalue, merge_depth - 1);
left_map.insert(rname, merged_value); left_map.insert(rname, merged_value);
} }
None => { None => {
@ -163,6 +160,9 @@ pub fn merge_toml_values(
} }
} }
Value::Table(left_map) Value::Table(left_map)
} else {
Value::Table(right_map)
}
} }
// Catch everything else we didn't handle, and use the right value // Catch everything else we didn't handle, and use the right value
(_, value) => value, (_, value) => value,
@ -187,7 +187,7 @@ mod merge_toml_tests {
.expect("Couldn't parse built-in languages config"); .expect("Couldn't parse built-in languages config");
let user: Value = toml::from_str(USER).unwrap(); let user: Value = toml::from_str(USER).unwrap();
let merged = merge_toml_values(base, user, true); let merged = merge_toml_values(base, user, 3);
let languages = merged.get("language").unwrap().as_array().unwrap(); let languages = merged.get("language").unwrap().as_array().unwrap();
let nix = languages let nix = languages
.iter() .iter()
@ -220,7 +220,7 @@ mod merge_toml_tests {
.expect("Couldn't parse built-in languages config"); .expect("Couldn't parse built-in languages config");
let user: Value = toml::from_str(USER).unwrap(); let user: Value = toml::from_str(USER).unwrap();
let merged = merge_toml_values(base, user, true); let merged = merge_toml_values(base, user, 3);
let languages = merged.get("language").unwrap().as_array().unwrap(); let languages = merged.get("language").unwrap().as_array().unwrap();
let ts = languages let ts = languages
.iter() .iter()