Skip to content

Commit

Permalink
cli: add jj config unset
Browse files Browse the repository at this point in the history
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"
```
  • Loading branch information
pylbrecht committed Oct 5, 2024
1 parent 9551794 commit 9b66e8c
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 0 deletions.
6 changes: 6 additions & 0 deletions cli/src/commands/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand 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),
}
}
36 changes: 36 additions & 0 deletions cli/src/commands/config/unset.rs
Original file line number Diff line number Diff line change
@@ -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)
}
65 changes: 65 additions & 0 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,71 @@ fn read_config_path(config_path: &Path) -> Result<config::Config, config::Config
.build()
}

pub fn remove_config_value_from_file(
key: &ConfigNamePathBuf,
path: &Path,
) -> 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,
Expand Down
78 changes: 78 additions & 0 deletions cli/tests/test_config_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 9b66e8c

Please sign in to comment.