Implement "config set" subcommand

Uses toml_edit to support simple config edits like:
  jj config set --repo user.email "somebody@example.com"
This commit is contained in:
David Barnett 2023-01-14 22:23:44 -06:00 committed by David Barnett
parent bbd6ef0c7b
commit 99cb0ba7c5
7 changed files with 326 additions and 29 deletions

View file

@ -26,6 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj git fetch` now supports a `--branch` argument to fetch some of the
branches only.
* `jj config set` command allows simple config edits like
`jj config set --repo user.email "somebody@example.com"`
### Fixed bugs
* Modify/delete conflicts now include context lines

41
Cargo.lock generated
View file

@ -829,6 +829,7 @@ dependencies = [
"textwrap 0.16.0",
"thiserror",
"timeago",
"toml_edit",
"tracing",
"tracing-subscriber",
]
@ -1023,6 +1024,15 @@ dependencies = [
"minimal-lexical",
]
[[package]]
name = "nom8"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ae01545c9c7fc4486ab7debaf2aad7003ac19431791868fb2e8066df97fad2f8"
dependencies = [
"memchr",
]
[[package]]
name = "normalize-line-endings"
version = "0.3.0"
@ -1594,6 +1604,15 @@ dependencies = [
"serde",
]
[[package]]
name = "serde_spanned"
version = "0.6.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0efd8caf556a6cebd3b285caf480045fcc1ac04f6bd786b09a6f11af30c4fcf4"
dependencies = [
"serde",
]
[[package]]
name = "sha2"
version = "0.10.6"
@ -1866,6 +1885,28 @@ dependencies = [
"serde",
]
[[package]]
name = "toml_datetime"
version = "0.6.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3ab8ed2edee10b50132aed5f331333428b011c99402b5a534154ed15746f9622"
dependencies = [
"serde",
]
[[package]]
name = "toml_edit"
version = "0.19.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "90a238ee2e6ede22fb95350acc78e21dc40da00bb66c0334bde83de4ed89424e"
dependencies = [
"indexmap",
"nom8",
"serde",
"serde_spanned",
"toml_datetime",
]
[[package]]
name = "tracing"
version = "0.1.37"

View file

@ -62,6 +62,7 @@ thiserror = "1.0.38"
tracing = "0.1.37"
tracing-subscriber = { version = "0.3.16", default-features = false, features = ["std", "ansi", "env-filter", "fmt"] }
indexmap = "1.9.2"
toml_edit = { version = "0.19.1", features = ["serde"] }
[target.'cfg(unix)'.dependencies]
libc = { version = "0.2.139" }

View file

@ -21,6 +21,7 @@ use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::process::ExitCode;
use std::rc::Rc;
use std::str::FromStr;
use std::sync::Arc;
use clap::builder::{NonEmptyStringValueParser, TypedValueParser, ValueParserFactory};
@ -55,10 +56,13 @@ use jujutsu_lib::working_copy::{
use jujutsu_lib::workspace::{Workspace, WorkspaceInitError, WorkspaceLoadError, WorkspaceLoader};
use jujutsu_lib::{dag_walk, file_util, git, revset};
use thiserror::Error;
use toml_edit;
use tracing_subscriber::prelude::*;
use crate::commit_templater;
use crate::config::{AnnotatedValue, CommandNameAndArgs, LayeredConfigs};
use crate::config::{
config_path, AnnotatedValue, CommandNameAndArgs, ConfigSource, LayeredConfigs,
};
use crate::formatter::{Formatter, PlainTextFormatter};
use crate::merge_tools::{ConflictResolveError, DiffEditError};
use crate::template_parser::{TemplateAliasesMap, TemplateParseError};
@ -1633,6 +1637,94 @@ pub fn serialize_config_value(value: &config::Value) -> String {
}
}
pub fn write_config_value_to_file(
key: &str,
value_str: &str,
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(format!(
"Failed to read file {path}: {err:?}",
path = path.display()
))),
}
})?;
let mut doc = toml_edit::Document::from_str(&config_toml).map_err(|err| {
user_error(format!(
"Failed to parse file {path}: {err:?}",
path = path.display()
))
})?;
// Apply config value
// Iterpret value as string unless it's another simple scalar type.
// TODO(#531): Infer types based on schema (w/ --type arg to override).
let item = match toml_edit::Value::from_str(value_str) {
Ok(value @ toml_edit::Value::Boolean(..))
| Ok(value @ toml_edit::Value::Integer(..))
| Ok(value @ toml_edit::Value::Float(..))
| Ok(value @ toml_edit::Value::String(..)) => toml_edit::value(value),
_ => toml_edit::value(value_str),
};
let mut target_table = doc.as_table_mut();
let mut key_parts_iter = key.split('.');
// Note: split guarantees at least one item.
let last_key_part = key_parts_iter.next_back().unwrap();
for key_part in key_parts_iter {
target_table = target_table
.entry(key_part)
.or_insert_with(|| toml_edit::Item::Table(toml_edit::Table::new()))
.as_table_mut()
.ok_or_else(|| {
user_error(format!(
"Failed to set {key}: would overwrite non-table value with parent table"
))
})?;
}
// Error out if overwriting non-scalar value for key (table or array).
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() => {}
_ => {
return Err(user_error(format!(
"Failed to set {key}: would overwrite entire non-scalar value with scalar"
)))
}
}
target_table[last_key_part] = item;
// Write config back
std::fs::write(path, doc.to_string()).map_err(|err| {
user_error(format!(
"Failed to write file {path}: {err:?}",
path = path.display()
))
})
}
pub fn get_config_file_path(
config_source: &ConfigSource,
workspace_loader: &WorkspaceLoader,
) -> Result<PathBuf, CommandError> {
let edit_path = match config_source {
// TODO(#531): Special-case for editors that can't handle viewing directories?
ConfigSource::User => {
config_path()?.ok_or_else(|| user_error("No repo config path found to edit"))?
}
ConfigSource::Repo => workspace_loader.repo_path().join("config.toml"),
_ => {
return Err(user_error(format!(
"Can't get path for config source {config_source:?}"
)));
}
};
Ok(edit_path)
}
pub fn run_ui_editor(settings: &UserSettings, edit_path: &PathBuf) -> Result<(), CommandError> {
let editor: CommandNameAndArgs = settings
.config()

View file

@ -49,12 +49,13 @@ use jujutsu_lib::{conflicts, file_util, revset};
use maplit::{hashmap, hashset};
use crate::cli_util::{
self, check_stale_working_copy, print_checkout_stats, resolve_multiple_nonempty_revsets,
resolve_mutliple_nonempty_revsets_flag_guarded, run_ui_editor, serialize_config_value,
short_commit_hash, user_error, user_error_with_hint, Args, CommandError, CommandHelper,
DescriptionArg, RevisionArg, WorkspaceCommandHelper,
self, check_stale_working_copy, get_config_file_path, print_checkout_stats,
resolve_multiple_nonempty_revsets, resolve_mutliple_nonempty_revsets_flag_guarded,
run_ui_editor, serialize_config_value, short_commit_hash, user_error, user_error_with_hint,
write_config_value_to_file, Args, CommandError, CommandHelper, DescriptionArg, RevisionArg,
WorkspaceCommandHelper,
};
use crate::config::{config_path, AnnotatedValue, ConfigSource};
use crate::config::{AnnotatedValue, ConfigSource};
use crate::diff_util::{self, DiffFormat, DiffFormatArgs};
use crate::formatter::{Formatter, PlainTextFormatter};
use crate::graphlog::{get_graphlog, Edge};
@ -153,6 +154,19 @@ struct ConfigArgs {
repo: bool,
}
impl ConfigArgs {
fn get_source_kind(&self) -> ConfigSource {
if self.user {
ConfigSource::User
} else if self.repo {
ConfigSource::Repo
} else {
// Shouldn't be reachable unless clap ArgGroup is broken.
panic!("No config_level provided");
}
}
}
/// Manage config options
///
/// Operates on jj configuration, which comes from the config file and
@ -163,14 +177,12 @@ struct ConfigArgs {
///
/// For supported config options and more details about jj config, see
/// https://github.com/martinvonz/jj/blob/main/docs/config.md.
///
/// Note: Currently only supports getting config options and editing config
/// files, but support for setting options is also planned (see
/// https://github.com/martinvonz/jj/issues/531).
#[derive(clap::Subcommand, Clone, Debug)]
enum ConfigSubcommand {
#[command(visible_alias("l"))]
List(ConfigListArgs),
#[command(visible_alias("s"))]
Set(ConfigSetArgs),
#[command(visible_alias("e"))]
Edit(ConfigEditArgs),
}
@ -188,6 +200,18 @@ struct ConfigListArgs {
// TODO(#1047): Support ConfigArgs (--user or --repo).
}
/// Update config file to set the given option to a given value.
#[derive(clap::Args, Clone, Debug)]
struct ConfigSetArgs {
#[arg(required = true)]
name: String,
#[arg(required = true)]
value: String,
#[clap(flatten)]
config_args: ConfigArgs,
}
/// Start an editor on a jj config file.
#[derive(clap::Args, Clone, Debug)]
struct ConfigEditArgs {
#[clap(flatten)]
@ -1064,6 +1088,7 @@ fn cmd_config(
) -> Result<(), CommandError> {
match subcommand {
ConfigSubcommand::List(sub_args) => cmd_config_list(ui, command, sub_args),
ConfigSubcommand::Set(sub_args) => cmd_config_set(ui, command, sub_args),
ConfigSubcommand::Edit(sub_args) => cmd_config_edit(ui, command, sub_args),
}
}
@ -1110,23 +1135,34 @@ fn cmd_config_list(
Ok(())
}
fn cmd_config_set(
_ui: &mut Ui,
command: &CommandHelper,
args: &ConfigSetArgs,
) -> Result<(), CommandError> {
let config_path = get_config_file_path(
&args.config_args.get_source_kind(),
command.workspace_loader()?,
)?;
if config_path.is_dir() {
return Err(user_error(format!(
"Can't set config in path {path} (dirs not supported)",
path = config_path.display()
)));
}
write_config_value_to_file(&args.name, &args.value, &config_path)
}
fn cmd_config_edit(
_ui: &mut Ui,
command: &CommandHelper,
args: &ConfigEditArgs,
) -> Result<(), CommandError> {
let edit_path = if args.config_args.user {
// TODO(#531): Special-case for editors that can't handle viewing directories?
config_path()?.ok_or_else(|| user_error("No repo config path found to edit"))?
} else if args.config_args.repo {
let workspace_loader = command.workspace_loader()?;
workspace_loader.repo_path().join("config.toml")
} else {
// Shouldn't be reachable unless clap ArgGroup is broken.
panic!("No config_level provided");
};
run_ui_editor(command.settings(), &edit_path)?;
Ok(())
let config_path = get_config_file_path(
&args.config_args.get_source_kind(),
command.workspace_loader()?,
)?;
run_ui_editor(command.settings(), &config_path)
}
fn cmd_checkout(

View file

@ -24,7 +24,7 @@ pub struct TestEnvironment {
_temp_dir: TempDir,
env_root: PathBuf,
home_dir: PathBuf,
config_dir: PathBuf,
config_path: PathBuf,
env_vars: HashMap<String, String>,
config_file_number: RefCell<i64>,
command_number: RefCell<i64>,
@ -45,7 +45,7 @@ impl Default for TestEnvironment {
_temp_dir: tmp_dir,
env_root,
home_dir,
config_dir,
config_path: config_dir,
env_vars,
config_file_number: RefCell::new(0),
command_number: RefCell::new(0),
@ -64,7 +64,7 @@ impl TestEnvironment {
}
cmd.env("RUST_BACKTRACE", "1");
cmd.env("HOME", self.home_dir.to_str().unwrap());
cmd.env("JJ_CONFIG", self.config_dir.to_str().unwrap());
cmd.env("JJ_CONFIG", self.config_path.to_str().unwrap());
cmd.env("JJ_USER", "Test User");
cmd.env("JJ_EMAIL", "test.user@example.com");
cmd.env("JJ_OP_HOSTNAME", "host.example.com");
@ -119,18 +119,25 @@ impl TestEnvironment {
&self.home_dir
}
pub fn config_dir(&self) -> &PathBuf {
&self.config_dir
pub fn config_path(&self) -> &PathBuf {
&self.config_path
}
pub fn set_config_path(&mut self, config_path: PathBuf) {
self.config_path = config_path;
}
pub fn add_config(&self, content: &str) {
if self.config_path.is_file() {
panic!("add_config not supported when config_path is a file");
}
// Concatenating two valid TOML files does not (generally) result in a valid
// TOML file, so we use create a new file every time instead.
let mut config_file_number = self.config_file_number.borrow_mut();
*config_file_number += 1;
let config_file_number = *config_file_number;
std::fs::write(
self.config_dir
self.config_path
.join(format!("config{config_file_number:04}.toml")),
content,
)

View file

@ -265,6 +265,123 @@ fn test_config_layer_workspace() {
"###);
}
#[test]
fn test_config_set_missing_opts() {
let test_env = TestEnvironment::default();
let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "set"]);
insta::assert_snapshot!(stderr, @r###"
error: The following required arguments were not provided:
<--user|--repo>
<NAME>
<VALUE>
Usage: jj config set <--user|--repo> <NAME> <VALUE>
For more information try '--help'
"###);
}
#[test]
fn test_config_set_for_user() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
// 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_success(
&repo_path,
&["config", "set", "--user", "test-key", "test-val"],
);
test_env.jj_cmd_success(
&repo_path,
&["config", "set", "--user", "test-table.foo", "true"],
);
// Ensure test-key successfully written to user config.
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###"
test-key = "test-val"
[test-table]
foo = true
"###);
}
#[test]
fn test_config_set_for_repo() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
test_env.jj_cmd_success(
&repo_path,
&["config", "set", "--repo", "test-key", "test-val"],
);
test_env.jj_cmd_success(
&repo_path,
&["config", "set", "--repo", "test-table.foo", "true"],
);
// Ensure test-key successfully written to user config.
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, @r###"
test-key = "test-val"
[test-table]
foo = true
"###);
}
#[test]
fn test_config_set_type_mismatch() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_success(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);
let repo_path = test_env.env_root().join("repo");
test_env.jj_cmd_success(
&repo_path,
&["config", "set", "--user", "test-table.foo", "test-val"],
);
let stderr = test_env.jj_cmd_failure(
&repo_path,
&["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
"###);
}
#[test]
fn test_config_set_nontable_parent() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_success(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);
let repo_path = test_env.env_root().join("repo");
test_env.jj_cmd_success(
&repo_path,
&["config", "set", "--user", "test-nontable", "test-val"],
);
let stderr = test_env.jj_cmd_failure(
&repo_path,
&["config", "set", "--user", "test-nontable.foo", "test-val"],
);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to set test-nontable.foo: would overwrite non-table value with parent table
"###);
}
#[test]
fn test_config_edit_missing_opt() {
let test_env = TestEnvironment::default();
@ -288,7 +405,7 @@ fn test_config_edit_user() {
std::fs::write(
edit_script,
format!("expectpath\n{}", test_env.config_dir().to_str().unwrap()),
format!("expectpath\n{}", test_env.config_path().to_str().unwrap()),
)
.unwrap();
test_env.jj_cmd_success(&repo_path, &["config", "edit", "--user"]);