From 7ab734488865ace972f2a340d99ce8c55b55d064 Mon Sep 17 00:00:00 2001
From: Samuel Tardieu <sam@rfc1149.net>
Date: Wed, 2 Oct 2024 13:01:47 +0200
Subject: [PATCH] cli: add hint when invalid fileset expressions look like file
 paths

---
 CHANGELOG.md                  |  3 +++
 cli/src/cli_util.rs           | 31 ++++++++++++++++++++++++-------
 cli/tests/test_global_opts.rs | 25 +++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d9466b01f37..27a6cc3e7b7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -97,6 +97,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 * Color author and committer names yellow
 
+* Commands accepting filesets suggest to use escaping if invalid fileset expressions
+  look like file paths.
+
 ### Fixed bugs
 
 * Update working copy before reporting changes. This prevents errors during reporting
diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index c49af6b7736..df640032fc3 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -1093,17 +1093,34 @@ impl WorkspaceCommandHelper {
         if values.is_empty() {
             Ok(FilesetExpression::all())
         } else if self.settings().config().get_bool("ui.allow-filesets")? {
-            self.parse_union_filesets(ui, values)
+            self.parse_union_filesets(ui, values).map_err(|mut e| {
+                if self.parse_file_path_expressions(values).is_ok() {
+                    e.add_hint(
+                        "Filesets are enabled by default. If you want to match file paths \
+                         containing characters used in fileset expressions (':', or '~'), escape \
+                         them, or prefix them with \"glob:\".",
+                    );
+                }
+                e
+            })
         } else {
-            let expressions = values
-                .iter()
-                .map(|v| self.parse_file_path(v))
-                .map_ok(FilesetExpression::prefix_path)
-                .try_collect()?;
-            Ok(FilesetExpression::union_all(expressions))
+            self.parse_file_path_expressions(values)
+                .map(FilesetExpression::union_all)
         }
     }
 
+    fn parse_file_path_expressions(
+        &self,
+        values: &[String],
+    ) -> Result<Vec<FilesetExpression>, CommandError> {
+        let expressions = values
+            .iter()
+            .map(|v| self.parse_file_path(v))
+            .map_ok(FilesetExpression::prefix_path)
+            .try_collect()?;
+        Ok(expressions)
+    }
+
     /// Parses the given fileset expressions and concatenates them all.
     pub fn parse_union_filesets(
         &self,
diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs
index a0ca70a5932..e49fd365de0 100644
--- a/cli/tests/test_global_opts.rs
+++ b/cli/tests/test_global_opts.rs
@@ -291,6 +291,31 @@ fn test_bad_path() {
     "###);
 }
 
+#[test]
+fn test_invalid_filesets_looking_like_filepaths() {
+    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 stderr = test_env.jj_cmd_failure(&repo_path, &["file", "show", "abc~"]);
+    insta::assert_snapshot!(stderr, @r#"
+    Error: Failed to parse fileset: Syntax error
+    Caused by:  --> 1:5
+      |
+    1 | abc~
+      |     ^---
+      |
+      = expected `~` or <primary>
+    Hint: Filesets are enabled by default. If you want to match file paths containing characters used in fileset expressions (':', or '~'), escape them, or prefix them with "glob:".
+    "#);
+
+    test_env.add_config(r#"ui.allow-filesets=false"#);
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["file", "show", "abc~"]);
+    insta::assert_snapshot!(stderr, @r#"
+    Error: No such path: abc~
+    "#);
+}
+
 #[test]
 fn test_broken_repo_structure() {
     let test_env = TestEnvironment::default();