Fix/support parentheses in paths#337
Conversation
|
Warning Rate limit exceeded@yamadashy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request introduces a new function, Sequence Diagram(s)sequenceDiagram
participant Caller
participant searchFiles
participant escapeGlobPattern
Caller->>searchFiles: Invoke searchFiles(config)
searchFiles->>escapeGlobPattern: For each pattern in config.include
escapeGlobPattern-->>searchFiles: Return escaped pattern
searchFiles->>Caller: Return searched file list
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/file/fileSearch.ts (1)
60-66: Consider enhancing the glob pattern escaping.While the current implementation handles parentheses and brackets, consider extending it to handle other special glob characters like
*,?,!,+,@, and!.Apply this diff to enhance the function:
const escapeGlobPattern = (pattern: string): string => { - return pattern.replace(/[()[\]{}]/g, '\\$&'); + if (typeof pattern !== 'string') { + throw new TypeError('Pattern must be a string'); + } + return pattern.replace(/[()[\]{}*?!+@]/g, '\\$&'); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/file/fileSearch.ts(2 hunks)
🔇 Additional comments (1)
src/core/file/fileSearch.ts (1)
80-81: LGTM!The changes correctly apply the
escapeGlobPatternfunction to each include pattern while maintaining the default behavior.
- Add escapeGlobPattern function to handle special characters
- Modify searchFiles to escape include patterns
- Handle (), [], and {} characters in paths
yamadashy
force-pushed
the
fix/support-parentheses-in-paths
branch
from
317bd4d to
aefed00
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #337 +/- ##
==========================================
+ Coverage 90.36% 90.38% +0.01%
==========================================
Files 48 48
Lines 2626 2631 +5
Branches 541 542 +1
==========================================
+ Hits 2373 2378 +5
Misses 253 253 ☔ View full report in Codecov by Sentry. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * Example: "src/(categories)" -> "src/\\(categories\\)" | ||
| */ | ||
| const escapeGlobPattern = (pattern: string): string => { | ||
| return pattern.replace(/[()[\]{}]/g, '\\$&'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/core/file/fileSearch.test.ts (1)
347-376: Great test coverage! Consider adding a few more edge cases.The test suite is well-structured and covers essential scenarios including basic escaping, multiple bracket types, nested brackets, and edge cases.
Consider adding test cases for:
- Invalid inputs (e.g., unmatched brackets)
- Paths with spaces (e.g.,
'src/(my folder)/**/*.ts')Example test cases:
test('should handle paths with spaces', () => { const pattern = 'src/(my folder)/**/*.ts'; expect(escapeGlobPattern(pattern)).toBe('src/\\(my folder\\)/**/*.ts'); }); test('should handle unmatched brackets', () => { const pattern = 'src/(incomplete/**/*.ts'; expect(escapeGlobPattern(pattern)).toBe('src/\\(incomplete/**/*.ts'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/file/fileSearch.ts(2 hunks)tests/core/file/fileSearch.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/file/fileSearch.ts
🔇 Additional comments (1)
tests/core/file/fileSearch.test.ts (1)
8-14: LGTM!The import of
escapeGlobPatternis properly organized with related functions.
|
Hi, @matheuscoelhomalta ! The code with the added test cases looks perfect! I'll merge this now. |
|
I've made a change to prevent escaping curly braces {} in glob patterns, as it was causing issues with pattern matching. |
2 participants
This PR adds support for paths containing special characters, specifically parentheses, in the
includepatterns. Previously, paths with characters like()were not correctly matched by the glob pattern matcher.Example Use Case
When working with frameworks like Next.js, developers often use route groups. The following include pattern will now work as expected:
{
"include": ["src/(categories)/**/*"]
}
Changes
escapeGlobPatternfunction:searchFilesmodification:Special characters handled:
()(parentheses)[](square brackets){}(curly brackets)Checklist
npm run testand ensure all tests passnpm run lintand resolve any linting issues