Skip to content

fix: let a middle ** in skipFiles match zero directories - #2424

Merged
Connor Peet (connor4312) merged 1 commit into
microsoft:mainfrom
NgoQuocViet2001:fix-skipfiles-middle-globstar
Oct 5, 2026
Merged

Connor Peet (connor4312) merged 1 commit into
microsoft:mainfrom
NgoQuocViet2001:fix-skipfiles-middle-globstar

Conversation

@NgoQuocViet2001

Copy link
Copy Markdown
Contributor

A ** in the middle of a skipFiles glob currently has to match at least one directory. globToRe turns it into .*/, and the segment before it already ends with \/. So with "skipFiles": ["${workspaceFolder}/lib/**/*.js"], lib/sub/index.js is skipped but lib/index.js is not, and ${workspaceFolder}/**/node_modules/** misses the top-level node_modules. Negations are affected the same way: with ["**/foo/**", "!**/foo/**/bar/**"], foo/bar/baz stays skipped. This came in with #1470. Before that, a middle ** could match zero directories.

This change makes the middle case (.*/)?, the same shape as the leading (.+/)?, so it matches zero or more directories without bringing back a nested quantifier. Possibly related: microsoft/vscode#204173 (!**/webpack-internal:///**/packages/** not un-skipping webpack-internal:///packages/...).

  • Added two truth-table cases to simpleGlobToRe.test.ts. Both fail on main and pass with this change, and their "is not catastrophic" checks pass.
  • Ran: npm run test:unit. The only difference from main is the 4 new passing tests.
  • Ran: npm run test:types, dprint check and eslint --max-warnings=0 on the touched files, all clean.

A `**` segment in the middle of a skipFiles glob compiled to `.*/`,
which needs at least one extra directory. So `${workspaceFolder}/lib/**/*.js`
did not skip `lib/index.js`, and a negation like `!**/foo/**/bar/**` did
not un-skip `foo/bar/...`. Make that part optional so it matches zero or
more directories, like the leading `**` already does.
@connor4312
Connor Peet (connor4312) merged commit e7cfef4 into microsoft:main Oct 5, 2026
6 checks passed
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.

4 participants