diff --git a/cli/src/commands/config/mod.rs b/cli/src/commands/config/mod.rs index 6f85f532b..be1476ccd 100644 --- a/cli/src/commands/config/mod.rs +++ b/cli/src/commands/config/mod.rs @@ -21,6 +21,7 @@ mod unset; use std::path::Path; +use jj_lib::config::ConfigFile; use jj_lib::config::ConfigSource; use tracing::instrument; @@ -83,6 +84,18 @@ impl ConfigLevelArgs { panic!("No config_level provided") } } + + fn edit_config_file(&self, config_env: &ConfigEnv) -> Result { + let path = self.new_config_file_path(config_env)?; + if path.is_dir() { + return Err(user_error(format!( + "Can't set config in path {path} (dirs not supported)", + path = path.display() + ))); + } + let source = self.get_source_kind().unwrap(); + Ok(ConfigFile::load_or_empty(source, path)?) + } } /// Manage config options diff --git a/cli/src/commands/config/set.rs b/cli/src/commands/config/set.rs index 2ddf032f0..3759c30b4 100644 --- a/cli/src/commands/config/set.rs +++ b/cli/src/commands/config/set.rs @@ -23,11 +23,10 @@ use tracing::instrument; use super::ConfigLevelArgs; use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; -use crate::command_error::user_error; +use crate::command_error::user_error_with_message; use crate::command_error::CommandError; use crate::complete; use crate::config::parse_toml_value_or_bare_string; -use crate::config::write_config_value_to_file; use crate::ui::Ui; /// Update config file to set the given option to a given value. @@ -53,13 +52,7 @@ pub fn cmd_config_set( command: &CommandHelper, args: &ConfigSetArgs, ) -> Result<(), CommandError> { - let config_path = args.level.new_config_file_path(command.config_env())?; - if config_path.is_dir() { - return Err(user_error(format!( - "Can't set config in path {path} (dirs not supported)", - path = config_path.display() - ))); - } + let mut file = args.level.edit_config_file(command.config_env())?; // TODO(#531): Infer types based on schema (w/ --type arg to override). let value = parse_toml_value_or_bare_string(&args.value); @@ -72,7 +65,10 @@ pub fn cmd_config_set( check_wc_author(ui, command, &value, AuthorChange::Email)?; }; - write_config_value_to_file(&args.name, value, config_path) + file.set_value(&args.name, value) + .map_err(|err| user_error_with_message(format!("Failed to set {}", args.name), err))?; + file.save()?; + Ok(()) } /// Returns the commit of the working copy if it exists. diff --git a/cli/src/commands/config/unset.rs b/cli/src/commands/config/unset.rs index 6f96028df..290fdec14 100644 --- a/cli/src/commands/config/unset.rs +++ b/cli/src/commands/config/unset.rs @@ -19,9 +19,9 @@ use tracing::instrument; use super::ConfigLevelArgs; use crate::cli_util::CommandHelper; use crate::command_error::user_error; +use crate::command_error::user_error_with_message; use crate::command_error::CommandError; use crate::complete; -use crate::config::remove_config_value_from_file; use crate::ui::Ui; /// Update config file to unset the given option. @@ -39,13 +39,13 @@ pub fn cmd_config_unset( command: &CommandHelper, args: &ConfigUnsetArgs, ) -> Result<(), CommandError> { - let config_path = args.level.new_config_file_path(command.config_env())?; - if config_path.is_dir() { - return Err(user_error(format!( - "Can't set config in path {path} (dirs not supported)", - path = config_path.display() - ))); + let mut file = args.level.edit_config_file(command.config_env())?; + let old_value = file + .delete_value(&args.name) + .map_err(|err| user_error_with_message(format!("Failed to unset {}", args.name), err))?; + if old_value.is_none() { + return Err(user_error(format!(r#""{}" doesn't exist"#, args.name))); } - - remove_config_value_from_file(&args.name, config_path) + file.save()?; + Ok(()) } diff --git a/cli/src/commands/git/mod.rs b/cli/src/commands/git/mod.rs index e874cd567..a434d0c6c 100644 --- a/cli/src/commands/git/mod.rs +++ b/cli/src/commands/git/mod.rs @@ -24,7 +24,8 @@ pub mod submodule; use std::path::Path; use clap::Subcommand; -use jj_lib::config::ConfigNamePathBuf; +use jj_lib::config::ConfigFile; +use jj_lib::config::ConfigSource; use self::clone::cmd_git_clone; use self::clone::GitCloneArgs; @@ -46,7 +47,6 @@ use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; use crate::command_error::user_error_with_message; use crate::command_error::CommandError; -use crate::config::write_config_value_to_file; use crate::ui::Ui; /// Commands for working with Git remotes and the underlying Git repo @@ -114,12 +114,10 @@ fn write_repository_level_trunk_alias( remote: &str, branch: &str, ) -> Result<(), CommandError> { - let config_path = repo_path.join("config.toml"); - write_config_value_to_file( - &ConfigNamePathBuf::from_iter(["revset-aliases", "trunk()"]), - format!("{branch}@{remote}").into(), - &config_path, - )?; + let mut file = ConfigFile::load_or_empty(ConfigSource::Repo, repo_path.join("config.toml"))?; + file.set_value(["revset-aliases", "trunk()"], format!("{branch}@{remote}")) + .expect("initial repo config shouldn't have invalid values"); + file.save()?; writeln!( ui.status(), r#"Setting the revset alias "trunk()" to "{branch}@{remote}""#, diff --git a/cli/src/config.rs b/cli/src/config.rs index df32a992d..b6d43640e 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -22,7 +22,6 @@ 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; @@ -34,10 +33,6 @@ use regex::Regex; use thiserror::Error; use tracing::instrument; -use crate::command_error::user_error; -use crate::command_error::user_error_with_message; -use crate::command_error::CommandError; - // TODO(#879): Consider generating entire schema dynamically vs. static file. pub const CONFIG_SCHEMA: &str = include_str!("config-schema.json"); @@ -427,35 +422,6 @@ pub fn parse_config_args(toml_strs: &[ConfigArg]) -> Result, Co .try_collect() } -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 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))?; - file.save()?; - Ok(()) -} - -pub fn remove_config_value_from_file( - key: &ConfigNamePathBuf, - path: &Path, -) -> Result<(), CommandError> { - // TODO: Load config layer by caller. Here we use a dummy source for now. - 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"#))); - } - file.save()?; - Ok(()) -} - /// Command name and arguments specified by config. #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize)] #[serde(untagged)]