Skip to content

Don't drop dot-entries for patterns with a literal leading dot - #189

Open
jaideeppyne wants to merge 1 commit into
rust-lang:masterfrom
jaideeppyne:fix/glob-with-literal-leading-dot
Open

Don't drop dot-entries for patterns with a literal leading dot#189
jaideeppyne wants to merge 1 commit into
rust-lang:masterfrom
jaideeppyne:fix/glob-with-literal-leading-dot

Conversation

@jaideeppyne

Copy link
Copy Markdown

glob_with returns nothing for patterns like .git* when require_literal_leading_dot is set, even though .gitignore is sitting right there.

fill_todo drops every directory entry whose name starts with a . before the entry is ever matched against the pattern for that component. But require_literal_leading_dot only asks that the . appear literally in the pattern — and in .git* it does. Pattern::matches_with gets this right (test_pattern_matches_require_literal_leading_dot already covers aaa/bbb/.* vs aaa/bbb/.ccc), so the two APIs disagree about the same component:

// .gitignore exists in the current directory
let opts = MatchOptions { require_literal_leading_dot: true, ..MatchOptions::new() };
Pattern::new(".git*").unwrap().matches_with(".gitignore", opts); // true
glob_with(".git*", opts).unwrap().count();                       // 0

fnmatch(".git*", ".gitignore", FNM_PERIOD) and the shell both match, so Pattern is the one that's right here.

The filter still has a job to do: a recursive ** component matches directory entries without consulting the pattern at all, which is what #128 fixed for #67. So I kept it for every pattern that doesn't start with a literal .. A ** component always parses to a single AnyRecursiveSequence token, so it can never take the new exemption and #67 stays fixed. The predicate is the same one that already guards the . and .. special entries a few lines below, so I hoisted it into a local.

Tests in tests/glob-std.rs cover .a*, .?aa and .[ab]bb matching hidden entries, plus * and ?aa still skipping them. I checked the results against bash on a dot-heavy tree for around 20 patterns and they agree.

I left the .to_str().unwrap() on that line alone since #162 is already dealing with it.

Disclosure: this change was written with AI assistance. The bug was found by differential-testing against fnmatch(3) and bash; I reviewed the diff and reproduced both the bug and the fix locally.

`fill_todo` removes every directory entry whose name starts with a `.`
when `require_literal_leading_dot` is set, before the entry is ever
matched against the pattern for that component. That also removes
entries a pattern is supposed to match: `require_literal_leading_dot`
only asks that the `.` appear literally in the pattern, so `.git*`
should still match `.gitignore`.

`Pattern::matches_with` gets this right (it is covered by
`test_pattern_matches_require_literal_leading_dot`), so `glob_with`
disagreed with matching the same component by hand:

    // .gitignore exists in the current directory
    let opts = MatchOptions {
        require_literal_leading_dot: true,
        ..MatchOptions::new()
    };
    Pattern::new(".git*").unwrap().matches_with(".gitignore", opts); // true
    glob_with(".git*", opts).unwrap().count();                       // 0

The filtering is still needed for a recursive `**` component, which
matches directory entries without consulting the pattern at all, so
keep it for every pattern that does not start with a literal `.`.

Reuse the same "starts with a literal dot" test that already guards the
`.` and `..` special entries a few lines below.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant