diff --git a/CHANGELOG.md b/CHANGELOG.md index e4b013bb..6b6a5b49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## Bugfixes - Handle invalid working directories gracefully when using `--full-path`, see #1900 (@Xavrir). +- Fire the "search pattern contains a path separator" diagnostic for any pattern containing `/`, not just patterns that happen to name an existing directory. Preserves the legacy Windows behaviour that also flags native `\` separators when the pattern resolves to a real directory. See #1873. # 10.4.2 diff --git a/src/main.rs b/src/main.rs index 8b7bd936..f6c0d7e4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -145,21 +145,56 @@ fn set_working_dir(opts: &Opts) -> Result<()> { Ok(()) } -/// Detect if the user accidentally supplied a path instead of a search pattern +/// Detect if the user accidentally supplied a path instead of a search pattern. +/// +/// Without `--full-path`, fd matches patterns against file names, so any pattern +/// containing a path separator can never match. Two cases are worth a friendly +/// error rather than silent "no results": +/// +/// 1. The pattern contains '/'. '/' is always a path separator (including on +/// Windows) and has no regex meaning, so flagging it is safe and catches the +/// common Linux/macOS mistake of pasting a full path as the pattern. +/// 2. On Windows only, the pattern contains the native `\` separator *and* +/// names an existing directory on disk. We can't treat `\` as a pure +/// path-separator signal there because it is also the regex escape char, +/// so valid regex patterns like `\Ac` or `\d+` must still run. Requiring +/// that the pattern resolves to a real directory avoids those false +/// positives while preserving the legacy diagnostic for operators who +/// literally typed a directory path. +/// +/// See https://github.com/sharkdp/fd/issues/1873. fn ensure_search_pattern_is_not_a_path(opts: &Opts) -> Result<()> { - if !opts.full_path - && opts.pattern.contains(std::path::MAIN_SEPARATOR) - && Path::new(&opts.pattern).is_dir() + if opts.full_path { + return Ok(()); + } + + // Start with the cheap check: '/' is always a path separator, including on + // Windows, and has no regex meaning, so flagging it is safe and catches the + // Linux/macOS mistake of pasting a full path as the pattern. + #[cfg_attr(not(windows), allow(unused_mut))] + let mut should_warn = opts.pattern.contains('/'); + + // On Windows we additionally accept the native `\` separator, but only when + // the pattern actually resolves to an existing directory - `\` is also the + // regex escape char there, so valid patterns like `\Ac` or `\d+` must still + // run. The is_dir syscall is only needed when `should_warn` is still false, + // so short-circuit via `||` to avoid the stat call on the happy path. + #[cfg(windows)] { + should_warn = should_warn + || (opts.pattern.contains(std::path::MAIN_SEPARATOR) + && Path::new(&opts.pattern).is_dir()); + } + + if should_warn { Err(anyhow!( - "The search pattern '{pattern}' contains a path-separation character ('{sep}') \ + "The search pattern '{pattern}' contains a path-separation character \ and will not lead to any search results.\n\n\ If you want to search for all files inside the '{pattern}' directory, use a match-all pattern:\n\n \ fd . '{pattern}'\n\n\ Instead, if you want your pattern to match the full file path, use:\n\n \ fd --full-path '{pattern}'", pattern = &opts.pattern, - sep = std::path::MAIN_SEPARATOR, )) } else { Ok(()) diff --git a/tests/tests.rs b/tests/tests.rs index c125d3a5..aa0919ca 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -374,6 +374,49 @@ fn test_multi_file_with_missing() { ); } +/// Without --full-path, a pattern containing '/' should always produce the +/// path-separator diagnostic, even if the pattern does not name an existing +/// directory. Before the fix for sharkdp/fd#1873 this only fired when the +/// pattern happened to resolve to a real directory, so the common typo of +/// pasting a full path silently returned zero matches. +#[test] +fn test_pattern_with_forward_slash_is_rejected() { + let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES); + + // Pattern that is NOT a real directory; old behaviour: no warning. + te.assert_failure_with_error( + &["nonexistent/path"], + "[fd error]: The search pattern 'nonexistent/path' contains a path-separation character and will not lead to any search results.", + ); + + // Pattern that IS a real directory; old behaviour: warning. Must still fire. + te.assert_failure_with_error( + &["one/two/three"], + "[fd error]: The search pattern 'one/two/three' contains a path-separation character and will not lead to any search results.", + ); +} + +/// --full-path is the user's explicit opt-in to regex-over-full-path matching, +/// so a path-separation character in the pattern is expected and must not +/// trigger the diagnostic. +/// +/// Gated off Windows: the actual match is regex-over-the-full-path, so a +/// forward-slash pattern only matches Unix-style paths. On Windows the OS +/// uses `\` and `one/two/c` (as a literal regex) does not match a real +/// entry — the behaviour this test is pinning (the diagnostic does not +/// fire) is covered by the fact that the invocation does not error. +#[test] +#[cfg(not(windows))] +fn test_pattern_with_forward_slash_allowed_with_full_path() { + let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES); + + te.assert_output( + &["--full-path", "one/two/c"], + "one/two/c.foo + one/two/C.Foo2", + ); +} + /// Explicit root path #[test] fn test_explicit_root_path() {