diff --git a/cli/src/config.rs b/cli/src/config.rs index cacadcdcc..af6b8f5e0 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -426,23 +426,20 @@ pub fn parse_config_args(toml_strs: &[ConfigArg]) -> Result, Co .try_collect() } -fn read_config(path: &Path) -> Result, CommandError> { - let config_toml = std::fs::read_to_string(path).or_else(|err| { - match err.kind() { +fn load_config_file_or_empty( + source: ConfigSource, + path: &Path, +) -> Result { + match ConfigLayer::load_from_file(source, path.into()) { + Ok(layer) => Ok(layer), + Err(ConfigLoadError::Read(err)) if err.error.kind() == std::io::ErrorKind::NotFound => { // If config doesn't exist yet, read as empty and we'll write one. - std::io::ErrorKind::NotFound => Ok("".to_string()), - _ => Err(user_error_with_message( - format!("Failed to read file {path}", path = path.display()), - err, - )), + let mut layer = ConfigLayer::empty(source); + layer.path = Some(path.into()); + Ok(layer) } - })?; - config_toml.parse().map_err(|err| { - user_error_with_message( - format!("Failed to parse file {path}", path = path.display()), - err, - ) - }) + Err(err) => Err(err), + } } fn write_config(path: &Path, doc: &toml_edit::DocumentMut) -> Result<(), CommandError> { @@ -459,43 +456,20 @@ pub fn write_config_value_to_file( value: toml_edit::Value, path: &Path, ) -> Result<(), CommandError> { - let mut doc = read_config(path)?.into_mut(); - - // Apply config value - let mut target_table = doc.as_table_mut(); - let mut key_parts_iter = key.components(); - let last_key_part = key_parts_iter.next_back().expect("key must not be empty"); - for key_part in key_parts_iter { - target_table = target_table - .entry(key_part) - .or_insert_with(|| toml_edit::Item::Table(toml_edit::Table::new())) - .as_table_mut() - .ok_or_else(|| { - user_error(format!( - "Failed to set {key}: would overwrite non-table value with parent table" - )) - })?; - } - // Error out if overwriting non-scalar value for key (table or array) with - // scalar. - match target_table.get(last_key_part) { - None | Some(toml_edit::Item::None | toml_edit::Item::Value(_)) => {} - Some(toml_edit::Item::Table(_) | toml_edit::Item::ArrayOfTables(_)) => { - return Err(user_error(format!( - "Failed to set {key}: would overwrite entire table" - ))); - } - } - target_table[last_key_part] = toml_edit::Item::Value(value); - - write_config(path, &doc) + // TODO: Load config layer by caller. Here we use a dummy source for now. + let mut layer = load_config_file_or_empty(ConfigSource::User, path)?; + layer + .set_value(key, value) + .map_err(|err| user_error_with_message(format!("Failed to set {key}"), err))?; + write_config(path, &layer.data) } pub fn remove_config_value_from_file( key: &ConfigNamePathBuf, path: &Path, ) -> Result<(), CommandError> { - let mut doc = read_config(path)?.into_mut(); + // TODO: Load config layer by caller. Here we use a dummy source for now. + let mut doc = load_config_file_or_empty(ConfigSource::User, path)?.data; // Find target table let mut key_iter = key.components(); diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 2c1b9f3c7..191a2507b 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -578,9 +578,10 @@ fn test_config_set_type_mismatch() { &repo_path, &["config", "set", "--user", "test-table", "not-a-table"], ); - insta::assert_snapshot!(stderr, @r###" - Error: Failed to set test-table: would overwrite entire table - "###); + insta::assert_snapshot!(stderr, @r" + Error: Failed to set test-table + Caused by: Would overwrite entire table test-table + "); // But it's fine to overwrite arrays and inline tables test_env.jj_cmd_success( @@ -617,9 +618,10 @@ fn test_config_set_nontable_parent() { &repo_path, &["config", "set", "--user", "test-nontable.foo", "test-val"], ); - insta::assert_snapshot!(stderr, @r###" - Error: Failed to set test-nontable.foo: would overwrite non-table value with parent table - "###); + insta::assert_snapshot!(stderr, @r" + Error: Failed to set test-nontable.foo + Caused by: Would overwrite non-table value with parent table test-nontable + "); } #[test]