cli: do not look up workspace path more than once, reuse WorkspaceLoader

src/cli_util.rs

@@ -265,6 +265,7 @@ pub struct CommandHelper {
     string_args: Vec<String>,
     global_args: GlobalArgs,
     settings: UserSettings,
+    maybe_workspace_loader: Result<WorkspaceLoader, CommandError>,
     store_factories: StoreFactories,
 }
 
@@ -275,6 +276,7 @@ impl CommandHelper {
         string_args: Vec<String>,
         global_args: GlobalArgs,
         settings: UserSettings,
+        maybe_workspace_loader: Result<WorkspaceLoader, CommandError>,
         store_factories: StoreFactories,
     ) -> Self {
         Self {
@@ -283,6 +285,7 @@ impl CommandHelper {
             string_args,
             global_args,
             settings,
+            maybe_workspace_loader,
             store_factories,
         }
     }
@@ -315,30 +318,10 @@ impl CommandHelper {
     }
 
     pub fn load_workspace(&self) -> Result<Workspace, CommandError> {
-        let wc_path_str = self.global_args.repository.as_deref().unwrap_or(".");
+        let loader = self.maybe_workspace_loader.as_ref().map_err(Clone::clone)?;
-        let wc_path = self.cwd.join(wc_path_str);
+        loader
-        Workspace::load(&self.settings, &wc_path, &self.store_factories).map_err(|err| match err {
+            .load(&self.settings, &self.store_factories)
-            WorkspaceLoadError::NoWorkspaceHere(wc_path) => {
+            .map_err(|e| user_error(format!("{}: {}", e, e.error)))
-                let message = format!("There is no jj repo in \"{wc_path_str}\"");
-                let git_dir = wc_path.join(".git");
-                if git_dir.is_dir() {
-                    user_error_with_hint(
-                        message,
-                        "It looks like this is a git repo. You can create a jj repo backed by it \
-                         by running this:
-jj init --git-repo=.",
-                    )
-                } else {
-                    user_error(message)
-                }
-            }
-            WorkspaceLoadError::RepoDoesNotExist(repo_dir) => user_error(format!(
-                "The repository directory at {} is missing. Was it moved?",
-                repo_dir.to_str().unwrap()
-            )),
-            WorkspaceLoadError::Path(e) => user_error(format!("{}: {}", e, e.error)),
-            WorkspaceLoadError::NonUnicodePath => user_error(err.to_string()),
-        })
     }
 
     pub fn resolve_operation(
@@ -990,6 +973,37 @@ impl WorkspaceCommandHelper {
     }
 }
 
+fn init_workspace_loader(
+    cwd: &Path,
+    global_args: &GlobalArgs,
+) -> Result<WorkspaceLoader, CommandError> {
+    let workspace_path_str = global_args.repository.as_deref().unwrap_or(".");
+    let workspace_path = cwd.join(workspace_path_str);
+    WorkspaceLoader::init(&workspace_path).map_err(|err| match err {
+        WorkspaceLoadError::NoWorkspaceHere(wc_path) => {
+            // Prefer user-specified workspace_path_str instead of absolute wc_path.
+            let message = format!(r#"There is no jj repo in "{workspace_path_str}""#);
+            let git_dir = wc_path.join(".git");
+            if git_dir.is_dir() {
+                user_error_with_hint(
+                    message,
+                    "It looks like this is a git repo. You can create a jj repo backed by it by \
+                     running this:
+jj init --git-repo=.",
+                )
+            } else {
+                user_error(message)
+            }
+        }
+        WorkspaceLoadError::RepoDoesNotExist(repo_dir) => user_error(format!(
+            "The repository directory at {} is missing. Was it moved?",
+            repo_dir.to_str().unwrap()
+        )),
+        WorkspaceLoadError::Path(e) => user_error(format!("{}: {}", e, e.error)),
+        WorkspaceLoadError::NonUnicodePath => user_error(err.to_string()),
+    })
+}
+
 #[derive(Debug, Error)]
 pub enum StaleWorkingCopyError {
     #[error("The working copy is behind the latest operation")]
@@ -1791,10 +1805,8 @@ impl CliRunner {
             &mut layered_configs,
         )?;
 
-        let workspace_path = cwd.join(args.global_args.repository.as_deref().unwrap_or("."));
+        let maybe_workspace_loader = init_workspace_loader(&cwd, &args.global_args);
-        // TODO: reuse loader to load workspace, but we can't report error here, and the
+        if let Ok(loader) = &maybe_workspace_loader {
-        // error isn't clonable.
-        if let Ok(loader) = WorkspaceLoader::init(&workspace_path) {
             // TODO: maybe show error/warning if repo config contained command alias
             layered_configs.read_repo_config(loader.repo_path())?;
         }
@@ -1807,6 +1819,7 @@ impl CliRunner {
             string_args,
             args.global_args,
             settings,
+            maybe_workspace_loader,
             self.store_factories.unwrap_or_default(),
         );
         (self.dispatch_fn)(ui, &command_helper, &matches)

tests/test_global_opts.rs

@@ -110,6 +110,31 @@ fn test_repo_arg_with_git_clone() {
     "###);
 }
 
+#[test]
+fn test_no_workspace_directory() {
+    let test_env = TestEnvironment::default();
+    let repo_path = test_env.env_root().join("repo");
+    std::fs::create_dir(&repo_path).unwrap();
+
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["status"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: There is no jj repo in "."
+    "###);
+
+    let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["status", "-R", "repo"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: There is no jj repo in "repo"
+    "###);
+
+    std::fs::create_dir(repo_path.join(".git")).unwrap();
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["status"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: There is no jj repo in "."
+    Hint: It looks like this is a git repo. You can create a jj repo backed by it by running this:
+    jj init --git-repo=.
+    "###);
+}
+
 #[test]
 fn test_color_config() {
     let mut test_env = TestEnvironment::default();