dofs: Add exclusions to recursive grep - #150
Merged
Merged
Conversation
find could prune directories from a traversal but grep could not, so there was no way to search a workspace while skipping node_modules -- the thing callers most often want to skip. A caller could pass include to name what they wanted, but not exclude to name what they did not. grep already delegates its traversal to find's walker via iterateFoundEntries, which has accepted exclusion globs since find gained them. So this passes them through rather than reimplementing matching: an excluded directory is pruned before its children are queried, instead of being walked and read with its matches discarded afterwards. Semantics follow find exactly, because sharing the walker means sharing its rules: globs match the directory-relative path, exclusion is applied before inclusion so it always wins, and a subtree needs both forms (node_modules and node_modules/**) since /** matches what is below a directory rather than the directory itself. A single-file search ignores exclude -- the caller named the file and there is no traversal to prune. The pruning test asserts the query count rather than just the results, because filtering after the walk would produce identical matches while still paying for the excluded tree. Writing it first with only 'vendor/**' showed the walker descending and excluding each child, which is what pinned the both-forms requirement. Also threads exclude through the computer-level grep tool, matching the option find's tool already exposes.
🦋 Changeset detectedLatest commit: 301ada1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
commit: |
Review caught two places the new option was missing from contracts a consumer actually reads. tools/fs/store.ts declares WorkspaceLike by inlining the grep options rather than importing GrepOptions, so it kept typechecking while silently not admitting exclude. A caller holding that structural type could not pass the option the tool now accepts. docs/09_tool_interface.md documents exclude under find but not grep, so the two tools read as if only one supports pruning. Adds the field and states the semantics it shares with find, including the both-forms requirement. Also drops PR.md, which was a description for review rather than repository content. Note: the same inlined WorkspaceLike omits exclude from find too, which predates this change and is left alone here.
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
1 participant
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.
Add exclude to grep.
Combine with
include— exclusion is applied first, so it always wins:Several subtrees at once:
Same option on the agent-facing tool, matching the one
findalready exposes:{ "path": "/workspace", "query": "TODO", "include": "**/*.ts", "exclude": ["node_modules/**"] }