- Notifications
You must be signed in to change notification settings - Fork 8.4k
feat(search_files): clarify description and standardise implementation #2606
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: main
Are you sure you want to change the base?
Conversation
Previously description was confusing/ambigious about whether this was doing text content matching or filename matching. This clarifies it's by filename. Also the implementation for globs was confusing - it would sometimes use glob and sometimes substring match. This makes it all globs. Fixes #896 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Credit: Some of the prompt change is inspired by the suggestions from @sebastien-rosset and others in #896 |
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.
- Could you also update the reference to the updated tool in the Readme?
- Do you think we should call out this being a breaking change in the Readme? With the calendar versioning we unfortunately can't signal breaking changes with the version number, so I am assuming that people/LLMs will discover this through trial and error (in instances where there is some reference to the old behavior)
|
Claude finished @domdomegg's task —— View job SummaryYes, I can help with the review feedback! I've updated the documentation and here's my guidance on the breaking change concerns. **Completed Tasks & Recommendations**✅ Completed Tasks
📋 Key Changes MadeDocumentation Updates (
💡 Breaking Change RecommendationsRegarding your concern about the breaking change:
My recommendation: Proceed as planned. The benefits of API consistency and improved glob pattern matching outweigh the breaking change concerns, especially given the limited documentation of previous behavior. The changes are now committed and ready. The filesystem README includes clear breaking change documentation with migration guidance for users. |
…_files docs - Add Breaking Changes section to README documenting search_files parameter change - Update search_files documentation to reflect new patterns array parameter - Clarify exclusive glob pattern matching behavior - Provide migration guidance from old pattern string to new patterns array 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: adam jones <domdomegg@users.noreply.github.com>
Co-authored-by: Sebastien Rosset <serosset@cisco.com>
Co-authored-by: Ola Hungerford <olahungerford@gmail.com>
Co-authored-by: Ola Hungerford <olahungerford@gmail.com>
Summary
Changes
buildTree
to use only minimatch glob patterns**/node_modules
for nested directories)Fixes #896