From 39063f18ff94dac107bf2e01d77b9c94c75a3a46 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 23 Nov 2024 18:25:00 +0900 Subject: [PATCH] config: make ConfigEnv public, replace global functions --- cli/src/cli_util.rs | 23 ++++++++++++++++------- cli/src/complete.rs | 4 +++- cli/src/config.rs | 38 +++++++++++++------------------------- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index a6407c4066..0aa3d7d6be 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -147,8 +147,8 @@ use crate::command_error::CommandError; use crate::commit_templater::CommitTemplateLanguage; use crate::commit_templater::CommitTemplateLanguageExtension; use crate::complete; -use crate::config::new_config_path; use crate::config::CommandNameAndArgs; +use crate::config::ConfigEnv; use crate::config::LayeredConfigs; use crate::diff_util; use crate::diff_util::DiffFormat; @@ -277,6 +277,7 @@ struct CommandHelperData { string_args: Vec, matches: ArgMatches, global_args: GlobalArgs, + config_env: ConfigEnv, settings: UserSettings, layered_configs: LayeredConfigs, revset_extensions: Arc, @@ -312,6 +313,10 @@ impl CommandHelper { &self.data.global_args } + pub fn config_env(&self) -> &ConfigEnv { + &self.data.config_env + } + pub fn settings(&self) -> &UserSettings { &self.data.settings } @@ -2765,9 +2770,11 @@ pub fn get_new_config_file_path( ) -> Result { let edit_path = match config_source { // TODO(#531): Special-case for editors that can't handle viewing directories? - ConfigSource::User => { - new_config_path()?.ok_or_else(|| user_error("No repo config path found to edit"))? - } + ConfigSource::User => command + .config_env() + .new_config_path()? + .ok_or_else(|| user_error("No repo config path found to edit"))? + .to_owned(), ConfigSource::Repo => command.workspace_loader()?.repo_path().join("config.toml"), _ => { return Err(user_error(format!( @@ -3546,6 +3553,7 @@ impl CliRunner { "Did you update to a commit where the directory doesn't exist?", ) })?; + let config_env = ConfigEnv::new()?; // Use cwd-relative workspace configs to resolve default command and // aliases. WorkspaceLoader::init() won't do any heavy lifting other // than the path resolution. @@ -3553,7 +3561,7 @@ impl CliRunner { .workspace_loader_factory .create(find_workspace_dir(&cwd)) .map_err(|err| map_workspace_load_error(err, None)); - layered_configs.read_user_config()?; + layered_configs.read_user_config(&config_env)?; let mut repo_config_path = None; if let Ok(loader) = &maybe_cwd_workspace_loader { layered_configs.read_repo_config(loader.repo_path())?; @@ -3561,8 +3569,8 @@ impl CliRunner { } let config = layered_configs.merge(); ui.reset(&config).map_err(|e| { - let user_config_path = layered_configs.user_config_path().unwrap_or(None); - let paths = [repo_config_path, user_config_path] + let user_config_path = config_env.existing_config_path(); + let paths = [repo_config_path.as_deref(), user_config_path] .into_iter() .flatten() .map(|path| format!("- {}", path.display())) @@ -3622,6 +3630,7 @@ impl CliRunner { string_args, matches, global_args: args.global_args, + config_env, settings, layered_configs, revset_extensions: self.revset_extensions.into(), diff --git a/cli/src/complete.rs b/cli/src/complete.rs index 56d7e87231..fc18939c9d 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -27,6 +27,7 @@ use crate::cli_util::GlobalArgs; use crate::command_error::user_error; use crate::command_error::CommandError; use crate::config::default_config; +use crate::config::ConfigEnv; use crate::config::LayeredConfigs; use crate::config::CONFIG_SCHEMA; use crate::ui::Ui; @@ -401,8 +402,9 @@ fn get_jj_command() -> Result<(std::process::Command, Config), CommandError> { let cwd = std::env::current_dir() .and_then(|cwd| cwd.canonicalize()) .map_err(user_error)?; + let config_env = ConfigEnv::new()?; let maybe_cwd_workspace_loader = DefaultWorkspaceLoaderFactory.create(find_workspace_dir(&cwd)); - let _ = layered_configs.read_user_config(); + let _ = layered_configs.read_user_config(&config_env); if let Ok(loader) = &maybe_cwd_workspace_loader { let _ = layered_configs.read_repo_config(loader.repo_path()); } diff --git a/cli/src/config.rs b/cli/src/config.rs index 880fba7cc8..9e9cdb0e0e 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -130,9 +130,9 @@ impl LayeredConfigs { } #[instrument] - pub fn read_user_config(&mut self) -> Result<(), ConfigEnvError> { + pub fn read_user_config(&mut self, env: &ConfigEnv) -> Result<(), ConfigError> { self.inner.remove_layers(ConfigSource::User); - if let Some(path) = existing_config_path()? { + if let Some(path) = env.existing_config_path() { if path.is_dir() { self.inner.load_dir(ConfigSource::User, path)?; } else { @@ -142,10 +142,6 @@ impl LayeredConfigs { Ok(()) } - pub fn user_config_path(&self) -> Result, ConfigEnvError> { - existing_config_path() - } - #[instrument] pub fn read_repo_config(&mut self, repo_path: &Path) -> Result<(), ConfigEnvError> { self.inner.remove_layers(ConfigSource::Repo); @@ -316,13 +312,13 @@ impl UnresolvedConfigEnv { } #[derive(Clone, Debug)] -struct ConfigEnv { +pub struct ConfigEnv { user_config_path: ConfigPath, } impl ConfigEnv { /// Initializes configuration loader based on environment variables. - fn new() -> Result { + pub fn new() -> Result { let env = UnresolvedConfigEnv { config_dir: dirs::config_dir(), home_dir: dirs::home_dir(), @@ -333,17 +329,24 @@ impl ConfigEnv { }) } - fn existing_config_path(&self) -> Option<&Path> { + /// Returns a path to the existing user-specific config file or directory. + pub fn existing_config_path(&self) -> Option<&Path> { match &self.user_config_path { ConfigPath::Existing(path) => Some(path), _ => None, } } - fn new_config_path(&self) -> Result, ConfigEnvError> { + /// Returns a path to the user-specific config file. + /// + /// If no config file is found, tries to guess a reasonable new location for + /// it. If a path to a new config file is returned, the parent directory may + /// be created as a result of this call. + pub fn new_config_path(&self) -> Result, ConfigEnvError> { match &self.user_config_path { ConfigPath::Existing(path) => Ok(Some(path)), ConfigPath::New(path) => { + // TODO: should we update self.user_config_path to Existing? create_config_file(path)?; Ok(Some(path)) } @@ -352,21 +355,6 @@ impl ConfigEnv { } } -pub fn existing_config_path() -> Result, ConfigEnvError> { - Ok(ConfigEnv::new()? - .existing_config_path() - .map(ToOwned::to_owned)) -} - -/// Returns a path to the user-specific config file. -/// -/// If no config file is found, tries to guess a reasonable new -/// location for it. If a path to a new config file is returned, the -/// parent directory may be created as a result of this call. -pub fn new_config_path() -> Result, ConfigEnvError> { - Ok(ConfigEnv::new()?.new_config_path()?.map(ToOwned::to_owned)) -} - /// Environment variables that should be overridden by config values fn env_base() -> config::Config { let mut builder = config::Config::builder();