From 30984dae4a92fbee0dc0b0cbcbaefab2a93a2aa3 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 8 Apr 2024 16:43:18 +0900
Subject: [PATCH] cli: if enabled, parse path arguments as fileset expressions

If this doesn't work out, maybe we can try one of these:
 a. fall back to bare file name if expression doesn't contain any operator-like
    characters (e.g. "f(x" is an error, but "f x" can be parsed as bare string)
 b. introduce command-line flag to opt in (e.g. -e FILESET)
 c. introduce pattern prefix to opt in (e.g. set:FILESET)

Closes #3239, #2915, #2286
---
 CHANGELOG.md                  |  5 +++--
 cli/src/cli_util.rs           | 11 ++---------
 cli/tests/test_log_command.rs | 17 ++++++++++++++---
 docs/filesets.md              | 14 ++++++++++++++
 lib/src/fileset.rs            | 11 -----------
 5 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a57c048e3..fd0e31e78 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,8 +31,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * `jj status` now supports filtering by paths. For example, `jj status .` will
   only list changed files that are descendants of the current directory.
 
-* A new config option `ui.allow-filesets` has been added to enable [file pattern
-  syntax](docs/filesets.md#file-patterns).
+* A new config option `ui.allow-filesets` has been added to enable ["fileset"
+  expressions](docs/filesets.md). Note that filesets are currently experimental,
+  but will be enabled by default in a future release.
 
 * `jj prev` and `jj next` now work when the working copy revision is a merge.
 
diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index ba34861ca..f4b129c7d 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -36,7 +36,7 @@ use indexmap::{IndexMap, IndexSet};
 use itertools::Itertools;
 use jj_lib::backend::{ChangeId, CommitId, MergedTreeId, TreeValue};
 use jj_lib::commit::Commit;
-use jj_lib::fileset::{FilePattern, FilesetExpression, FilesetParseContext};
+use jj_lib::fileset::{FilesetExpression, FilesetParseContext};
 use jj_lib::git_backend::GitBackend;
 use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile};
 use jj_lib::hex_util::to_reverse_hex;
@@ -657,14 +657,7 @@ impl WorkspaceCommandHelper {
         if values.is_empty() {
             Ok(FilesetExpression::all())
         } else if self.settings.config().get_bool("ui.allow-filesets")? {
-            let ctx = self.fileset_parse_context();
-            let expressions = values
-                .iter()
-                .map(|v| FilePattern::parse(&ctx, v))
-                .map_ok(FilesetExpression::pattern)
-                .try_collect()
-                .map_err(user_error)?;
-            Ok(FilesetExpression::union_all(expressions))
+            self.parse_union_filesets(values)
         } else {
             let expressions = values
                 .iter()
diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs
index b2c1b55e6..9b6ec1bd4 100644
--- a/cli/tests/test_log_command.rs
+++ b/cli/tests/test_log_command.rs
@@ -825,15 +825,26 @@ fn test_log_filtered_by_path() {
     // Fileset/pattern syntax is disabled by default.
     let stderr = test_env.jj_cmd_failure(
         test_env.env_root(),
-        &["log", "-R", repo_path.to_str().unwrap(), "root:file1"],
+        &["log", "-R", repo_path.to_str().unwrap(), "all()"],
     );
     insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
-    Error: Path "root:file1" is not in the repo "repo"
-    Caused by: Invalid component ".." in repo-relative path "../root:file1"
+    Error: Path "all()" is not in the repo "repo"
+    Caused by: Invalid component ".." in repo-relative path "../all()"
     "###);
 
     test_env.add_config("ui.allow-filesets = true");
 
+    // empty revisions are filtered out by "all()" fileset.
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-Tdescription", "-s", "all()"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @  second
+    │  M file1
+    │  A file2
+    ◉  first
+    │  A file1
+    ~
+    "###);
+
     // "root:<path>" is resolved relative to the workspace root.
     let stdout = test_env.jj_cmd_success(
         test_env.env_root(),
diff --git a/docs/filesets.md b/docs/filesets.md
index 768c70660..ea910c280 100644
--- a/docs/filesets.md
+++ b/docs/filesets.md
@@ -42,3 +42,17 @@ You can also specify patterns by using functions.
 
 * `all()`: Matches everything.
 * `none()`: Matches nothing.
+
+## Examples
+
+Show diff excluding `Cargo.lock`.
+
+```
+jj diff '~Cargo.lock'
+```
+
+Split a revision in two, putting `foo` into the second commit.
+
+```
+jj split '~foo'
+```
diff --git a/lib/src/fileset.rs b/lib/src/fileset.rs
index aada568f2..19ee78698 100644
--- a/lib/src/fileset.rs
+++ b/lib/src/fileset.rs
@@ -60,17 +60,6 @@ pub enum FilePattern {
 }
 
 impl FilePattern {
-    /// Parses the given `input` string as a file pattern.
-    // TODO: If we decide to parse any file argument as a fileset expression,
-    // this function can be removed.
-    pub fn parse(ctx: &FilesetParseContext, input: &str) -> Result<Self, FilePatternParseError> {
-        if let Some((kind, pat)) = input.split_once(':') {
-            Self::from_str_kind(ctx, pat, kind)
-        } else {
-            Self::cwd_prefix_path(ctx, input)
-        }
-    }
-
     /// Parses the given `input` string as pattern of the specified `kind`.
     pub fn from_str_kind(
         ctx: &FilesetParseContext,