From ebec82ee0c2a2a5487bcab67624c37a92284cc91 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 10 Oct 2023 22:02:21 -0700 Subject: [PATCH] cli: allow overwriting non-scalar with `jj config set` Before this patch, it was an error to run `jj config set --user foo '[1]'` twice. But it's only been broken since the previous commit because '[1]' was interpreted as a string before then. --- cli/src/cli_util.rs | 10 +++++----- cli/tests/test_config_command.rs | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index f2e5b1a86..a851688c8 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -2199,13 +2199,13 @@ pub fn write_config_value_to_file( )) })?; } - // Error out if overwriting non-scalar value for key (table or array). + // 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) => {} - Some(toml_edit::Item::Value(val)) if !val.is_array() && !val.is_inline_table() => {} - _ => { + 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 non-scalar value with scalar" + "Failed to set {key}: would overwrite entire table" ))); } } diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index b1f013f81..ab06b0434 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -340,7 +340,7 @@ fn test_config_set_for_repo() { #[test] fn test_config_set_toml_types() { let mut test_env = TestEnvironment::default(); - test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); let user_config_path = test_env.config_path().join("config.toml"); test_env.set_config_path(user_config_path.clone()); let repo_path = test_env.env_root().join("repo"); @@ -382,8 +382,26 @@ fn test_config_set_type_mismatch() { &["config", "set", "--user", "test-table", "not-a-table"], ); insta::assert_snapshot!(stderr, @r###" - Error: Failed to set test-table: would overwrite entire non-scalar value with scalar + Error: Failed to set test-table: would overwrite entire table "###); + + // But it's fine to overwrite arrays and inline tables + test_env.jj_cmd_success( + &repo_path, + &["config", "set", "--user", "test-table.array", "[1,2,3]"], + ); + test_env.jj_cmd_success( + &repo_path, + &["config", "set", "--user", "test-table.array", "[4,5,6]"], + ); + test_env.jj_cmd_success( + &repo_path, + &["config", "set", "--user", "test-table.inline", "{ x = 42}"], + ); + test_env.jj_cmd_success( + &repo_path, + &["config", "set", "--user", "test-table.inline", "42"], + ); } #[test]