From 13206c4a1853b2a389cfae2f9a08de030b4a5fac Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 23 Nov 2024 18:53:56 +0900 Subject: [PATCH] config: rename ConfigEnv methods to clarify the intent We'll add existing/new_repo_config_path() methods. --- cli/src/cli_util.rs | 6 +++--- cli/src/complete.rs | 2 +- cli/src/config.rs | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 0aa3d7d6be..83697538c5 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -2772,7 +2772,7 @@ pub fn get_new_config_file_path( // TODO(#531): Special-case for editors that can't handle viewing directories? ConfigSource::User => command .config_env() - .new_config_path()? + .new_user_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"), @@ -3553,7 +3553,7 @@ impl CliRunner { "Did you update to a commit where the directory doesn't exist?", ) })?; - let config_env = ConfigEnv::new()?; + let config_env = ConfigEnv::from_environment()?; // Use cwd-relative workspace configs to resolve default command and // aliases. WorkspaceLoader::init() won't do any heavy lifting other // than the path resolution. @@ -3569,7 +3569,7 @@ impl CliRunner { } let config = layered_configs.merge(); ui.reset(&config).map_err(|e| { - let user_config_path = config_env.existing_config_path(); + let user_config_path = config_env.existing_user_config_path(); let paths = [repo_config_path.as_deref(), user_config_path] .into_iter() .flatten() diff --git a/cli/src/complete.rs b/cli/src/complete.rs index fc18939c9d..b9cb9de4fd 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -402,7 +402,7 @@ 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 config_env = ConfigEnv::from_environment()?; let maybe_cwd_workspace_loader = DefaultWorkspaceLoaderFactory.create(find_workspace_dir(&cwd)); let _ = layered_configs.read_user_config(&config_env); if let Ok(loader) = &maybe_cwd_workspace_loader { diff --git a/cli/src/config.rs b/cli/src/config.rs index 9e9cdb0e0e..1fd26fb876 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -132,7 +132,7 @@ impl LayeredConfigs { #[instrument] pub fn read_user_config(&mut self, env: &ConfigEnv) -> Result<(), ConfigError> { self.inner.remove_layers(ConfigSource::User); - if let Some(path) = env.existing_config_path() { + if let Some(path) = env.existing_user_config_path() { if path.is_dir() { self.inner.load_dir(ConfigSource::User, path)?; } else { @@ -318,7 +318,7 @@ pub struct ConfigEnv { impl ConfigEnv { /// Initializes configuration loader based on environment variables. - pub fn new() -> Result { + pub fn from_environment() -> Result { let env = UnresolvedConfigEnv { config_dir: dirs::config_dir(), home_dir: dirs::home_dir(), @@ -330,7 +330,7 @@ impl ConfigEnv { } /// Returns a path to the existing user-specific config file or directory. - pub fn existing_config_path(&self) -> Option<&Path> { + pub fn existing_user_config_path(&self) -> Option<&Path> { match &self.user_config_path { ConfigPath::Existing(path) => Some(path), _ => None, @@ -342,7 +342,7 @@ impl ConfigEnv { /// 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> { + pub fn new_user_config_path(&self) -> Result, ConfigEnvError> { match &self.user_config_path { ConfigPath::Existing(path) => Ok(Some(path)), ConfigPath::New(path) => { @@ -1205,7 +1205,7 @@ mod tests { let env = self .resolve(tmp.path()) .map_err(|e| anyhow!("existing_config_path: {e}"))?; - let got = env.existing_config_path(); + let got = env.existing_user_config_path(); if got != want.as_deref() { return Err(anyhow!("existing_config_path: got {got:?}, want {want:?}")); } @@ -1223,7 +1223,7 @@ mod tests { .resolve(tmp.path()) .map_err(|e| anyhow!("new_config_path: {e}"))?; let got = env - .new_config_path() + .new_user_config_path() .map_err(|e| anyhow!("new_config_path: {e}"))?; if got != want.as_deref() { return Err(anyhow!("new_config_path: got {got:?}, want {want:?}"));