diff --git a/cli/src/config.rs b/cli/src/config.rs index cf889d2ec4..b4b6a29ac7 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").unwrap_or_default().is_empty() { // "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.set_value("ui.color", "never").unwrap(); } if let Ok(value) = env::var("PAGER") { - builder = builder.set_override("ui.pager", value).unwrap(); + layer.set_value("ui.pager", value).unwrap(); } if let Ok(value) = env::var("VISUAL") { - builder = builder.set_override("ui.editor", value).unwrap(); + layer.set_value("ui.editor", value).unwrap(); } else if let Ok(value) = env::var("EDITOR") { - builder = builder.set_override("ui.editor", value).unwrap(); + layer.set_value("ui.editor", value).unwrap(); } - ConfigLayer::with_data(ConfigSource::EnvBase, builder.build().unwrap()) + layer } pub fn default_config_layers() -> Vec { @@ -397,38 +397,32 @@ 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.set_value("user.name", value).unwrap(); } if let Ok(value) = env::var("JJ_EMAIL") { - builder = builder.set_override("user.email", value).unwrap(); + layer.set_value("user.email", value).unwrap(); } if let Ok(value) = env::var("JJ_TIMESTAMP") { - builder = builder - .set_override("debug.commit-timestamp", value) - .unwrap(); + layer.set_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.set_value("debug.randomness-seed", value).unwrap(); } if let Ok(value) = env::var("JJ_OP_TIMESTAMP") { - builder = builder - .set_override("debug.operation-timestamp", value) - .unwrap(); + layer.set_value("debug.operation-timestamp", value).unwrap(); } if let Ok(value) = env::var("JJ_OP_HOSTNAME") { - builder = builder.set_override("operation.hostname", value).unwrap(); + layer.set_value("operation.hostname", value).unwrap(); } if let Ok(value) = env::var("JJ_OP_USERNAME") { - builder = builder.set_override("operation.username", value).unwrap(); + layer.set_value("operation.username", value).unwrap(); } if let Ok(value) = env::var("JJ_EDITOR") { - builder = builder.set_override("ui.editor", value).unwrap(); + layer.set_value("ui.editor", value).unwrap(); } - ConfigLayer::with_data(ConfigSource::EnvOverrides, builder.build().unwrap()) + layer } /// Parses `--config-toml` arguments. @@ -762,24 +756,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 + .set_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 + .set_value("user.email", "base@user.email") + .unwrap(); + let mut repo_layer = ConfigLayer::empty(ConfigSource::Repo); + repo_layer + .set_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 +896,14 @@ 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() - .unwrap(); - let repo_config = config::Config::builder() - .set_override("test-table1.bar", "repo-BAR") - .unwrap() - .build() - .unwrap(); + let mut user_layer = ConfigLayer::empty(ConfigSource::User); + user_layer.set_value("test-table1.foo", "user-FOO").unwrap(); + user_layer.set_value("test-table2.bar", "user-BAR").unwrap(); + let mut repo_layer = ConfigLayer::empty(ConfigSource::Repo); + repo_layer.set_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..f781ec63f9 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()) } + + /// Sets new `value` to the `name` path. + pub fn set_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..d134feacbd 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 + .set_value("debug.commit-timestamp", timestamp) + .unwrap(); + config.add_layer(layer); config }