Skip to content

Commit

Permalink
Only merge top-level array when merging languages.toml (#2215)
Browse files Browse the repository at this point in the history
* Revert "Revert "override nested arrays when merging TOML (#2145)""

This reverts commit 35d2693.

* flip top-level table merging flag
  • Loading branch information
the-mikedavis authored Apr 23, 2022
1 parent 19d042d commit 2c7f770
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 24 deletions.
2 changes: 1 addition & 1 deletion helix-loader/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
crate::merge_toml_values(b, a, true)
});

Ok(config)
Expand Down
101 changes: 78 additions & 23 deletions helix-loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,24 @@ pub fn find_root_impl(root: Option<&str>, root_markers: &[String]) -> Vec<std::p
directories
}

// right overrides left
pub fn merge_toml_values(left: toml::Value, right: toml::Value) -> toml::Value {
/// 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 {
use toml::Value;

fn get_name(v: &Value) -> Option<&str> {
Expand All @@ -108,24 +124,35 @@ pub fn merge_toml_values(left: toml::Value, right: toml::Value) -> toml::Value {

match (left, right) {
(Value::Array(mut left_items), 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);
// 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)
}
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);
let merged_value = merge_toml_values(lvalue, rvalue, merge_toplevel_arrays);
left_map.insert(rname, merged_value);
}
None => {
Expand All @@ -143,23 +170,22 @@ pub fn merge_toml_values(left: toml::Value, right: toml::Value) -> toml::Value {
#[cfg(test)]
mod merge_toml_tests {
use super::merge_toml_values;
use toml::Value;

#[test]
fn language_tomls() {
use toml::Value;

const USER: &str = "
fn language_toml_map_merges() {
const USER: &str = r#"
[[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);
let merged = merge_toml_values(base, user, true);
let languages = merged.get("language").unwrap().as_array().unwrap();
let nix = languages
.iter()
Expand All @@ -179,4 +205,33 @@ 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())]
)
}
}

0 comments on commit 2c7f770

Please sign in to comment.