-
-
Notifications
You must be signed in to change notification settings - Fork 219
Fix ignore patterns not working in configuration files #744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix ignore patterns not working in configuration files #744
Conversation
Fixed issue kucherenko#496 where ignore patterns specified in .jscpd.json or package.json were not being applied correctly. The root cause was that ignore patterns were not being resolved relative to the configuration file location, causing fast-glob to interpret them incorrectly. This change adds a resolveIgnorePattern() function that: - Preserves absolute paths unchanged - Preserves **/ patterns (which match at any depth) - Converts relative patterns to be relative to process.cwd() Also explicitly set cwd in fast-glob options to ensure consistent path resolution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
WalkthroughNormalize jscpd ignore and path entries by resolving patterns relative to the configuration directory and add deterministic glob options (explicit cwd and boolean for absolute) for file discovery behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/jscpd/src/options.ts(3 hunks)packages/finder/src/files.ts(1 hunks)
🔇 Additional comments (3)
packages/finder/src/files.ts (1)
101-103: LGTM! Improved determinism for glob resolution.The explicit
|| falsedefault forabsoluteand the explicitcwdsetting make path resolution more predictable and align with the config-relative ignore pattern handling introduced in this PR.apps/jscpd/src/options.ts (2)
94-100: Config path resolution looks correct.The change from
dirname(config)todirname(configFile)is important—it ensures paths are resolved against the absolute config location rather than the potentially-relative input parameter. The ignore pattern transformation correctly delegates toresolveIgnorePattern, though that function needs the substring bug fix noted separately.
110-116: Package.json config resolution matches the .jscpd.json pattern.Consistent application of the same path and ignore pattern resolution logic. Once
resolveIgnorePatternis fixed, this will work correctly.
Fixed critical bug where absolutePattern.startsWith(cwd) could incorrectly match sibling directories with similar prefixes (e.g., '/project' matching '/projectdata'). Replaced string prefix check with path.relative() for proper path comparison. Now correctly checks if a path is under cwd by verifying the relative path doesn't start with '..'. All existing tests pass (82 tests total). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Summary
Changes
apps/jscpd/src/options.ts: Added resolveIgnorePattern() function and applied to ignore patterns in readConfigJson() and readPackageJsonConfig()packages/finder/src/files.ts: Explicitly set cwd in fast-glob options for consistent path resolutionTesting
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.