move RepoPathUiConverter to separate module

cli/src/cli_util.rs

@@ -106,8 +106,6 @@ use jj_lib::repo::StoreLoadError;
 use jj_lib::repo::merge_factories_map;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::repo_path::RepoPathBuf;
-use jj_lib::repo_path::RepoPathUiConverter;
-use jj_lib::repo_path::UiPathParseError;
 use jj_lib::revset;
 use jj_lib::revset::ResolvedRevsetExpression;
 use jj_lib::revset::RevsetAliasesMap;
@@ -130,6 +128,8 @@ use jj_lib::str_util::StringExpression;
 use jj_lib::str_util::StringMatcher;
 use jj_lib::transaction::Transaction;
 use jj_lib::transaction::TransactionCommitError;
+use jj_lib::ui_path::RepoPathUiConverter;
+use jj_lib::ui_path::UiPathParseError;
 use jj_lib::working_copy;
 use jj_lib::working_copy::CheckoutStats;
 use jj_lib::working_copy::LockedWorkingCopy;

cli/src/command_error.rs

@@ -45,7 +45,6 @@ use jj_lib::repo::EditCommitError;
 use jj_lib::repo::RepoLoaderError;
 use jj_lib::repo::RewriteRootCommit;
 use jj_lib::repo_path::RepoPathBuf;
-use jj_lib::repo_path::UiPathParseError;
 use jj_lib::revset;
 use jj_lib::revset::RevsetEvaluationError;
 use jj_lib::revset::RevsetParseError;
@@ -55,6 +54,7 @@ use jj_lib::secure_config::SecureConfigError;
 use jj_lib::str_util::StringPatternParseError;
 use jj_lib::trailer::TrailerParseError;
 use jj_lib::transaction::TransactionCommitError;
+use jj_lib::ui_path::UiPathParseError;
 use jj_lib::view::RenameWorkspaceError;
 use jj_lib::working_copy::RecoverWorkspaceError;
 use jj_lib::working_copy::ResetError;

cli/src/commands/file/track.rs

@@ -15,7 +15,7 @@
 use std::io;
 use std::io::Write as _;
 
-use jj_lib::repo_path::RepoPathUiConverter;
+use jj_lib::ui_path::RepoPathUiConverter;
 use jj_lib::working_copy::SnapshotStats;
 use jj_lib::working_copy::UntrackedReason;
 use tracing::instrument;

cli/src/commands/fix.rs

@@ -37,10 +37,10 @@ use jj_lib::fix::compute_file_line_count;
 use jj_lib::fix::fix_files;
 use jj_lib::matchers::Matcher;
 use jj_lib::repo::Repo as _;
-use jj_lib::repo_path::RepoPathUiConverter;
 use jj_lib::revset::RevsetStreamExt as _;
 use jj_lib::settings::UserSettings;
 use jj_lib::store::Store;
+use jj_lib::ui_path::RepoPathUiConverter;
 use pollster::FutureExt as _;
 use tracing::instrument;
 

cli/src/commit_templater.rs

@@ -67,7 +67,6 @@ use jj_lib::ref_name::WorkspaceNameBuf;
 use jj_lib::repo::Repo;
 use jj_lib::repo::RepoLoader;
 use jj_lib::repo_path::RepoPathBuf;
-use jj_lib::repo_path::RepoPathUiConverter;
 use jj_lib::revset;
 use jj_lib::revset::Revset;
 use jj_lib::revset::RevsetContainingFn;
@@ -83,6 +82,7 @@ use jj_lib::signing::Verification;
 use jj_lib::store::Store;
 use jj_lib::trailer;
 use jj_lib::trailer::Trailer;
+use jj_lib::ui_path::RepoPathUiConverter;
 use jj_lib::workspace::DefaultWorkspaceLoaderFactory;
 use jj_lib::workspace::WorkspaceLoaderFactory as _;
 use jj_lib::workspace_store::SimpleWorkspaceStore;

cli/src/diff_util.rs

@@ -75,10 +75,10 @@ use jj_lib::merged_tree::MergedTree;
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::InvalidRepoPathError;
 use jj_lib::repo_path::RepoPath;
-use jj_lib::repo_path::RepoPathUiConverter;
 use jj_lib::rewrite::rebase_to_dest_parent;
 use jj_lib::settings::UserSettings;
 use jj_lib::store::Store;
+use jj_lib::ui_path::RepoPathUiConverter;
 use thiserror::Error;
 use tracing::instrument;
 use unicode_width::UnicodeWidthStr as _;

cli/src/merge_tools/external.rs

@@ -23,8 +23,8 @@ use jj_lib::merge::Diff;
 use jj_lib::merge::Merge;
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::merged_tree_builder::MergedTreeBuilder;
-use jj_lib::repo_path::RepoPathUiConverter;
 use jj_lib::store::Store;
+use jj_lib::ui_path::RepoPathUiConverter;
 use thiserror::Error;
 
 use super::ConflictResolveError;

cli/src/merge_tools/mod.rs

@@ -38,8 +38,8 @@ use jj_lib::merged_tree_builder::MergedTreeBuilder;
 use jj_lib::repo_path::InvalidRepoPathError;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::repo_path::RepoPathBuf;
-use jj_lib::repo_path::RepoPathUiConverter;
 use jj_lib::settings::UserSettings;
+use jj_lib::ui_path::RepoPathUiConverter;
 use jj_lib::working_copy::SnapshotError;
 use thiserror::Error;
 

lib/src/fileset.rs

@@ -49,8 +49,8 @@ use crate::matchers::UnionMatcher;
 use crate::repo_path::RelativePathParseError;
 use crate::repo_path::RepoPath;
 use crate::repo_path::RepoPathBuf;
-use crate::repo_path::RepoPathUiConverter;
-use crate::repo_path::UiPathParseError;
+use crate::ui_path::RepoPathUiConverter;
+use crate::ui_path::UiPathParseError;
 
 /// Error occurred during file pattern parsing.
 #[derive(Debug, Error)]

lib/src/lib.rs

@@ -115,6 +115,7 @@ pub mod trailer;
 pub mod transaction;
 pub mod tree;
 pub mod tree_builder;
+pub mod ui_path;
 pub mod union_find;
 pub mod view;
 pub mod working_copy;

lib/src/repo_path.rs

@@ -34,7 +34,6 @@ use thiserror::Error;
 
 use crate::content_hash::ContentHash;
 use crate::file_util;
-use crate::merge::Diff;
 
 /// Owned `RepoPath` component.
 #[derive(ContentHash, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -648,111 +647,6 @@ fn is_valid_repo_path_str(value: &str) -> bool {
     !value.starts_with('/') && !value.ends_with('/') && !value.contains("//")
 }
 
-/// An error from `RepoPathUiConverter::parse_file_path`.
-#[derive(Debug, Error)]
-pub enum UiPathParseError {
-    #[error(transparent)]
-    Fs(FsPathParseError),
-}
-
-/// Converts `RepoPath`s to and from plain strings as displayed to the user
-/// (e.g. relative to CWD).
-#[derive(Debug, Clone)]
-pub enum RepoPathUiConverter {
-    /// Variant for a local file system. Paths are interpreted relative to `cwd`
-    /// with the repo rooted in `base`.
-    ///
-    /// The `cwd` and `base` paths are supposed to be absolute and normalized in
-    /// the same manner.
-    Fs { cwd: PathBuf, base: PathBuf },
-    // TODO: Add a no-op variant that uses the internal `RepoPath` representation. Can be useful
-    // on a server.
-}
-
-impl RepoPathUiConverter {
-    /// Format a path for display in the UI.
-    pub fn format_file_path(&self, file: &RepoPath) -> String {
-        match self {
-            Self::Fs { cwd, base } => {
-                file_util::relative_path(cwd, &file.to_fs_path_unchecked(base))
-                    .display()
-                    .to_string()
-            }
-        }
-    }
-
-    /// Format a copy from `before` to `after` for display in the UI by
-    /// extracting common components and producing something like
-    /// "common/prefix/{before => after}/common/suffix".
-    ///
-    /// If `before == after`, this is equivalent to `format_file_path()`.
-    pub fn format_copied_path(&self, paths: Diff<&RepoPath>) -> String {
-        match self {
-            Self::Fs { .. } => {
-                let paths = paths.map(|path| self.format_file_path(path));
-                collapse_copied_path(paths.as_deref(), std::path::MAIN_SEPARATOR)
-            }
-        }
-    }
-
-    /// Parses a path from the UI.
-    ///
-    /// It's up to the implementation whether absolute paths are allowed, and
-    /// where relative paths are interpreted as relative to.
-    pub fn parse_file_path(&self, input: &str) -> Result<RepoPathBuf, UiPathParseError> {
-        match self {
-            Self::Fs { cwd, base } => {
-                RepoPathBuf::parse_fs_path(cwd, base, input).map_err(UiPathParseError::Fs)
-            }
-        }
-    }
-}
-
-fn collapse_copied_path(paths: Diff<&str>, separator: char) -> String {
-    // The last component should never match middle components. This is ensured
-    // by including trailing separators. e.g. ("a/b", "a/b/x") => ("a/", _)
-    let components = paths.map(|path| path.split_inclusive(separator));
-    let prefix_len: usize = iter::zip(components.before, components.after)
-        .take_while(|(before, after)| before == after)
-        .map(|(_, after)| after.len())
-        .sum();
-    if paths.before.len() == prefix_len && paths.after.len() == prefix_len {
-        return paths.after.to_owned();
-    }
-
-    // The first component should never match middle components, but the first
-    // uncommon middle component can. e.g. ("a/b", "x/a/b") => ("", "/b"),
-    // ("a/b", "a/x/b") => ("a/", "/b")
-    let components = paths.map(|path| {
-        let mut remainder = &path[prefix_len.saturating_sub(1)..];
-        iter::from_fn(move || {
-            let pos = remainder.rfind(separator)?;
-            let (prefix, last) = remainder.split_at(pos);
-            remainder = prefix;
-            Some(last)
-        })
-    });
-    let suffix_len: usize = iter::zip(components.before, components.after)
-        .take_while(|(before, after)| before == after)
-        .map(|(_, after)| after.len())
-        .sum();
-
-    // Middle range may be invalid (start > end) because the same separator char
-    // can be distributed to both common prefix and suffix. e.g.
-    // ("a/b", "a/x/b") == ("a//b", "a/x/b") => ("a/", "/b")
-    let middle = paths.map(|path| path.get(prefix_len..path.len() - suffix_len).unwrap_or(""));
-
-    let mut collapsed = String::new();
-    collapsed.push_str(&paths.after[..prefix_len]);
-    collapsed.push('{');
-    collapsed.push_str(middle.before);
-    collapsed.push_str(" => ");
-    collapsed.push_str(middle.after);
-    collapsed.push('}');
-    collapsed.push_str(&paths.after[paths.after.len() - suffix_len..]);
-    collapsed
-}
-
 /// Tree that maps `RepoPath` to value of type `V`.
 #[derive(Clone, Default, Eq, PartialEq)]
 pub struct RepoPathTree<V> {
@@ -1317,89 +1211,4 @@ mod tests {
             Ok(repo_path("dir/file"))
         );
     }
-
-    #[test]
-    fn test_format_copied_path() {
-        let ui = RepoPathUiConverter::Fs {
-            cwd: PathBuf::from("."),
-            base: PathBuf::from("."),
-        };
-
-        let format = |before, after| {
-            ui.format_copied_path(Diff::new(repo_path(before), repo_path(after)))
-                .replace('\\', "/")
-        };
-
-        assert_eq!(format("one/two/three", "one/two/three"), "one/two/three");
-        assert_eq!(format("one/two", "one/two/three"), "one/{two => two/three}");
-        assert_eq!(format("one/two", "zero/one/two"), "{one => zero/one}/two");
-        assert_eq!(format("one/two/three", "one/two"), "one/{two/three => two}");
-        assert_eq!(format("zero/one/two", "one/two"), "{zero/one => one}/two");
-        assert_eq!(
-            format("one/two", "one/two/three/one/two"),
-            "one/{ => two/three/one}/two"
-        );
-
-        assert_eq!(format("two/three", "four/three"), "{two => four}/three");
-        assert_eq!(
-            format("one/two/three", "one/four/three"),
-            "one/{two => four}/three"
-        );
-        assert_eq!(format("one/two/three", "one/three"), "one/{two => }/three");
-        assert_eq!(format("one/two", "one/four"), "one/{two => four}");
-        assert_eq!(format("two", "four"), "{two => four}");
-        assert_eq!(format("file1", "file2"), "{file1 => file2}");
-        assert_eq!(format("file-1", "file-2"), "{file-1 => file-2}");
-        assert_eq!(
-            format("x/something/something/2to1.txt", "x/something/2to1.txt"),
-            "x/something/{something => }/2to1.txt"
-        );
-        assert_eq!(
-            format("x/something/1to2.txt", "x/something/something/1to2.txt"),
-            "x/something/{ => something}/1to2.txt"
-        );
-    }
-
-    #[test]
-    fn test_split_common_prefix() {
-        assert_eq!(
-            repo_path("foo/bar").split_common_prefix(repo_path("foo/bar/baz")),
-            (repo_path("foo/bar"), repo_path(""))
-        );
-
-        assert_eq!(
-            repo_path("foo/bar/baz").split_common_prefix(repo_path("foo/bar")),
-            (repo_path("foo/bar"), repo_path("baz"))
-        );
-
-        assert_eq!(
-            repo_path("foo/bar/bing").split_common_prefix(repo_path("foo/bar/baz")),
-            (repo_path("foo/bar"), repo_path("bing"))
-        );
-
-        assert_eq!(
-            repo_path("no/common/prefix").split_common_prefix(repo_path("foo/bar/baz")),
-            (RepoPath::root(), repo_path("no/common/prefix"))
-        );
-
-        assert_eq!(
-            repo_path("same/path").split_common_prefix(repo_path("same/path")),
-            (repo_path("same/path"), RepoPath::root())
-        );
-
-        assert_eq!(
-            RepoPath::root().split_common_prefix(repo_path("foo")),
-            (RepoPath::root(), RepoPath::root())
-        );
-
-        assert_eq!(
-            RepoPath::root().split_common_prefix(RepoPath::root()),
-            (RepoPath::root(), RepoPath::root())
-        );
-
-        assert_eq!(
-            repo_path("foo/bar").split_common_prefix(RepoPath::root()),
-            (RepoPath::root(), repo_path("foo/bar"))
-        );
-    }
 }

lib/src/revset.rs

@@ -62,7 +62,6 @@ use crate::ref_name::WorkspaceNameBuf;
 use crate::repo::ReadonlyRepo;
 use crate::repo::Repo;
 use crate::repo::RepoLoaderError;
-use crate::repo_path::RepoPathUiConverter;
 use crate::revset_parser;
 pub use crate::revset_parser::BinaryOp;
 pub use crate::revset_parser::ExpressionKind;
@@ -81,6 +80,7 @@ use crate::str_util::StringExpression;
 use crate::str_util::StringPattern;
 use crate::time_util::DatePattern;
 use crate::time_util::DatePatternContext;
+use crate::ui_path::RepoPathUiConverter;
 
 /// Error occurred during symbol resolution.
 #[derive(Debug, Error)]