config: make ConfigEnv public, replace global functions

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<String>,
     matches: ArgMatches,
     global_args: GlobalArgs,
+    config_env: ConfigEnv,
     settings: UserSettings,
     layered_configs: LayeredConfigs,
     revset_extensions: Arc<RevsetExtensions>,
@@ -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<PathBuf, CommandError> {
     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(),

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());
     }

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<Option<PathBuf>, 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<Self, ConfigEnvError> {
+    pub fn new() -> Result<Self, ConfigEnvError> {
         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<Option<&Path>, 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<Option<&Path>, 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<Option<PathBuf>, 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<Option<PathBuf>, 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();