fix: match variable-depth ** alongside {placeholder} and excludeFiles globs#58
Open
mliem2k wants to merge 1 commit into
Open
fix: match variable-depth ** alongside {placeholder} and excludeFiles globs#58mliem2k wants to merge 1 commit into
mliem2k wants to merge 1 commit into
Conversation
…excludeFiles globs
Path matching required the pattern and matched path to have the same
number of segments when extracting placeholder values. This made any
`**` segment collapse to matching exactly one directory level whenever
the pattern also contained a `{placeholder}`, silently dropping
candidates at every other depth (vercel-labs#56).
`excludeFiles` entries hit the same equal-segment-count check, so a
`**/`-prefixed entry (as shown in docs/reference/configuration.md)
never matched and the file stayed in scope (vercel-labs#57).
Path segment matching now backtracks through candidate consumption
counts for `**`, so it can reconcile a variable-depth wildcard with
placeholder extraction and with plain excludeFiles glob patterns.
fe0e55a to
294e853
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Fixes #56
Fixes #57
Both issues share the same root cause, per the reproduction steps in each:
tryExtractPlaceholders()insrc/core/path-matcher.tsrequired the pattern and the matched path to have the exact same number of/-separated segments. Since**can legitimately match a variable number of segments, this equal-length check silently rejected any candidate where**didn't happen to consume exactly one segment.**next to a{placeholder}silently matches only one directory level (not all depths) #56:deep/**/{name}.tsagainstdeep/top.ts(depth 0),deep/a/mid.ts(depth 1),deep/a/b/leaf.ts(depth 2) only matched the depth-1 file. The initial glob (viatinyglobby) correctly found all three candidates, but the subsequent placeholder-extraction step dropped the depth-0 and depth-2 ones because their segment counts didn't equal the pattern's.excludeFilespatterns prefixed with**/are silently ignored (docs show**/*.test.tsas valid) #57:excludeFiles: ["**/__test-env-tdd-state.ts"]never excluded anything, because file exclusion falls through to the same kind of segment-count-sensitive matching once glob syntax is involved (bare filenames and full relative paths worked because they only ever went through basename/exact-string comparison, which doesn't care about segment counts).Relevant technical choices
Rather than special-casing or warning about
**next to a placeholder, this implements the "real fix" the reporter suggested: path segment matching (matchPatternSegmentsinpath-matcher.ts) now backtracks through candidate consumption counts when it hits a**pattern segment, trying each possible number of consumed path segments until the rest of the pattern (including any placeholders after the**) also matches. This is a standard backtracking glob-with-captures matcher; it replaces the old index-by-index walk intryExtractPlaceholders.excludeFilesmatching inrunner.ts(isFileExcluded) previously only supported exact-path and basename comparisons — it never actually ran patterns through glob matching, which is why**/-prefixed patterns (and any other wildcard syntax) never worked despitedocs/reference/configuration.mddocumenting"**/*.test.ts"as valid. I added a new exportedmatchesPathPattern()inpath-matcher.ts(reusing the same backtracking segment matcher) and call it fromisFileExcludedas an additional check, soexcludeFilesnow supports the same*,?, and**glob syntax aspaths, while keeping the existing exact-path/basename behavior for plain filenames unchanged.Testing
path-matcher.test.tsreproducing the exact**next to a{placeholder}silently matches only one directory level (not all depths) #56 fixture tree/pattern (all three depths now match with correct placeholder values), a baseline check that plain**without a placeholder still matches every depth, a multi-placeholder-consistency check with**present, and direct tests ofmatchesPathPatterncovering**/-prefixed, trailing-**, and literal patterns.runner.test.tsreproducingexcludeFilespatterns prefixed with**/are silently ignored (docs show**/*.test.tsas valid) #57's exclusion scenario end-to-end throughrun().exclude-filese2e fixture to exclude via a**/-prefixed pattern instead of a full relative path, exercising the realtinyglobby-backed CLI path; confirmed (by temporarily reverting the source fix) that this fixture fails without the fix and passes with it.pnpm test,pnpm typecheck,pnpm check,pnpm test:e2e) passes.Checklist
pnpm changesetin the project root if package files were modified)