From 7400736a95c9e9eaa34c364241383df3eac95f4e Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 8 Apr 2024 20:52:45 +0900
Subject: [PATCH] cli: suggest root:"<path>" if cwd-relative path is not in
 workspace

Closes #3216
---
 cli/src/command_error.rs      | 20 +++++++++++---
 cli/tests/test_global_opts.rs | 51 +++++++++++++++++++++++++++++++++--
 lib/src/repo_path.rs          |  9 ++++---
 3 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs
index c9121e85c5..d107ac2add 100644
--- a/cli/src/command_error.rs
+++ b/cli/src/command_error.rs
@@ -19,14 +19,14 @@ use std::{error, io, iter, str};
 
 use itertools::Itertools as _;
 use jj_lib::backend::BackendError;
-use jj_lib::fileset::{FilesetParseError, FilesetParseErrorKind};
+use jj_lib::fileset::{FilePatternParseError, FilesetParseError, FilesetParseErrorKind};
 use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError, GitRemoteManagementError};
 use jj_lib::gitignore::GitIgnoreError;
 use jj_lib::op_heads_store::OpHeadResolutionError;
 use jj_lib::op_store::OpStoreError;
 use jj_lib::op_walk::OpsetEvaluationError;
 use jj_lib::repo::{CheckOutCommitError, EditCommitError, RepoLoaderError, RewriteRootCommit};
-use jj_lib::repo_path::FsPathParseError;
+use jj_lib::repo_path::{FsPathParseError, RepoPathBuf};
 use jj_lib::revset::{
     RevsetEvaluationError, RevsetParseError, RevsetParseErrorKind, RevsetResolutionError,
 };
@@ -539,15 +539,27 @@ impl From<GitIgnoreError> for CommandError {
 
 fn find_source_parse_error_hint(err: &dyn error::Error) -> Option<String> {
     let source = err.source()?;
-    // TODO: For FilePatternParseError, suggest "root:<path>" if the user
-    // input looks like repo-relative path #3216.
     if let Some(source) = source.downcast_ref() {
+        file_pattern_parse_error_hint(source)
+    } else if let Some(source) = source.downcast_ref() {
         string_pattern_parse_error_hint(source)
     } else {
         None
     }
 }
 
+fn file_pattern_parse_error_hint(err: &FilePatternParseError) -> Option<String> {
+    match err {
+        FilePatternParseError::InvalidKind(_) => None,
+        // Suggest root:"<path>" if input can be parsed as repo-relative path
+        FilePatternParseError::FsPath(e) => RepoPathBuf::from_relative_path(&e.input)
+            .ok()
+            .map(|path| format!(r#"Consider using root:{path:?} to specify repo-relative path"#)),
+        FilePatternParseError::RelativePath(_) => None,
+        FilePatternParseError::GlobPattern(_) => None,
+    }
+}
+
 fn string_pattern_parse_error_hint(err: &StringPatternParseError) -> Option<String> {
     match err {
         StringPatternParseError::InvalidKind(_) => {
diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs
index cf358b9b90..9b55fd5217 100644
--- a/cli/tests/test_global_opts.rs
+++ b/cli/tests/test_global_opts.rs
@@ -230,13 +230,60 @@ fn test_bad_path() {
     let 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");
+    let subdir = repo_path.join("dir");
+    std::fs::create_dir_all(&subdir).unwrap();
+
+    test_env.add_config("ui.allow-filesets = true");
 
+    // cwd == workspace_root
     let stderr = test_env.jj_cmd_failure(&repo_path, &["cat", "../out"]);
     insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
-    Error: Path "../out" is not in the repo "."
-    Caused by: Invalid component ".." in repo-relative path "../out"
+    Error: Failed to parse fileset: Invalid file pattern
+    Caused by:
+    1:  --> 1:1
+      |
+    1 | ../out
+      | ^----^
+      |
+      = Invalid file pattern
+    2: Path "../out" is not in the repo "."
+    3: Invalid component ".." in repo-relative path "../out"
+    "###);
+
+    // cwd != workspace_root, can't be parsed as repo-relative path
+    let stderr = test_env.jj_cmd_failure(&subdir, &["cat", "../.."]);
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Error: Failed to parse fileset: Invalid file pattern
+    Caused by:
+    1:  --> 1:1
+      |
+    1 | ../..
+      | ^---^
+      |
+      = Invalid file pattern
+    2: Path "../.." is not in the repo "../"
+    3: Invalid component ".." in repo-relative path "../"
     "###);
 
+    // cwd != workspace_root, can be parsed as repo-relative path
+    let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["cat", "-Rrepo", "out"]);
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Error: Failed to parse fileset: Invalid file pattern
+    Caused by:
+    1:  --> 1:1
+      |
+    1 | out
+      | ^-^
+      |
+      = Invalid file pattern
+    2: Path "out" is not in the repo "repo"
+    3: Invalid component ".." in repo-relative path "../out"
+    Hint: Consider using root:"out" to specify repo-relative path
+    "###);
+
+    test_env.add_config("ui.allow-filesets = false");
+
+    // If fileset/pattern syntax is disabled, no hint should be generated
     let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["cat", "-Rrepo", "out"]);
     insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
     Error: Path "out" is not in the repo "repo"
diff --git a/lib/src/repo_path.rs b/lib/src/repo_path.rs
index 6a8716ddb3..feba9d2731 100644
--- a/lib/src/repo_path.rs
+++ b/lib/src/repo_path.rs
@@ -452,9 +452,12 @@ pub enum RelativePathParseError {
 #[derive(Clone, Debug, Eq, Error, PartialEq)]
 #[error(r#"Path "{input}" is not in the repo "{base}""#)]
 pub struct FsPathParseError {
-    base: Box<Path>,
-    input: Box<Path>,
-    source: RelativePathParseError,
+    /// Repository or workspace root path relative to the `cwd`.
+    pub base: Box<Path>,
+    /// Input path without normalization.
+    pub input: Box<Path>,
+    /// Source error.
+    pub source: RelativePathParseError,
 }
 
 fn is_valid_repo_path_component_str(value: &str) -> bool {