Repository: sharkdp/fd
Pull request #1139 (2022-10-15 12:15:40)

add support for matching multiple patterns
Added the ~`--and`~ ~`--expr`~ `--and` flags that search for matches containing all the patterns, similar to the `-and` flag of GNU find.

https://github.com/minad/consult/issues/6
#315 

sharkdp: Thank you very much for your contribution!

Before we consider the integration of this new feature, please see my comment in https://github.com/sharkdp/fd/issues/315#issuecomment-968323590:

> Ok, I'm inclined to accept a feature request to support `--and <pattern>`. Before we implement this, we need:
> 
> - A short discussion about the command-line option name. What do other tools use?
> - A detailed analysis if this could clash with any of the other command-line options or features of `fd`. There are some immediate questions like: what does `fd patternA --and patternB --type f` mean? (we are not going to support the meaning *patternA AND (patternB AND type==file)*).

Ideally, the second point should be discussed in detail and represented in corresponding unit tests.
Uthar: > A short discussion about the command-line option name. What do other tools use?

- ripgrep uses `-e`

```
-e, --regexp <PATTERN>...                    
  A pattern to search for. This option can be provided multiple times, where
  all patterns given are searched. Lines matching at least one of the provided
  patterns are printed. This flag can also be used when searching for patterns
  that start with a dash.
  
  For example, to search for the literal '-foo', you can use this flag:
  
      rg -e -foo
  
  You can also use the special '--' delimiter to indicate that no more flags
  will be provided. Namely, the following is equivalent to the above:
  
      rg -- -foo

```
- GNU find uses the following syntax:

```
expr1 expr2
       Two expressions in a row are taken to be joined with an implied -a; expr2 is not evaluated if expr1 is false.

expr1 -a expr2
       Same as expr1 expr2.

expr1 -and expr2
       Same as expr1 expr2, but not POSIX compliant.
```
Uthar: > what does fd patternA --and patternB --type f mean?

I think the `--and` flag would simply mean "Find all files whose names match all patterns specified by the initial [pattern] and all the --and flags"

No such nesting like you described would be possible

I don't know all of fd's flags, but it looks to me like it will compose well, being just an additional filter for names.

> unit tests

I created some basic unit tests. What other cases should be covered?

sharkdp: > I think the `--and` flag would simply mean "Find all files whose names match all patterns specified by the initial [pattern] and all the --and flags"

Sounds reasonable. I would still like to see some thought going into potential conflicts (or chances for combination) with other command line options - like `--type` in the example quoted above. Other potentially interesting options to look at:
* `--glob`/`--fixed-strings`
* `--ignore-case`/`--case-sensitive`
* `--full-path`
* `--extension`/`--type`/`--size`/etc.


> a broken pattern in any of the --and flags should signal an error

That is definitely worth checking.

Another thing to potentially test would be patterns starting with a dash. Usually, we solve this using the `--` separator: `fd -- '-foo'`. Not sure if we could make it work for `--and` as well. Might not even require the `--` separator, but maybe a special clap setting to allow the value to start with a dash.
Uthar: > patterns starting with a dash

One could also use `^[-]`
Uthar: Hmm you're right, maybe an `--expr`, stolen from ripgrep, would sound better?
Uthar: Wrote tests for all these in new commit:

> --glob

`--and` patterns contribute to the search as globs, not regexps


> --fixed-strings

`--and` patterns are matched as whole strings, not regexp


> --ignore-case

case is ignored in `--and` patterns as well as the primary pattern


> --case-sensitive

case is sensitive in `--and` patterns as well as the primary pattern


> --full-path

`--and` patterns contribute to the search in the whole path, not just file or dir name


> --extension

`--extension` and `--and` patterns are composed in a logical way, i.e. patterns match file names via AND and extension matches extension via OR


> --type

`--type` additionally restricts the search done by all the `--and` patterns 


> --size

didn't check, I didn't know how

also added these from my post:

> test with empty patterns
> test with unreasonable pattern should match nothing
> a broken pattern in any of the --and flags should signal an error


tests fails for non-ubuntu CI... let me check...

Uthar: > tests fails for non-ubuntu CI... let me check...

Can't make sense out of it. Is there some Mac and Windows experts on board?
tmccombs: We probably want to clearly document how `--and` interacts with other options. The behaviors described above makes sense to me and are probably what I would expect in the absence of documentation, but I could easily see someone else expecting different behavior. 

Regarding hyphens. We probably want to use the [`allow_hyphen_values`](https://docs.rs/clap/latest/clap/struct.Arg.html#method.allow_hyphen_values) option on the argument. 
sharkdp: @Uthar Thank you very much for the updates! Unfortunately, there are some larger merge conflicts now that I merged a few other PRs, especially #1067. I hope you can sort this out, otherwise please let us know.
Uthar: Yes, I will attempt to solve this, stay tuned
Uthar: Damn, looks like I will have to write it from scratch
It will take some time, I haven't written any rust code before
tmccombs: I'm willing to help if you would like. After all, I made the PR that caused most of the conflicts. 
Uthar: Very nice of you tmccombs. My main problem is to write the code again, it takes a lot of time especially debugging the type errors with borrow and so on, because I lack experience.  I just don't have the time right now

Ultimately we just have to change `pattern` to `patterns` in this line:

https://github.com/sharkdp/fd/blob/master/src/walk.rs#L153

Everything else is just argument parsing/changing parameters to functions 

Uthar: I started some work here, the easy stuff :-) https://github.com/Uthar/fd/tree/conflicts
Uthar: Ok, I've got this working now... but there are some issues that Rust experienced programmer will probably easily solve:

I'm trying to push the "initial" pattern to the optional vector of "and" patterns, but can't do that without borrowing, do you know how to handle it?

Also its panicking with no --expr flag currently, maybe unwrap() is not the best there?

I also don't know how to exit the program from the match .. Ok .. Err clause, when there is an error, it still requires a regex type.

Could you help me with these issues? See this commit: https://github.com/Uthar/fd/commit/d66470a50b05a1eb176d5ce37a2e6f89d32171a1
Uthar: Full diff here https://github.com/sharkdp/fd/compare/master...Uthar:fd:conflicts
Uthar: Done after all, we're back in the game :-)

But you should review the changes
sharkdp: Thank you. The tests are currently failing on Windows and macOS
Uthar: yeah... the failuress are really weird, dont make sense...

My friend has a macbook, I will ask them to try on it, also try to find a windows machine
Uthar: fixed the tests
Uthar: Thank you, tmccombs, for the thorough review.

See the last commit, where I added your solution to avoid `match` and `panic!` during generation of the regexp and changed type hints as you requested.

Let me know if you have some other improvement ideas.
Uthar: https://github.com/sharkdp/fd/blob/882e13a77276255e21c65af7eef6382a1fcb45a8/src/walk.rs#L47

pattern -> patterns
Uthar: So the `--expr` flag instead of `--and` sounds better to you?
Uthar: https://github.com/sharkdp/fd/blob/d498724edd8be4680ea74ad235f4d2ee8a304911/src/main.rs#L438-L442

The pattern -> A pattern / Some patterns ?
sharkdp: > So the `--expr` flag instead of `--and` sounds better to you?

No, I like `--and`.

> The pattern -> A pattern / Some patterns ?

Maybe: "The pattern(s) seems to only match… or adjust your search pattern(s)".

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 713f6b6..44e5aa8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## Features
 
+- New `--and <pattern>` option to add additional patterns that must also be matched. See #315
+  and #1139 (@Uthar)
 - Added `--changed-after` as alias for `--changed-within`, to have a name consistent with `--changed-before`.
 
 
diff --git a/src/cli.rs b/src/cli.rs
index 36c75ad..84abe9d 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -144,6 +144,18 @@ pub struct Opts {
     )]
     pub fixed_strings: bool,
 
+    /// Additional search patterns that need to be matched
+    #[arg(
+        long = "and",
+        value_name = "pattern",
+        long_help = "Add additional required search patterns, all of which must be matched. Multiple \
+                     additional patterns can be specified. The patterns are regular expressions, \
+                     unless '--glob' or '--fixed-strings' is used.",
+        hide_short_help = true,
+        allow_hyphen_values = true
+    )]
+    pub exprs: Option<Vec<String>>,
+
     /// Show absolute instead of relative paths
     #[arg(
         long,
diff --git a/src/main.rs b/src/main.rs
index cdc48c7..ec677cc 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -21,7 +21,7 @@ use atty::Stream;
 use clap::{CommandFactory, Parser};
 use globset::GlobBuilder;
 use lscolors::LsColors;
-use regex::bytes::{RegexBuilder, RegexSetBuilder};
+use regex::bytes::{Regex, RegexBuilder, RegexSetBuilder};
 
 use crate::cli::{ColorWhen, Opts};
 use crate::config::Config;
@@ -81,12 +81,28 @@ fn run() -> Result<ExitCode> {
     }
 
     ensure_search_pattern_is_not_a_path(&opts)?;
-    let pattern_regex = build_pattern_regex(&opts)?;
+    let pattern = &opts.pattern;
+    let exprs = &opts.exprs;
+    let empty = Vec::new();
+
+    let pattern_regexps = exprs
+        .as_ref()
+        .unwrap_or(&empty)
+        .iter()
+        .chain([pattern])
+        .map(|pat| build_pattern_regex(pat, &opts))
+        .collect::<Result<Vec<String>>>()?;
+
+    let config = construct_config(opts, &pattern_regexps)?;
 
-    let config = construct_config(opts, &pattern_regex)?;
-    ensure_use_hidden_option_for_leading_dot_pattern(&config, &pattern_regex)?;
-    let re = build_regex(pattern_regex, &config)?;
-    walk::scan(&search_paths, Arc::new(re), Arc::new(config))
+    ensure_use_hidden_option_for_leading_dot_pattern(&config, &pattern_regexps)?;
+
+    let regexps = pattern_regexps
+        .into_iter()
+        .map(|pat| build_regex(pat, &config))
+        .collect::<Result<Vec<Regex>>>()?;
+
+    walk::scan(&search_paths, Arc::new(regexps), Arc::new(config))
 }
 
 #[cfg(feature = "completions")]
@@ -145,8 +161,7 @@ fn ensure_search_pattern_is_not_a_path(opts: &Opts) -> Result<()> {
     }
 }
 
-fn build_pattern_regex(opts: &Opts) -> Result<String> {
-    let pattern = &opts.pattern;
+fn build_pattern_regex(pattern: &str, opts: &Opts) -> Result<String> {
     Ok(if opts.glob && !pattern.is_empty() {
         let glob = GlobBuilder::new(pattern).literal_separator(true).build()?;
         glob.regex().to_owned()
@@ -172,11 +187,14 @@ fn check_path_separator_length(path_separator: Option<&str>) -> Result<()> {
     }
 }
 
-fn construct_config(mut opts: Opts, pattern_regex: &str) -> Result<Config> {
+fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result<Config> {
     // The search will be case-sensitive if the command line flag is set or
-    // if the pattern has an uppercase character (smart case).
-    let case_sensitive =
-        !opts.ignore_case && (opts.case_sensitive || pattern_has_uppercase_char(pattern_regex));
+    // if any of the patterns has an uppercase character (smart case).
+    let case_sensitive = !opts.ignore_case
+        && (opts.case_sensitive
+            || pattern_regexps
+                .iter()
+                .any(|pat| pattern_has_uppercase_char(pat)));
 
     let path_separator = opts
         .path_separator
@@ -415,14 +433,18 @@ fn extract_time_constraints(opts: &Opts) -> Result<Vec<TimeFilter>> {
 
 fn ensure_use_hidden_option_for_leading_dot_pattern(
     config: &Config,
-    pattern_regex: &str,
+    pattern_regexps: &[String],
 ) -> Result<()> {
-    if cfg!(unix) && config.ignore_hidden && pattern_matches_strings_with_leading_dot(pattern_regex)
+    if cfg!(unix)
+        && config.ignore_hidden
+        && pattern_regexps
+            .iter()
+            .any(|pat| pattern_matches_strings_with_leading_dot(pat))
     {
         Err(anyhow!(
-            "The pattern seems to only match files with a leading dot, but hidden files are \
+            "The pattern(s) seems to only match files with a leading dot, but hidden files are \
             filtered by default. Consider adding -H/--hidden to search hidden files as well \
-            or adjust your search pattern."
+            or adjust your search pattern(s)."
         ))
     } else {
         Ok(())
diff --git a/src/walk.rs b/src/walk.rs
index 29e591b..a1afda9 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -47,12 +47,12 @@ pub const MAX_BUFFER_LENGTH: usize = 1000;
 /// Default duration until output buffering switches to streaming.
 pub const DEFAULT_MAX_BUFFER_TIME: Duration = Duration::from_millis(100);
 
-/// Recursively scan the given search path for files / pathnames matching the pattern.
+/// Recursively scan the given search path for files / pathnames matching the patterns.
 ///
 /// If the `--exec` argument was supplied, this will create a thread pool for executing
 /// jobs in parallel from a given command line and the discovered paths. Otherwise, each
 /// path will simply be written to standard output.
-pub fn scan(paths: &[PathBuf], pattern: Arc<Regex>, config: Arc<Config>) -> Result<ExitCode> {
+pub fn scan(paths: &[PathBuf], patterns: Arc<Vec<Regex>>, config: Arc<Config>) -> Result<ExitCode> {
     let first_path = &paths[0];
 
     // Channel capacity was chosen empircally to perform similarly to an unbounded channel
@@ -153,7 +153,7 @@ pub fn scan(paths: &[PathBuf], pattern: Arc<Regex>, config: Arc<Config>) -> Resu
     let receiver_thread = spawn_receiver(&config, &quit_flag, &interrupt_flag, rx);
 
     // Spawn the sender threads.
-    spawn_senders(&config, &quit_flag, pattern, parallel_walker, tx);
+    spawn_senders(&config, &quit_flag, patterns, parallel_walker, tx);
 
     // Wait for the receiver thread to print out all results.
     let exit_code = receiver_thread.join().unwrap();
@@ -383,13 +383,13 @@ fn spawn_receiver(
 fn spawn_senders(
     config: &Arc<Config>,
     quit_flag: &Arc<AtomicBool>,
-    pattern: Arc<Regex>,
+    patterns: Arc<Vec<Regex>>,
     parallel_walker: ignore::WalkParallel,
     tx: Sender<WorkerResult>,
 ) {
     parallel_walker.run(|| {
         let config = Arc::clone(config);
-        let pattern = Arc::clone(&pattern);
+        let patterns = Arc::clone(&patterns);
         let tx_thread = tx.clone();
         let quit_flag = Arc::clone(quit_flag);
 
@@ -459,7 +459,10 @@ fn spawn_senders(
                 }
             };
 
-            if !pattern.is_match(&filesystem::osstr_to_bytes(search_str.as_ref())) {
+            if !patterns
+                .iter()
+                .all(|pat| pat.is_match(&filesystem::osstr_to_bytes(search_str.as_ref())))
+            {
                 return ignore::WalkState::Continue;
             }
 


Test patch:
diff --git a/tests/tests.rs b/tests/tests.rs
index 732b61e..e884c50 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -76,6 +76,203 @@ fn test_simple() {
     );
 }
 
+static AND_EXTRA_FILES: &[&str] = &[
+    "a.foo",
+    "one/b.foo",
+    "one/two/c.foo",
+    "one/two/C.Foo2",
+    "one/two/three/baz-quux",
+    "one/two/three/Baz-Quux2",
+    "one/two/three/d.foo",
+    "fdignored.foo",
+    "gitignored.foo",
+    ".hidden.foo",
+    "A-B.jpg",
+    "A-C.png",
+    "B-A.png",
+    "B-C.png",
+    "C-A.jpg",
+    "C-B.png",
+    "e1 e2",
+];
+
+/// AND test
+#[test]
+fn test_and_basic() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &["foo", "--and", "c"],
+        "one/two/C.Foo2
+        one/two/c.foo
+        one/two/three/directory_foo/",
+    );
+
+    te.assert_output(
+        &["f", "--and", "[ad]", "--and", "[_]"],
+        "one/two/three/directory_foo/",
+    );
+
+    te.assert_output(
+        &["f", "--and", "[ad]", "--and", "[.]"],
+        "a.foo
+        one/two/three/d.foo",
+    );
+
+    te.assert_output(&["Foo", "--and", "C"], "one/two/C.Foo2");
+
+    te.assert_output(&["foo", "--and", "asdasdasdsadasd"], "");
+}
+
+#[test]
+fn test_and_empty_pattern() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+    te.assert_output(&["Foo", "--and", "2", "--and", ""], "one/two/C.Foo2");
+}
+
+#[test]
+fn test_and_bad_pattern() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_failure(&["Foo", "--and", "2", "--and", "[", "--and", "C"]);
+    te.assert_failure(&["Foo", "--and", "[", "--and", "2", "--and", "C"]);
+    te.assert_failure(&["Foo", "--and", "2", "--and", "C", "--and", "["]);
+    te.assert_failure(&["[", "--and", "2", "--and", "C", "--and", "Foo"]);
+}
+
+#[test]
+fn test_and_pattern_starts_with_dash() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &["baz", "--and", "quux"],
+        "one/two/three/Baz-Quux2
+        one/two/three/baz-quux",
+    );
+    te.assert_output(
+        &["baz", "--and", "-"],
+        "one/two/three/Baz-Quux2
+        one/two/three/baz-quux",
+    );
+    te.assert_output(
+        &["Quu", "--and", "x", "--and", "-"],
+        "one/two/three/Baz-Quux2",
+    );
+}
+
+#[test]
+fn test_and_plus_extension() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &[
+            "A",
+            "--and",
+            "B",
+            "--extension",
+            "jpg",
+            "--extension",
+            "png",
+        ],
+        "A-B.jpg
+        B-A.png",
+    );
+
+    te.assert_output(
+        &[
+            "A",
+            "--extension",
+            "jpg",
+            "--and",
+            "B",
+            "--extension",
+            "png",
+        ],
+        "A-B.jpg
+        B-A.png",
+    );
+}
+
+#[test]
+fn test_and_plus_type() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &["c", "--type", "d", "--and", "foo"],
+        "one/two/three/directory_foo/",
+    );
+
+    te.assert_output(
+        &["c", "--type", "f", "--and", "foo"],
+        "one/two/C.Foo2
+        one/two/c.foo",
+    );
+}
+
+#[test]
+fn test_and_plus_glob() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(&["*foo", "--glob", "--and", "c*"], "one/two/c.foo");
+}
+
+#[test]
+fn test_and_plus_fixed_strings() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &["foo", "--fixed-strings", "--and", "c", "--and", "."],
+        "one/two/c.foo
+        one/two/C.Foo2",
+    );
+
+    te.assert_output(
+        &["foo", "--fixed-strings", "--and", "[c]", "--and", "."],
+        "",
+    );
+
+    te.assert_output(
+        &["Foo", "--fixed-strings", "--and", "C", "--and", "."],
+        "one/two/C.Foo2",
+    );
+}
+
+#[test]
+fn test_and_plus_ignore_case() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &["Foo", "--ignore-case", "--and", "C", "--and", "[.]"],
+        "one/two/C.Foo2
+        one/two/c.foo",
+    );
+}
+
+#[test]
+fn test_and_plus_case_sensitive() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &["foo", "--case-sensitive", "--and", "c", "--and", "[.]"],
+        "one/two/c.foo",
+    );
+}
+
+#[test]
+fn test_and_plus_full_path() {
+    let te = TestEnv::new(DEFAULT_DIRS, AND_EXTRA_FILES);
+
+    te.assert_output(
+        &["three", "--full-path", "--and", "foo", "--and", "dir"],
+        "one/two/three/directory_foo/",
+    );
+
+    te.assert_output(
+        &["three", "--full-path", "--and", "two", "--and", "dir"],
+        "one/two/three/directory_foo/",
+    );
+}
+
 /// Test each pattern type with an empty pattern.
 #[test]
 fn test_empty_pattern() {