cli: parse file names as relative and using platform separator

src/commands.rs

@@ -44,7 +44,6 @@ use jujutsu_lib::op_heads_store::OpHeadsStore;
 use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId};
 use jujutsu_lib::operation::Operation;
 use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, RepoInitError, RepoLoadError, RepoLoader};
-use jujutsu_lib::repo_path::RepoPath;
 use jujutsu_lib::revset::{RevsetError, RevsetExpression, RevsetParseError};
 use jujutsu_lib::revset_graph_iterator::RevsetGraphEdgeType;
 use jujutsu_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit};
@@ -64,7 +63,7 @@ use crate::formatter::Formatter;
 use crate::graphlog::{AsciiGraphDrawer, Edge};
 use crate::template_parser::TemplateParser;
 use crate::templater::Template;
-use crate::ui::Ui;
+use crate::ui::{FilePathParseError, Ui};
 
 enum CommandError {
     UserError(String),
@@ -119,6 +118,16 @@ impl From<RevsetError> for CommandError {
     }
 }
 
+impl From<FilePathParseError> for CommandError {
+    fn from(err: FilePathParseError) -> Self {
+        match err {
+            FilePathParseError::InputNotInRepo(input) => {
+                CommandError::UserError(format!("Path \"{}\" is not in the repo", input))
+            }
+        }
+    }
+}
+
 fn get_repo(ui: &Ui, matches: &ArgMatches) -> Result<Arc<ReadonlyRepo>, CommandError> {
     let wc_path_str = matches.value_of("repository").unwrap();
     let wc_path = ui.cwd().join(wc_path_str);
@@ -492,18 +501,23 @@ fn resolve_single_op_from_store(
     }
 }
 
-fn matcher_from_values(values: Option<clap::Values>) -> Box<dyn Matcher> {
+fn matcher_from_values(
+    ui: &Ui,
+    wc_path: &Path,
+    values: Option<clap::Values>,
+) -> Result<Box<dyn Matcher>, CommandError> {
     if let Some(values) = values {
         // TODO: Interpret path relative to cwd (not repo root)
         // TODO: Accept backslash separator on Windows
         // TODO: Add support for matching directories (and probably globs and other
         // formats)
-        let paths = values
+        let mut paths = HashSet::new();
-            .map(|path| RepoPath::from_internal_string(path))
+        for value in values {
-            .collect();
+            paths.insert(ui.parse_file_path(wc_path, value)?);
-        Box::new(FilesMatcher::new(paths))
+        }
+        Ok(Box::new(FilesMatcher::new(paths)))
     } else {
-        Box::new(EverythingMatcher)
+        Ok(Box::new(EverythingMatcher))
     }
 }
 
@@ -1071,8 +1085,9 @@ fn cmd_diff(
         from_tree = merge_commit_trees(repo_command.repo().as_repo_ref(), &parents);
         to_tree = commit.tree()
     }
-    let matcher = matcher_from_values(sub_matches.values_of("paths"));
     let repo = repo_command.repo();
+    let matcher =
+        matcher_from_values(ui, repo.working_copy_path(), sub_matches.values_of("paths"))?;
     if sub_matches.is_present("summary") {
         let summary = from_tree.diff_summary(&to_tree, matcher.as_ref());
         show_diff_summary(ui, repo.working_copy_path(), &summary)?;
@@ -1815,7 +1830,8 @@ fn cmd_restore(
         tree_id =
             crate::diff_edit::edit_diff(ui, &from_commit.tree(), &to_commit.tree(), &instructions)?;
     } else if sub_matches.is_present("paths") {
-        let matcher = matcher_from_values(sub_matches.values_of("paths"));
+        let matcher =
+            matcher_from_values(ui, repo.working_copy_path(), sub_matches.values_of("paths"))?;
         let mut tree_builder = repo.store().tree_builder(to_commit.tree().id().clone());
         for (repo_path, diff) in from_commit.tree().diff(&to_commit.tree(), matcher.as_ref()) {
             match diff.into_options().0 {

src/ui.rs

@@ -13,13 +13,13 @@
 // limitations under the License.
 
 use std::io::Write;
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 use std::sync::{Mutex, MutexGuard};
 use std::{fmt, io};
 
 use jujutsu_lib::commit::Commit;
 use jujutsu_lib::repo::RepoRef;
-use jujutsu_lib::repo_path::RepoPath;
+use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
 use jujutsu_lib::settings::UserSettings;
 
 use crate::formatter::{ColorFormatter, Formatter, PlainTextFormatter};
@@ -125,6 +125,40 @@ impl<'stdout> Ui<'stdout> {
             .unwrap()
             .to_owned()
     }
+
+    /// 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 repo_relative_path = relative_path(wc_path, &self.cwd.join(input));
+        let mut repo_path = RepoPath::root();
+        for component in repo_relative_path.components() {
+            match component {
+                Component::Normal(a) => {
+                    repo_path = repo_path.join(&RepoPathComponent::from(a.to_str().unwrap()));
+                }
+                Component::CurDir => {}
+                Component::ParentDir => {
+                    if let Some(parent) = repo_path.parent() {
+                        repo_path = parent;
+                    } else {
+                        return Err(FilePathParseError::InputNotInRepo(input.to_string()));
+                    }
+                }
+                _ => {
+                    return Err(FilePathParseError::InputNotInRepo(input.to_string()));
+                }
+            }
+        }
+        Ok(repo_path)
+    }
+}
+
+#[derive(PartialEq, Eq, Clone, Debug)]
+pub enum FilePathParseError {
+    InputNotInRepo(String),
 }
 
 fn relative_path(mut from: &Path, to: &Path) -> PathBuf {

tests/test_ui.rs

@@ -0,0 +1,117 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use std::io::Cursor;
+
+use jujutsu::ui::{FilePathParseError, Ui};
+use jujutsu_lib::repo_path::RepoPath;
+use jujutsu_lib::testutils::user_settings;
+
+#[test]
+fn test_parse_file_path_wc_in_cwd() {
+    let temp_dir = tempfile::tempdir().unwrap();
+    let cwd_path = temp_dir.path().join("repo");
+    let wc_path = cwd_path.clone();
+    let mut unused_stdout_buf = vec![];
+    let unused_stdout = Box::new(Cursor::new(&mut unused_stdout_buf));
+    let ui = Ui::new(cwd_path, unused_stdout, false, user_settings());
+
+    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()))
+    );
+    // TODO: handle these cases:
+    // 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")));
+}
+
+#[test]
+fn test_parse_file_path_wc_in_cwd_parent() {
+    let temp_dir = tempfile::tempdir().unwrap();
+    let cwd_path = temp_dir.path().join("dir");
+    let wc_path = cwd_path.parent().unwrap().to_path_buf();
+    let mut unused_stdout_buf = vec![];
+    let unused_stdout = Box::new(Cursor::new(&mut unused_stdout_buf));
+    let ui = Ui::new(cwd_path, unused_stdout, false, user_settings());
+
+    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 test_parse_file_path_wc_in_cwd_child() {
+    let temp_dir = tempfile::tempdir().unwrap();
+    let cwd_path = temp_dir.path().join("cwd");
+    let wc_path = cwd_path.join("repo");
+    let mut unused_stdout_buf = vec![];
+    let unused_stdout = Box::new(Cursor::new(&mut unused_stdout_buf));
+    let ui = Ui::new(cwd_path, unused_stdout, false, user_settings());
+
+    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"))
+    );
+}