repo_path: migrate parse_file_path() from ui

It seems a bit invasive that RepoPath constructor processes an environment
like cwd, but we need an unmodified input string to build a readable error.
The error could be rewrapped at cli boundary, but I don't think it would
worth inserting indirection just for that.

I made s/file_path/fs_path/ change because there's already to_fs_path()
function, and "file path" in RepoPath context may be ambiguous.

lib/src/repo_path.rs

@@ -13,10 +13,12 @@
 // limitations under the License.
 
 use std::fmt::{Debug, Error, Formatter};
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 
 use itertools::Itertools;
 
+use crate::file_util;
+
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
 pub struct RepoPathComponent {
     value: String,
@@ -76,6 +78,27 @@ impl RepoPath {
         RepoPath { components }
     }
 
+    /// Parses an `input` path into a `RepoPath` relative to `base`.
+    ///
+    /// The `cwd` and `base` paths are supposed to be absolute and normalized in
+    /// the same manner. The `input` path may be either relative to `cwd` or
+    /// absolute.
+    pub fn parse_fs_path(cwd: &Path, base: &Path, input: &str) -> Result<Self, FsPathParseError> {
+        let abs_input_path = file_util::normalize_path(&cwd.join(input));
+        let repo_relative_path = file_util::relative_path(base, &abs_input_path);
+        if repo_relative_path == Path::new(".") {
+            return Ok(RepoPath::root());
+        }
+        let components = repo_relative_path
+            .components()
+            .map(|c| match c {
+                Component::Normal(a) => Ok(RepoPathComponent::from(a.to_str().unwrap())),
+                _ => Err(FsPathParseError::InputNotInRepo(input.to_string())),
+            })
+            .collect::<Result<Vec<_>, _>>()?;
+        Ok(RepoPath::from_components(components))
+    }
+
     /// The full string form used internally, not for presenting to users (where
     /// we may want to use the platform's separator). This format includes a
     /// trailing slash, unless this path represents the root directory. That
@@ -155,9 +178,15 @@ impl RepoPathJoin<RepoPathComponent> for RepoPath {
     }
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub enum FsPathParseError {
+    InputNotInRepo(String),
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::testutils;
 
     #[test]
     fn test_is_root() {
@@ -276,4 +305,122 @@ mod tests {
             Path::new("dir/file")
         );
     }
+
+    #[test]
+    fn parse_fs_path_wc_in_cwd() {
+        let temp_dir = testutils::new_temp_dir();
+        let cwd_path = temp_dir.path().join("repo");
+        let wc_path = cwd_path.clone();
+
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, ""),
+            Ok(RepoPath::root())
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "."),
+            Ok(RepoPath::root())
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "file"),
+            Ok(RepoPath::from_internal_string("file"))
+        );
+        // Both slash and the platform's separator are allowed
+        assert_eq!(
+            RepoPath::parse_fs_path(
+                &cwd_path,
+                &wc_path,
+                &format!("dir{}file", std::path::MAIN_SEPARATOR)
+            ),
+            Ok(RepoPath::from_internal_string("dir/file"))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "dir/file"),
+            Ok(RepoPath::from_internal_string("dir/file"))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, ".."),
+            Err(FsPathParseError::InputNotInRepo("..".to_string()))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &cwd_path, "../repo"),
+            Ok(RepoPath::root())
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &cwd_path, "../repo/file"),
+            Ok(RepoPath::from_internal_string("file"))
+        );
+        // Input may be absolute path with ".."
+        assert_eq!(
+            RepoPath::parse_fs_path(
+                &cwd_path,
+                &cwd_path,
+                cwd_path.join("../repo").to_str().unwrap()
+            ),
+            Ok(RepoPath::root())
+        );
+    }
+
+    #[test]
+    fn parse_fs_path_wc_in_cwd_parent() {
+        let temp_dir = testutils::new_temp_dir();
+        let cwd_path = temp_dir.path().join("dir");
+        let wc_path = cwd_path.parent().unwrap().to_path_buf();
+
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, ""),
+            Ok(RepoPath::from_internal_string("dir"))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "."),
+            Ok(RepoPath::from_internal_string("dir"))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "file"),
+            Ok(RepoPath::from_internal_string("dir/file"))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "subdir/file"),
+            Ok(RepoPath::from_internal_string("dir/subdir/file"))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, ".."),
+            Ok(RepoPath::root())
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "../.."),
+            Err(FsPathParseError::InputNotInRepo("../..".to_string()))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "../other-dir/file"),
+            Ok(RepoPath::from_internal_string("other-dir/file"))
+        );
+    }
+
+    #[test]
+    fn parse_fs_path_wc_in_cwd_child() {
+        let temp_dir = testutils::new_temp_dir();
+        let cwd_path = temp_dir.path().join("cwd");
+        let wc_path = cwd_path.join("repo");
+
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, ""),
+            Err(FsPathParseError::InputNotInRepo("".to_string()))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "not-repo"),
+            Err(FsPathParseError::InputNotInRepo("not-repo".to_string()))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "repo"),
+            Ok(RepoPath::root())
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "repo/file"),
+            Ok(RepoPath::from_internal_string("file"))
+        );
+        assert_eq!(
+            RepoPath::parse_fs_path(&cwd_path, &wc_path, "repo/dir/file"),
+            Ok(RepoPath::from_internal_string("dir/file"))
+        );
+    }
 }

src/cli_util.rs

@@ -32,7 +32,7 @@ use jujutsu_lib::op_heads_store::{OpHeadResolutionError, OpHeads, OpHeadsStore};
 use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, WorkspaceId};
 use jujutsu_lib::operation::Operation;
 use jujutsu_lib::repo::{BackendFactories, MutableRepo, ReadonlyRepo, RepoRef, RewriteRootCommit};
-use jujutsu_lib::repo_path::RepoPath;
+use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};
 use jujutsu_lib::revset::{RevsetError, RevsetParseError};
 use jujutsu_lib::settings::UserSettings;
 use jujutsu_lib::transaction::Transaction;
@@ -47,7 +47,7 @@ use crate::config::read_config;
 use crate::diff_edit::DiffEditError;
 use crate::formatter::Formatter;
 use crate::templater::TemplateFormatter;
-use crate::ui::{ColorChoice, FilePathParseError, Ui};
+use crate::ui::{ColorChoice, Ui};
 
 pub enum CommandError {
     UserError(String),
@@ -166,10 +166,10 @@ impl From<RevsetError> for CommandError {
     }
 }
 
-impl From<FilePathParseError> for CommandError {
-    fn from(err: FilePathParseError) -> Self {
+impl From<FsPathParseError> for CommandError {
+    fn from(err: FsPathParseError) -> Self {
         match err {
-            FilePathParseError::InputNotInRepo(input) => {
+            FsPathParseError::InputNotInRepo(input) => {
                 CommandError::UserError(format!("Path \"{input}\" is not in the repo"))
             }
         }
@@ -994,7 +994,7 @@ pub fn repo_paths_from_values(
         // TODO: Add support for globs and other formats
         let mut paths = vec![];
         for value in values {
-            let repo_path = ui.parse_file_path(wc_path, value)?;
+            let repo_path = RepoPath::parse_fs_path(ui.cwd(), wc_path, value)?;
             paths.push(repo_path);
         }
         Ok(paths)

src/commands.rs

@@ -1253,7 +1253,7 @@ fn cmd_files(ui: &mut Ui, command: &CommandHelper, args: &FilesArgs) -> Result<(
 fn cmd_print(ui: &mut Ui, command: &CommandHelper, args: &PrintArgs) -> Result<(), CommandError> {
     let workspace_command = command.workspace_helper(ui)?;
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
-    let path = ui.parse_file_path(workspace_command.workspace_root(), &args.path)?;
+    let path = RepoPath::parse_fs_path(ui.cwd(), workspace_command.workspace_root(), &args.path)?;
     let repo = workspace_command.repo();
     match commit.tree().path_value(&path) {
         None => {

src/ui.rs

@@ -13,13 +13,11 @@
 // limitations under the License.
 
 use std::io::{Stderr, Stdout, Write};
-use std::path::{Component, Path, PathBuf};
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use std::{fmt, io};
 
 use atty::Stream;
-use jujutsu_lib::file_util;
-use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent};
 use jujutsu_lib::settings::UserSettings;
 
 use crate::formatter::{Formatter, FormatterFactory};
@@ -170,142 +168,8 @@ impl Ui {
         formatter.remove_label()?;
         Ok(())
     }
-
-    /// Parses a path relative to cwd into a RepoPath relative to wc_path
-    pub fn parse_file_path(
-        &self,
-        wc_path: &Path,
-        input: &str,
-    ) -> Result<RepoPath, FilePathParseError> {
-        let abs_input_path = file_util::normalize_path(&self.cwd.join(input));
-        let repo_relative_path = file_util::relative_path(wc_path, &abs_input_path);
-        if repo_relative_path == Path::new(".") {
-            return Ok(RepoPath::root());
-        }
-        let components = repo_relative_path
-            .components()
-            .map(|c| match c {
-                Component::Normal(a) => Ok(RepoPathComponent::from(a.to_str().unwrap())),
-                _ => Err(FilePathParseError::InputNotInRepo(input.to_string())),
-            })
-            .collect::<Result<Vec<_>, _>>()?;
-        Ok(RepoPath::from_components(components))
-    }
 }
 
 enum UiOutputPair {
     Terminal { stdout: Stdout, stderr: Stderr },
 }
-
-#[derive(PartialEq, Eq, Clone, Debug)]
-pub enum FilePathParseError {
-    InputNotInRepo(String),
-}
-
-#[cfg(test)]
-mod tests {
-    use jujutsu_lib::testutils;
-
-    use super::*;
-
-    #[test]
-    fn parse_file_path_wc_in_cwd() {
-        let temp_dir = testutils::new_temp_dir();
-        let cwd_path = temp_dir.path().join("repo");
-        let wc_path = cwd_path.clone();
-        let ui = Ui::with_cwd(cwd_path.clone(), UserSettings::default());
-
-        assert_eq!(ui.parse_file_path(&wc_path, ""), Ok(RepoPath::root()));
-        assert_eq!(ui.parse_file_path(&wc_path, "."), Ok(RepoPath::root()));
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "file"),
-            Ok(RepoPath::from_internal_string("file"))
-        );
-        // Both slash and the platform's separator are allowed
-        assert_eq!(
-            ui.parse_file_path(&wc_path, &format!("dir{}file", std::path::MAIN_SEPARATOR)),
-            Ok(RepoPath::from_internal_string("dir/file"))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "dir/file"),
-            Ok(RepoPath::from_internal_string("dir/file"))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, ".."),
-            Err(FilePathParseError::InputNotInRepo("..".to_string()))
-        );
-        assert_eq!(
-            ui.parse_file_path(&cwd_path, "../repo"),
-            Ok(RepoPath::root())
-        );
-        assert_eq!(
-            ui.parse_file_path(&cwd_path, "../repo/file"),
-            Ok(RepoPath::from_internal_string("file"))
-        );
-        // Input may be absolute path with ".."
-        assert_eq!(
-            ui.parse_file_path(&cwd_path, cwd_path.join("../repo").to_str().unwrap()),
-            Ok(RepoPath::root())
-        );
-    }
-
-    #[test]
-    fn parse_file_path_wc_in_cwd_parent() {
-        let temp_dir = testutils::new_temp_dir();
-        let cwd_path = temp_dir.path().join("dir");
-        let wc_path = cwd_path.parent().unwrap().to_path_buf();
-        let ui = Ui::with_cwd(cwd_path, UserSettings::default());
-
-        assert_eq!(
-            ui.parse_file_path(&wc_path, ""),
-            Ok(RepoPath::from_internal_string("dir"))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "."),
-            Ok(RepoPath::from_internal_string("dir"))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "file"),
-            Ok(RepoPath::from_internal_string("dir/file"))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "subdir/file"),
-            Ok(RepoPath::from_internal_string("dir/subdir/file"))
-        );
-        assert_eq!(ui.parse_file_path(&wc_path, ".."), Ok(RepoPath::root()));
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "../.."),
-            Err(FilePathParseError::InputNotInRepo("../..".to_string()))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "../other-dir/file"),
-            Ok(RepoPath::from_internal_string("other-dir/file"))
-        );
-    }
-
-    #[test]
-    fn parse_file_path_wc_in_cwd_child() {
-        let temp_dir = testutils::new_temp_dir();
-        let cwd_path = temp_dir.path().join("cwd");
-        let wc_path = cwd_path.join("repo");
-        let ui = Ui::with_cwd(cwd_path, UserSettings::default());
-
-        assert_eq!(
-            ui.parse_file_path(&wc_path, ""),
-            Err(FilePathParseError::InputNotInRepo("".to_string()))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "not-repo"),
-            Err(FilePathParseError::InputNotInRepo("not-repo".to_string()))
-        );
-        assert_eq!(ui.parse_file_path(&wc_path, "repo"), Ok(RepoPath::root()));
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "repo/file"),
-            Ok(RepoPath::from_internal_string("file"))
-        );
-        assert_eq!(
-            ui.parse_file_path(&wc_path, "repo/dir/file"),
-            Ok(RepoPath::from_internal_string("dir/file"))
-        );
-    }
-}