Skip to content

Glob patterns **/p* incorrectly match on /foo/ap #144458

Description

VSCode is implement its own solution for glob pattern matching and does not rely on a 3rd party module. The reasons are mostly historic but our glob patterns also behave slightly different compared to others (e.g. **/foo.txt matches on foo.txt, both backslash and slash are supported).

Our glob matching works in 2 stages:

  • if it matches a typical pattern (such as **/*.js), we optimize the matching to avoid a regex use (from here)
  • otherwise we try to convert the pattern into a single regex to match (from here)

Specifically for the conversion of ** to a regular expression we have a flaw that results in the issue at hand here, the regex is:

// Matches: (Path Sep OR Path Val followed by Path Sep OR Path Sep followed by Path Val) 0-many times
// Group is non capturing because we don't need to capture at all (?:...)
// Overall we use non-greedy matching because it could be that we match too much
return `(?:${PATH_REGEX}|${NO_PATH_REGEX}+${PATH_REGEX}|${PATH_REGEX}${NO_PATH_REGEX}+)*?`;

Or specifically for **/p* ends up to be:

/^(?:[/\\]|[^/\\]+[/\\]|[/\\][^/\\]+)*?p[^/\\]*?$/

Unfortunately, given a string of /foo/ap, the segment [/\\][^/\\]+ eagerly matches on /a and as such, the overall result is a match, even though that is unexpected.

Help wanted
Opening up for others to contribute, the solution should:

  • preserve our tests for globs to succeed (glob.test.ts)
  • enable the now skipped test to succeed (code)
  • somewhat preserve our strategy for computing a single regex for matching [1]

[1] however if this is not possible, I am also open for ideas how to solve this differently if the solution is not overly complex.

Activity

  1. added
    bugIssue identified by VS Code Team member as probable bug
    help wantedIssues identified as good community contribution opportunities
    good first issueIssues identified as good for first-time contributors
    on Mar 5, 2022
  2. changed the title [-]Glob patterns `**/p*` incorrectly match on `p` anywhere in path[/-] [+]Glob patterns `**/p*` incorrectly match on `/foo/ap` anywhere in path[/+] on Mar 5, 2022
  3. changed the title [-]Glob patterns `**/p*` incorrectly match on `/foo/ap` anywhere in path[/-] [+]Glob patterns `**/p*` incorrectly match on `/foo/ap`[/+] on Mar 5, 2022
  4. added 2 commits that reference this issue on Mar 5, 2022
    60f8b58
    051c386
  5. jeanp413 commented on Mar 5, 2022

    @jeanp413
    Contributor

    Created a PR #144473 with a possible fix

  6. added a commit that references this issue on Mar 6, 2022
    e80a0ca
  7. added
    unreleasedPatch has not yet been released in VS Code Insiders
    insiders-releasedPatch has been released in VS Code Insiders
    and removed
    unreleasedPatch has not yet been released in VS Code Insiders
    on Mar 6, 2022
  8. locked and limited conversation to collaborators on Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugIssue identified by VS Code Team member as probable bugfile-globFile glob enginegood first issueIssues identified as good for first-time contributorshelp wantedIssues identified as good community contribution opportunitiesinsiders-releasedPatch has been released in VS Code InsidersverifiedVerification succeeded

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions