cli: fix "jj config path" not create new file at default path

This patch adds simpler user/repo_config_path() accessors. existing_*_path()
are kinda implementation details (for testing), so they are now private methods.
new_user_config_path() will be removed later.

CHANGELOG.md

@@ -51,6 +51,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * Fixed performance of progress bar rendering when fetching from Git remote.
   [#5057](https://github.com/martinvonz/jj/issues/5057)
 
+* `jj config path --user` no longer creates new file at the default config path.
+
 ## [0.24.0] - 2024-12-04
 
 ### Release highlights

cli/src/commands/config/mod.rs

@@ -78,13 +78,27 @@ impl ConfigLevelArgs {
                 .ok_or_else(|| user_error("No user config path found to edit"))
         } else if self.repo {
             config_env
-                .new_repo_config_path()
+                .repo_config_path()
                 .ok_or_else(|| user_error("No repo config path found to edit"))
         } else {
             panic!("No config_level provided")
         }
     }
 
+    fn config_path<'a>(&self, config_env: &'a ConfigEnv) -> Result<&'a Path, CommandError> {
+        if self.user {
+            config_env
+                .user_config_path()
+                .ok_or_else(|| user_error("No user config path found"))
+        } else if self.repo {
+            config_env
+                .repo_config_path()
+                .ok_or_else(|| user_error("No repo config path found"))
+        } else {
+            panic!("No config_level provided")
+        }
+    }
+
     fn edit_config_file(&self, config_env: &ConfigEnv) -> Result<ConfigFile, CommandError> {
         let path = self.new_config_file_path(config_env)?;
         if path.is_dir() {

cli/src/commands/config/path.rs

@@ -39,7 +39,7 @@ pub fn cmd_config_path(
     command: &CommandHelper,
     args: &ConfigPathArgs,
 ) -> Result<(), CommandError> {
-    let config_path = args.level.new_config_file_path(command.config_env())?;
+    let config_path = args.level.config_path(command.config_env())?;
     writeln!(
         ui.stdout(),
         "{}",

cli/src/config.rs

@@ -141,6 +141,13 @@ impl ConfigPath {
             None => ConfigPath::Unavailable,
         }
     }
+
+    fn as_path(&self) -> Option<&Path> {
+        match self {
+            ConfigPath::Existing(path) | ConfigPath::New(path) => Some(path),
+            ConfigPath::Unavailable => None,
+        }
+    }
 }
 
 /// Like std::fs::create_dir_all but creates new directories to be accessible to
@@ -225,8 +232,13 @@ impl ConfigEnv {
         })
     }
 
+    /// Returns a path to the user-specific config file or directory.
+    pub fn user_config_path(&self) -> Option<&Path> {
+        self.user_config_path.as_path()
+    }
+
     /// Returns a path to the existing user-specific config file or directory.
-    pub fn existing_user_config_path(&self) -> Option<&Path> {
+    fn existing_user_config_path(&self) -> Option<&Path> {
         match &self.user_config_path {
             ConfigPath::Existing(path) => Some(path),
             _ => None,
@@ -243,8 +255,7 @@ impl ConfigEnv {
             ConfigPath::Existing(path) => Ok(Some(path)),
             ConfigPath::New(path) => {
                 // TODO: Maybe we shouldn't create new file here. Not all
-                // callers need an empty file. For example, "jj config path"
-                // should be a readonly operation. "jj config set" doesn't have
+                // callers need an empty file. "jj config set" doesn't have
                 // to create an empty file to be overwritten. Since it's unclear
                 // who and when to update ConfigPath::New(_) to ::Existing(_),
                 // it's probably better to not cache the path existence.
@@ -276,20 +287,16 @@ impl ConfigEnv {
         self.repo_config_path = ConfigPath::new(Some(path.join("config.toml")));
     }
 
-    /// Returns a path to the existing repo-specific config file.
-    pub fn existing_repo_config_path(&self) -> Option<&Path> {
-        match &self.repo_config_path {
-            ConfigPath::Existing(path) => Some(path),
-            _ => None,
-        }
+    /// Returns a path to the repo-specific config file.
+    pub fn repo_config_path(&self) -> Option<&Path> {
+        self.repo_config_path.as_path()
     }
 
-    /// Returns a path to the repo-specific config file.
-    pub fn new_repo_config_path(&self) -> Option<&Path> {
+    /// Returns a path to the existing repo-specific config file.
+    fn existing_repo_config_path(&self) -> Option<&Path> {
         match &self.repo_config_path {
             ConfigPath::Existing(path) => Some(path),
-            ConfigPath::New(path) => Some(path),
-            ConfigPath::Unavailable => None,
+            _ => None,
         }
     }
 

cli/tests/test_config_command.rs

@@ -15,7 +15,6 @@
 use std::path::PathBuf;
 
 use indoc::indoc;
-use insta::assert_snapshot;
 use itertools::Itertools;
 use regex::Regex;
 
@@ -793,22 +792,33 @@ fn test_config_edit_repo() {
 
 #[test]
 fn test_config_path() {
-    let test_env = TestEnvironment::default();
+    let mut test_env = TestEnvironment::default();
     test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
     let repo_path = test_env.env_root().join("repo");
 
-    assert_snapshot!(
-      test_env.jj_cmd_success(&repo_path, &["config", "path", "--user"]),
-      @r###"
-      $TEST_ENV/config
-      "###
+    let user_config_path = test_env.env_root().join("config.toml");
+    let repo_config_path = repo_path.join(PathBuf::from_iter([".jj", "repo", "config.toml"]));
+    test_env.set_config_path(user_config_path.clone());
+
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["config", "path", "--user"]),
+        @"$TEST_ENV/config.toml");
+    assert!(
+        !user_config_path.exists(),
+        "jj config path shouldn't create new file"
     );
-    assert_snapshot!(
-      test_env.jj_cmd_success(&repo_path, &["config", "path", "--repo"]),
-      @r###"
-      $TEST_ENV/repo/.jj/repo/config.toml
-      "###
+
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["config", "path", "--repo"]),
+        @"$TEST_ENV/repo/.jj/repo/config.toml");
+    assert!(
+        !repo_config_path.exists(),
+        "jj config path shouldn't create new file"
     );
+
+    insta::assert_snapshot!(
+        test_env.jj_cmd_failure(test_env.env_root(), &["config", "path", "--repo"]),
+        @"Error: No repo config path found");
 }
 
 #[test]