From d6ca0c9940965adb73f4228127f908a30470d0f3 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 12 Dec 2024 23:07:46 +0900 Subject: [PATCH] config: add convenient ConfigLayer wrapper that provides .save() method I'm going to remove write/remove_config_value_to/from_file() functions, but I don't want to copy layer.path.expect(..) to all callers. --- cli/src/command_error.rs | 7 ++++ cli/src/config.rs | 41 +++++------------------ lib/src/config.rs | 71 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 32 deletions(-) diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 48e017891..89c4ec58b 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -22,6 +22,7 @@ use std::sync::Arc; use itertools::Itertools as _; use jj_lib::backend::BackendError; +use jj_lib::config::ConfigFileSaveError; use jj_lib::config::ConfigGetError; use jj_lib::config::ConfigLoadError; use jj_lib::dsl_util::Diagnostics; @@ -246,6 +247,12 @@ impl From for CommandError { } } +impl From for CommandError { + fn from(err: ConfigFileSaveError) -> Self { + user_error(err) + } +} + impl From for CommandError { fn from(err: ConfigGetError) -> Self { let hint = match &err { diff --git a/cli/src/config.rs b/cli/src/config.rs index aba8b2793..df32a992d 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -22,6 +22,7 @@ use std::path::PathBuf; use std::process::Command; use itertools::Itertools; +use jj_lib::config::ConfigFile; use jj_lib::config::ConfigLayer; use jj_lib::config::ConfigLoadError; use jj_lib::config::ConfigNamePathBuf; @@ -426,42 +427,17 @@ pub fn parse_config_args(toml_strs: &[ConfigArg]) -> Result, Co .try_collect() } -fn load_config_file_or_empty( - source: ConfigSource, - path: &Path, -) -> Result { - match ConfigLayer::load_from_file(source, path.into()) { - Ok(layer) => Ok(layer), - Err(ConfigLoadError::Read(err)) if err.error.kind() == std::io::ErrorKind::NotFound => { - // If config doesn't exist yet, read as empty and we'll write one. - let mut layer = ConfigLayer::empty(source); - layer.path = Some(path.into()); - Ok(layer) - } - Err(err) => Err(err), - } -} - -fn write_config(path: &Path, doc: &toml_edit::DocumentMut) -> Result<(), CommandError> { - 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, path: &Path, ) -> Result<(), CommandError> { // TODO: Load config layer by caller. Here we use a dummy source for now. - let mut layer = load_config_file_or_empty(ConfigSource::User, path)?; - layer - .set_value(key, value) + let mut file = ConfigFile::load_or_empty(ConfigSource::User, path)?; + file.set_value(key, value) .map_err(|err| user_error_with_message(format!("Failed to set {key}"), err))?; - write_config(path, &layer.data) + file.save()?; + Ok(()) } pub fn remove_config_value_from_file( @@ -469,14 +445,15 @@ pub fn remove_config_value_from_file( path: &Path, ) -> Result<(), CommandError> { // TODO: Load config layer by caller. Here we use a dummy source for now. - let mut layer = load_config_file_or_empty(ConfigSource::User, path)?; - let old_value = layer + let mut file = ConfigFile::load_or_empty(ConfigSource::User, path)?; + let old_value = file .delete_value(key) .map_err(|err| user_error_with_message(format!("Failed to unset {key}"), err))?; if old_value.is_none() { return Err(user_error(format!(r#""{key}" doesn't exist"#))); } - write_config(path, &layer.data) + file.save()?; + Ok(()) } /// Command name and arguments specified by config. diff --git a/lib/src/config.rs b/lib/src/config.rs index c1beb4fdc..0f89aff68 100644 --- a/lib/src/config.rs +++ b/lib/src/config.rs @@ -18,6 +18,7 @@ use std::borrow::Borrow; use std::convert::Infallible; use std::fmt; use std::fs; +use std::io; use std::ops::Range; use std::path::Path; use std::path::PathBuf; @@ -58,6 +59,11 @@ pub enum ConfigLoadError { }, } +/// Error that can occur when saving config variables to file. +#[derive(Debug, Error)] +#[error("Failed to write configuration file")] +pub struct ConfigFileSaveError(#[source] pub PathError); + /// Error that can occur when looking up config variable. #[derive(Debug, Error)] pub enum ConfigGetError { @@ -467,6 +473,71 @@ fn ensure_parent_table<'a, 'b>( Ok((parent_table, leaf_key)) } +/// Wrapper for file-based [`ConfigLayer`], providing convenient methods for +/// modification. +#[derive(Debug)] +pub struct ConfigFile { + layer: ConfigLayer, +} + +impl ConfigFile { + /// Loads TOML file from the specified `path` if exists. Returns an empty + /// object if the file doesn't exist. + pub fn load_or_empty( + source: ConfigSource, + path: impl Into, + ) -> Result { + let layer = match ConfigLayer::load_from_file(source, path.into()) { + Ok(layer) => layer, + Err(ConfigLoadError::Read(PathError { path, error })) + if error.kind() == io::ErrorKind::NotFound => + { + ConfigLayer { + source, + path: Some(path), + data: DocumentMut::new(), + } + } + Err(err) => return Err(err), + }; + Ok(ConfigFile { layer }) + } + + /// Writes serialized data to the source file. + pub fn save(&self) -> Result<(), ConfigFileSaveError> { + fs::write(self.path(), self.layer.data.to_string()) + .context(self.path()) + .map_err(ConfigFileSaveError) + } + + /// Source file path. + pub fn path(&self) -> &Path { + self.layer.path.as_ref().expect("path must be known") + } + + /// Returns the underlying config layer. + pub fn layer(&self) -> &ConfigLayer { + &self.layer + } + + /// See [`ConfigLayer::set_value()`]. + pub fn set_value( + &mut self, + name: impl ToConfigNamePath, + new_value: impl Into, + ) -> Result, ConfigUpdateError> { + self.layer.set_value(name, new_value) + } + + /// See [`ConfigLayer::delete_value()`]. + pub fn delete_value( + &mut self, + name: impl ToConfigNamePath, + ) -> Result, ConfigUpdateError> { + self.layer.delete_value(name) + } +} + /// Stack of configuration layers which can be merged as needed. /// /// A [`StackedConfig`] is something like a read-only `overlayfs`. Tables and