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 a key might leave the hosting table empty. For now we do not
delete such empty tables, as there may be cases where an empty table is
semantically meaningful (#4458 (comment)).

```toml
[table]
key = "value"

[another-table]
key = "value"
```

Running `jj config unset --user table.key` will leave us with `table`
being empty:
```toml
[table]

[another-table]
key = "value"
```
  • Loading branch information
pylbrecht committed Oct 22, 2024
1 parent f4ec653 commit ee92399
Show file tree
Hide file tree
Showing 6 changed files with 208 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* New template functions `pad_start()`, `pad_end()`, `truncate_start()`, and
`truncate_end()` are added.

* New command `jj config unset` that unsets config values. For example,
`jj config unset --user user.name`.

### Fixed bugs

* Error on `trunk()` revset resolution is now handled gracefully.
Expand Down
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)
}
53 changes: 53 additions & 0 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use jj_lib::settings::ConfigResultExt as _;
use regex::Captures;
use regex::Regex;
use thiserror::Error;
use toml_edit::TableLike;
use tracing::instrument;

use crate::command_error::user_error;
Expand Down Expand Up @@ -583,6 +584,58 @@ 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 root: &mut dyn TableLike = doc.as_table_mut();
let target_table = key_iter.try_fold(root, |table, key| {
table
.get_mut(key)
.ok_or(config::ConfigError::NotFound(key.to_string()))
.and_then(|table| {
table
.as_table_like_mut()
.ok_or(config::ConfigError::Message(format!(
"{key} is not a table",
)))
})
})?;

// Remove config value
target_table
.remove(last_key)
.ok_or(config::ConfigError::NotFound(key.to_string()))?;

// 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
19 changes: 19 additions & 0 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ This document contains the help content for the `jj` command-line program.
* [`jj config list`↴](#jj-config-list)
* [`jj config path`↴](#jj-config-path)
* [`jj config set`↴](#jj-config-set)
* [`jj config unset`↴](#jj-config-unset)
* [`jj describe`↴](#jj-describe)
* [`jj diff`↴](#jj-diff)
* [`jj diffedit`↴](#jj-diffedit)
Expand Down Expand Up @@ -474,6 +475,7 @@ For file locations, supported config options, and other details about jj config,
* `list` — List variables set in config file, along with their values
* `path` — Print the path to the config file
* `set` — Update config file to set the given option to a given value
* `unset` — Update config file to unset the given option
Expand Down Expand Up @@ -575,6 +577,23 @@ Update config file to set the given option to a given value
## `jj config unset`
Update config file to unset the given option
**Usage:** `jj config unset <--user|--repo> <NAME>`
###### **Arguments:**
* `<NAME>`
###### **Options:**
* `--user` — Target the user-level config
* `--repo` — Target the repo-level config
## `jj describe`
Update the change description or other metadata
Expand Down
91 changes: 91 additions & 0 deletions cli/tests/test_config_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,97 @@ 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###"
Config error: configuration property "nonexistent" not found
For help, see https://martinvonz.github.io/jj/latest/config/.
"###);
}

#[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.clone());
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_ok(&repo_path, &["config", "set", "--user", "foo", "true"]);
test_env.jj_cmd_ok(&repo_path, &["config", "unset", "--user", "foo"]);

test_env.jj_cmd_ok(
&repo_path,
&["config", "set", "--user", "table.foo", "true"],
);
test_env.jj_cmd_ok(&repo_path, &["config", "unset", "--user", "table.foo"]);

test_env.jj_cmd_ok(
&repo_path,
&["config", "set", "--user", "inline-table", "{ foo = true }"],
);
test_env.jj_cmd_ok(
&repo_path,
&["config", "unset", "--user", "inline-table.foo"],
);

test_env.jj_cmd_ok(
&repo_path,
&[
"config",
"set",
"--user",
"table.inline-table",
"{ foo = true }",
],
);
test_env.jj_cmd_ok(
&repo_path,
&["config", "unset", "--user", "table.inline-table.foo"],
);
// TODO(pylbrecht): add cases for arrays and arrays of tables

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, @r#"
inline-table = {}
[table]
inline-table = {}
"#);
}

#[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 ee92399

Please sign in to comment.