From a1925cef5007910b1316898e971ed718ec6b69d8 Mon Sep 17 00:00:00 2001 From: pylbrecht Date: Thu, 26 Sep 2024 11:09:06 +0200 Subject: [PATCH] cli: add `jj config unset` Allow unsetting config values similar to `git config unset`. ```bash $ jj config set --user some-key some-val $ jj config get some-key some-val $ jj config unset --user some-key $ jj config get some-key Config error: configuration property "some-key" not found For help, see https://martinvonz.github.io/jj/latest/config/. ``` Unsetting nonexistent keys will result in an error: ```bash $ jj config unset --user nonexistent Error: configuration property "nonexistent" not found For help, see https://martinvonz.github.io/jj/latest/config/. ``` If unsetting a key leaves the hosting table empty, the table is removed as well. Given the following config: ```toml [table] key = value [another-table] key = value ``` Running `jj config unset --user table.key` will leave us with: ```toml [another-table] key = value ``` --- cli/src/commands/config/mod.rs | 6 +++ cli/src/commands/config/unset.rs | 36 +++++++++++++++ cli/src/config.rs | 65 ++++++++++++++++++++++++++ cli/tests/test_config_command.rs | 78 ++++++++++++++++++++++++++++++++ 4 files changed, 185 insertions(+) create mode 100644 cli/src/commands/config/unset.rs diff --git a/cli/src/commands/config/mod.rs b/cli/src/commands/config/mod.rs index c6c1f624e41..7d60fb08b61 100644 --- a/cli/src/commands/config/mod.rs +++ b/cli/src/commands/config/mod.rs @@ -17,8 +17,11 @@ mod get; mod list; mod path; mod set; +mod unset; use tracing::instrument; +use unset::cmd_config_unset; +use unset::ConfigUnsetArgs; use self::edit::cmd_config_edit; use self::edit::ConfigEditArgs; @@ -82,6 +85,8 @@ pub(crate) enum ConfigCommand { Path(ConfigPathArgs), #[command(visible_alias("s"))] Set(ConfigSetArgs), + #[command(visible_alias("u"))] + Unset(ConfigUnsetArgs), } #[instrument(skip_all)] @@ -96,5 +101,6 @@ pub(crate) fn cmd_config( ConfigCommand::List(args) => cmd_config_list(ui, command, args), ConfigCommand::Path(args) => cmd_config_path(ui, command, args), ConfigCommand::Set(args) => cmd_config_set(ui, command, args), + ConfigCommand::Unset(args) => cmd_config_unset(command, args), } } diff --git a/cli/src/commands/config/unset.rs b/cli/src/commands/config/unset.rs new file mode 100644 index 00000000000..0613c58af76 --- /dev/null +++ b/cli/src/commands/config/unset.rs @@ -0,0 +1,36 @@ +use tracing::instrument; + +use super::ConfigLevelArgs; +use crate::cli_util::get_new_config_file_path; +use crate::cli_util::CommandHelper; +use crate::command_error::user_error; +use crate::command_error::CommandError; +use crate::config::remove_config_value_from_file; +use crate::config::ConfigNamePathBuf; + +/// Update config file to unset the given option. +#[derive(clap::Args, Clone, Debug)] +pub struct ConfigUnsetArgs { + #[arg(required = true)] + name: ConfigNamePathBuf, + #[command(flatten)] + level: ConfigLevelArgs, +} + +#[instrument(skip_all)] +pub fn cmd_config_unset( + command: &CommandHelper, + args: &ConfigUnsetArgs, +) -> Result<(), CommandError> { + let config_path = get_new_config_file_path(&args.level.expect_source_kind(), command)?; + if config_path.is_dir() { + return Err(user_error(format!( + "Can't set config in path {path} (dirs not supported)", + path = config_path.display() + ))); + } + + // TODO(pylbrecht): do we need to check_wc_author() here? + + remove_config_value_from_file(&args.name, &config_path) +} diff --git a/cli/src/config.rs b/cli/src/config.rs index c22871dc234..7a0f6bfa510 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -582,6 +582,71 @@ fn read_config_path(config_path: &Path) -> Result Result<(), CommandError> { + // Read config + let config_toml = std::fs::read_to_string(path).or_else(|err| { + match err.kind() { + // 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 doc: toml_edit::Document = config_toml.parse().map_err(|err| { + user_error_with_message( + format!("Failed to parse file {path}", path = path.display()), + err, + ) + })?; + + let mut key_iter = key.components(); + let last_key = key_iter.next_back().expect("key must not be empty"); + let table_key = key_iter.next_back(); + let target_table = match table_key { + Some(table_key) => doc + .get_mut(table_key) + .expect("table must exist") + .as_table_mut() + .expect("must be table"), + None => doc.as_table_mut(), + }; + + // Remove config value + if target_table.remove(&last_key).is_none() { + // TODO(pylbrecht): use proper error type + return Err(user_error(format!( + "configuration property \"{key}\" not found", + key = key + ))); + } + + // Remove empty tables + if table_key.is_some() && target_table.is_empty() { + let parent_table = match key_iter.next_back() { + Some(parent_key) => doc + .get_mut(parent_key) + .expect("table must exist") + .as_table_mut() + .expect("must be table"), + None => doc.as_table_mut(), + }; + parent_table.remove(table_key.unwrap()); + } + + // Write config back + std::fs::write(path, doc.to_string()).map_err(|err| { + user_error_with_message( + format!("Failed to write file {path}", path = path.display()), + err, + ) + }) +} + pub fn write_config_value_to_file( key: &ConfigNamePathBuf, value: toml_edit::Value, diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 24c77059197..5f61a732bd2 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -602,6 +602,84 @@ fn test_config_set_nontable_parent() { "###); } +#[test] +fn test_config_unset_non_existent_key() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + // Point to a config file since `config set` can't handle directories. + let user_config_path = test_env.config_path().join("config.toml"); + test_env.set_config_path(user_config_path); + let repo_path = test_env.env_root().join("repo"); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["config", "unset", "--user", "nonexistent"]); + insta::assert_snapshot!(stderr, @r###" + Error: configuration property "nonexistent" not found + "###); +} + +#[test] +fn test_config_unset_for_user() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + // Point to a config file since `config set` can't handle directories. + let user_config_path = test_env.config_path().join("config.toml"); + test_env.set_config_path(user_config_path.to_owned()); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok( + &repo_path, + &["config", "set", "--user", "test-key", "test-val"], + ); + test_env.jj_cmd_ok(&repo_path, &["config", "unset", "--user", "test-key"]); + + let user_config_toml = std::fs::read_to_string(&user_config_path) + .unwrap_or_else(|_| panic!("Failed to read file {}", user_config_path.display())); + insta::assert_snapshot!(user_config_toml, @""); +} + +#[test] +fn test_config_unset_for_user_table_keys() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + // Point to a config file since `config set` can't handle directories. + let user_config_path = test_env.config_path().join("config.toml"); + test_env.set_config_path(user_config_path.to_owned()); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok( + &repo_path, + &["config", "set", "--user", "test-table.foo", "test-val"], + ); + test_env.jj_cmd_ok(&repo_path, &["config", "unset", "--user", "test-table.foo"]); + + let user_config_toml = std::fs::read_to_string(&user_config_path) + .unwrap_or_else(|_| panic!("Failed to read file {}", user_config_path.display())); + insta::assert_snapshot!(user_config_toml, @""); +} + +#[test] +fn test_config_unset_for_repo() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok( + &repo_path, + &["config", "set", "--repo", "test-key", "test-val"], + ); + test_env.jj_cmd_ok(&repo_path, &["config", "unset", "--repo", "test-key"]); + + let expected_repo_config_path = repo_path.join(".jj/repo/config.toml"); + let repo_config_toml = + std::fs::read_to_string(&expected_repo_config_path).unwrap_or_else(|_| { + panic!( + "Failed to read file {}", + expected_repo_config_path.display() + ) + }); + insta::assert_snapshot!(repo_config_toml, @""); +} + #[test] fn test_config_edit_missing_opt() { let test_env = TestEnvironment::default();