From 8741b9f426f587329508f6e1ea2827aefa0bc8f6 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 30 Nov 2024 11:14:34 +0900 Subject: [PATCH] config: add convenient method to insert value into layer, migrate callers The current implementation is silly, which will be reimplemented to be using toml_edit::DocumentMut. "jj config set" will probably be ported to this API. --- cli/src/config.rs | 87 ++++++++++++++------------------ lib/src/config.rs | 26 ++++++++++ lib/tests/test_commit_builder.rs | 13 ++--- 3 files changed, 70 insertions(+), 56 deletions(-) diff --git a/cli/src/config.rs b/cli/src/config.rs index c974b1524b..2b9da5a188 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -358,21 +358,21 @@ pub fn config_from_environment( /// Environment variables that should be overridden by config values fn env_base_layer() -> ConfigLayer { - let mut builder = config::Config::builder(); + let mut layer = ConfigLayer::empty(ConfigSource::EnvBase); if env::var("NO_COLOR").is_ok() { // "User-level configuration files and per-instance command-line arguments // should override $NO_COLOR." https://no-color.org/ - builder = builder.set_override("ui.color", "never").unwrap(); + layer.insert_value("ui.color", "never").unwrap(); } if let Ok(value) = env::var("PAGER") { - builder = builder.set_override("ui.pager", value).unwrap(); + layer.insert_value("ui.pager", value).unwrap(); } if let Ok(value) = env::var("VISUAL") { - builder = builder.set_override("ui.editor", value).unwrap(); + layer.insert_value("ui.editor", value).unwrap(); } else if let Ok(value) = env::var("EDITOR") { - builder = builder.set_override("ui.editor", value).unwrap(); + layer.insert_value("ui.editor", value).unwrap(); } - ConfigLayer::with_data(ConfigSource::EnvBase, builder.build().unwrap()) + layer } pub fn default_config_layers() -> Vec { @@ -397,38 +397,34 @@ pub fn default_config_layers() -> Vec { /// Environment variables that override config values fn env_overrides_layer() -> ConfigLayer { - let mut builder = config::Config::builder(); + let mut layer = ConfigLayer::empty(ConfigSource::EnvOverrides); if let Ok(value) = env::var("JJ_USER") { - builder = builder.set_override("user.name", value).unwrap(); + layer.insert_value("user.name", value).unwrap(); } if let Ok(value) = env::var("JJ_EMAIL") { - builder = builder.set_override("user.email", value).unwrap(); + layer.insert_value("user.email", value).unwrap(); } if let Ok(value) = env::var("JJ_TIMESTAMP") { - builder = builder - .set_override("debug.commit-timestamp", value) - .unwrap(); + layer.insert_value("debug.commit-timestamp", value).unwrap(); } if let Ok(Ok(value)) = env::var("JJ_RANDOMNESS_SEED").map(|s| s.parse::()) { - builder = builder - .set_override("debug.randomness-seed", value) - .unwrap(); + layer.insert_value("debug.randomness-seed", value).unwrap(); } if let Ok(value) = env::var("JJ_OP_TIMESTAMP") { - builder = builder - .set_override("debug.operation-timestamp", value) + layer + .insert_value("debug.operation-timestamp", value) .unwrap(); } if let Ok(value) = env::var("JJ_OP_HOSTNAME") { - builder = builder.set_override("operation.hostname", value).unwrap(); + layer.insert_value("operation.hostname", value).unwrap(); } if let Ok(value) = env::var("JJ_OP_USERNAME") { - builder = builder.set_override("operation.username", value).unwrap(); + layer.insert_value("operation.username", value).unwrap(); } if let Ok(value) = env::var("JJ_EDITOR") { - builder = builder.set_override("ui.editor", value).unwrap(); + layer.insert_value("ui.editor", value).unwrap(); } - ConfigLayer::with_data(ConfigSource::EnvOverrides, builder.build().unwrap()) + layer } /// Parses `--config-toml` arguments. @@ -762,24 +758,20 @@ mod tests { #[test] fn test_resolved_config_values_single_key() { - let env_base_config = config::Config::builder() - .set_override("user.name", "base-user-name") - .unwrap() - .set_override("user.email", "base@user.email") - .unwrap() - .build() + let mut env_base_layer = ConfigLayer::empty(ConfigSource::EnvBase); + env_base_layer + .insert_value("user.name", "base-user-name") .unwrap(); - let repo_config = config::Config::builder() - .set_override("user.email", "repo@user.email") - .unwrap() - .build() + env_base_layer + .insert_value("user.email", "base@user.email") + .unwrap(); + let mut repo_layer = ConfigLayer::empty(ConfigSource::Repo); + repo_layer + .insert_value("user.email", "repo@user.email") .unwrap(); let mut config = StackedConfig::empty(); - config.add_layer(ConfigLayer::with_data( - ConfigSource::EnvBase, - env_base_config, - )); - config.add_layer(ConfigLayer::with_data(ConfigSource::Repo, repo_config)); + config.add_layer(env_base_layer); + config.add_layer(repo_layer); // Note: "email" is alphabetized, before "name" from same layer. insta::assert_debug_snapshot!( resolved_config_values(&config, &ConfigNamePathBuf::root()), @@ -906,21 +898,20 @@ mod tests { #[test] fn test_resolved_config_values_filter_path() { - let user_config = config::Config::builder() - .set_override("test-table1.foo", "user-FOO") - .unwrap() - .set_override("test-table2.bar", "user-BAR") - .unwrap() - .build() + let mut user_layer = ConfigLayer::empty(ConfigSource::User); + user_layer + .insert_value("test-table1.foo", "user-FOO") + .unwrap(); + user_layer + .insert_value("test-table2.bar", "user-BAR") .unwrap(); - let repo_config = config::Config::builder() - .set_override("test-table1.bar", "repo-BAR") - .unwrap() - .build() + let mut repo_layer = ConfigLayer::empty(ConfigSource::Repo); + repo_layer + .insert_value("test-table1.bar", "repo-BAR") .unwrap(); let mut config = StackedConfig::empty(); - config.add_layer(ConfigLayer::with_data(ConfigSource::User, user_config)); - config.add_layer(ConfigLayer::with_data(ConfigSource::Repo, repo_config)); + config.add_layer(user_layer); + config.add_layer(repo_layer); insta::assert_debug_snapshot!( resolved_config_values(&config, &ConfigNamePathBuf::from_iter(["test-table1"])), @r#" diff --git a/lib/src/config.rs b/lib/src/config.rs index 9fca4065c5..968e7c1db4 100644 --- a/lib/src/config.rs +++ b/lib/src/config.rs @@ -17,6 +17,7 @@ use std::borrow::Borrow; use std::convert::Infallible; use std::fmt; +use std::mem; use std::ops::Range; use std::path::Path; use std::path::PathBuf; @@ -60,6 +61,11 @@ pub enum ConfigGetError { }, } +/// Error that can occur when updating config variable. +#[derive(Debug, Error)] +#[error(transparent)] +pub struct ConfigUpdateError(pub ConfigError); + /// Extension methods for `Result`. pub trait ConfigGetResultExt { /// Converts `NotFound` error to `Ok(None)`, leaving other errors. @@ -232,6 +238,11 @@ pub struct ConfigLayer { } impl ConfigLayer { + /// Creates new layer with empty data. + pub fn empty(source: ConfigSource) -> Self { + Self::with_data(source, config::Config::default()) + } + /// Creates new layer with the configuration variables `data`. pub fn with_data(source: ConfigSource, data: config::Config) -> Self { ConfigLayer { @@ -314,6 +325,21 @@ impl ConfigLayer { ) -> Result, &ConfigValue> { look_up_item(&self.data.cache, name.into_name_path().borrow()) } + + /// Inserts new `value` at the `name` path. + pub fn insert_value( + &mut self, + name: &str, // TODO: impl ToConfigNamePath + value: impl Into, + ) -> Result<(), ConfigUpdateError> { + self.data = config::Config::builder() + .add_source(mem::take(&mut self.data)) + .set_override(name, value) + .map_err(ConfigUpdateError)? + .build() + .map_err(ConfigUpdateError)?; + Ok(()) + } } /// Looks up item from the `root_item`. Returns `Some(item)` if an item found at diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index 682351dbdb..4a5eabf016 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -38,14 +38,11 @@ use testutils::TestRepoBackend; fn config_with_commit_timestamp(timestamp: &str) -> StackedConfig { let mut config = testutils::base_user_config(); - config.add_layer(ConfigLayer::with_data( - ConfigSource::User, - config::Config::builder() - .set_override("debug.commit-timestamp", timestamp) - .unwrap() - .build() - .unwrap(), - )); + let mut layer = ConfigLayer::empty(ConfigSource::User); + layer + .insert_value("debug.commit-timestamp", timestamp) + .unwrap(); + config.add_layer(layer); config }