cli: fix "config unset" to not delete a whole table
Some checks are pending
binaries / Build binary artifacts (push) Waiting to run
nix / flake check (push) Waiting to run
build / build (, macos-13) (push) Waiting to run
build / build (, macos-14) (push) Waiting to run
build / build (, ubuntu-latest) (push) Waiting to run
build / build (, windows-latest) (push) Waiting to run
build / build (--all-features, ubuntu-latest) (push) Waiting to run
build / Build jj-lib without Git support (push) Waiting to run
build / Check protos (push) Waiting to run
build / Check formatting (push) Waiting to run
build / Check that MkDocs can build the docs (push) Waiting to run
build / Check that MkDocs can build the docs with Poetry 1.8 (push) Waiting to run
build / cargo-deny (advisories) (push) Waiting to run
build / cargo-deny (bans licenses sources) (push) Waiting to run
build / Clippy check (push) Waiting to run
Codespell / Codespell (push) Waiting to run
website / prerelease-docs-build-deploy (ubuntu-latest) (push) Waiting to run
Scorecards supply-chain security / Scorecards analysis (push) Waiting to run

It can be a footgun, and is inconsistent because other mutation commands can't
overwrite a table.
This commit is contained in:
Yuya Nishihara 2024-11-17 21:50:56 +09:00
parent 694e20d255
commit 896ab5f179
3 changed files with 50 additions and 3 deletions

View file

@ -43,6 +43,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
### Fixed bugs
* `jj config unset <TABLE-NAME>` no longer removes a table (such as `[ui]`.)
## [0.23.0] - 2024-11-06
### Security fixes

View file

@ -669,9 +669,17 @@ pub fn remove_config_value_from_file(
})?;
// Remove config value
target_table
.remove(last_key)
.ok_or_else(|| config::ConfigError::NotFound(key.to_string()))?;
match target_table.entry(last_key) {
toml_edit::Entry::Occupied(entry) => {
if entry.get().is_table() {
return Err(user_error(format!("Won't remove table {key}")));
}
entry.remove();
}
toml_edit::Entry::Vacant(_) => {
return Err(config::ConfigError::NotFound(key.to_string()).into());
}
}
write_config(path, &doc)
}

View file

@ -14,6 +14,7 @@
use std::path::PathBuf;
use indoc::indoc;
use insta::assert_snapshot;
use itertools::Itertools;
use regex::Regex;
@ -642,6 +643,42 @@ fn test_config_unset_inline_table_key() {
"###);
}
#[test]
fn test_config_unset_table_like() {
let mut test_env = TestEnvironment::default();
// Point to a config file since `config unset` can't handle directories.
let user_config_path = test_env.config_path().join("config.toml");
test_env.set_config_path(user_config_path.clone());
std::fs::write(
&user_config_path,
indoc! {b"
inline-table = { foo = true }
[non-inline-table]
foo = true
"},
)
.unwrap();
// Inline table is a "value", so it can be deleted.
test_env.jj_cmd_success(
test_env.env_root(),
&["config", "unset", "--user", "inline-table"],
);
// Non-inline table cannot be deleted.
let stderr = test_env.jj_cmd_failure(
test_env.env_root(),
&["config", "unset", "--user", "non-inline-table"],
);
insta::assert_snapshot!(stderr, @"Error: Won't remove table non-inline-table");
let user_config_toml = std::fs::read_to_string(&user_config_path).unwrap();
insta::assert_snapshot!(user_config_toml, @r"
[non-inline-table]
foo = true
");
}
#[test]
fn test_config_unset_for_user() {
let mut test_env = TestEnvironment::default();