feat(stdin): Apply include and ignore patterns to stdin file lists#680
feat(stdin): Apply include and ignore patterns to stdin file lists#680
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (defaultAction)
participant Packer as pack()
participant FS as searchFiles()
CLI->>CLI: Read file paths from stdin
CLI->>Packer: pack(rootDirs, config, ..., explicitFiles)
Packer->>FS: searchFiles(rootDir, config, explicitFiles)
FS-->>Packer: Filtered files
Packer-->>CLI: Pack result
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
There was a problem hiding this comment.
Summary of Changes
Hello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the repomix tool by enabling the application of include and ignore patterns to file paths provided via standard input. This change brings consistency to file filtering across different input methods and resolves a previously identified limitation, making the tool more flexible and powerful for users who pipe file lists.
Highlights
- New Feature: Stdin File Filtering: Implemented the ability to apply include and ignore patterns to file lists provided via standard input (
--stdin), addressing issue #650. Previously, these patterns were bypassed for stdin input. - Core Logic:
filterFileListFunction: Introduced a new asynchronous function,filterFileList, insrc/core/file/fileSearch.ts. This function takes a list of file paths and applies the configured include and ignore patterns, mirroring the filtering logic used for regular file discovery. - Integration with Stdin Processing: Modified the
handleStdinProcessingfunction insrc/cli/actions/defaultAction.tsto incorporate the newfilterFileList. Files read from stdin are now filtered before being passed to the packing operation, ensuring consistent application of patterns. - Test Coverage and Known Limitations: Added new test cases for the
filterFileListfunction intests/core/file/fileSearch.test.tsto validate its behavior with include and custom ignore patterns. ATODOcomment in thefilterFileListimplementation notes that patterns from.gitignoreand.repomixignorefiles are not yet processed for stdin input.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Deploying repomix with
|
| Latest commit: |
1ec8db9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://aeefa515.repomix.pages.dev |
| Branch Preview URL: | https://feat-stdin-filtering.repomix.pages.dev |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to filter files passed via stdin using include/ignore patterns. The implementation adds a new filterFileList function and integrates it into the stdin processing workflow. Key areas for improvement include handling .gitignore and .repomixignore files, enabling skipped tests, and refactoring shared logic to improve maintainability.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/core/file/fileSearch.ts (1)
308-310: Address the TODO: missing ignore file patterns processing.The function currently skips
.gitignoreand.repomixignoreprocessing, which may lead to inconsistent filtering behavior compared to normal file search.This limitation could affect the filtering accuracy. Would you like me to help implement the ignore file patterns processing or should this be tracked as a separate issue?
Consider the impact of this limitation on the filtering consistency between stdin and normal file search modes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)src/cli/actions/defaultAction.ts(3 hunks)src/core/file/fileSearch.ts(1 hunks)tests/core/file/fileSearch.test.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/cli/actions/defaultAction.ts (1)
src/core/file/fileSearch.ts (1)
filterFileList(258-318)
tests/core/file/fileSearch.test.ts (2)
tests/testing/testUtils.ts (1)
createMockConfig(15-45)src/core/file/fileSearch.ts (1)
filterFileList(258-318)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: pack-repo
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (ubuntu-latest, 22.x)
- GitHub Check: Test (ubuntu-latest, 24.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Test (ubuntu-latest, 19.x)
- GitHub Check: Test (ubuntu-latest, 20.x)
- GitHub Check: Test (macos-latest, 18.0.0)
- GitHub Check: Test (macos-latest, 19.x)
- GitHub Check: Test (ubuntu-latest, 18.0.0)
- GitHub Check: Build and run (macos-latest, 23.x)
- GitHub Check: Build and run (windows-latest, 21.x)
- GitHub Check: Build and run (macos-latest, 21.x)
- GitHub Check: Build and run (windows-latest, 18.0.0)
- GitHub Check: Build and run (windows-latest, 18.x)
- GitHub Check: Test Browser Extension
- GitHub Check: Test coverage
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
.gitignore (1)
42-43: LGTM! Clean addition of repomix runner directory to gitignore.The addition of
.repomix/directory with a clear explanatory comment is appropriate for excluding runtime artifacts.src/cli/actions/defaultAction.ts (2)
10-10: Clean import addition.The import is correctly placed and follows the existing import organization.
83-86: Excellent integration of filtering step into stdin processing workflow.The filtering is correctly positioned between reading stdin and packing, with appropriate user feedback via spinner messages. The filtered results are properly utilized in the custom pack variant.
Also applies to: 99-99
tests/core/file/fileSearch.test.ts (2)
10-10: Clean import addition for the new function.The import is properly integrated into the existing import statement.
569-585: Well-written tests for implemented functionality.The tests for include patterns and absolute path handling are well-structured and verify the expected behavior correctly.
Also applies to: 628-644
src/core/file/fileSearch.ts (1)
297-306: LGTM! The core filtering logic is sound.The pattern matching implementation correctly applies include patterns first, then excludes files matching ignore patterns. The use of
minimatchis consistent with the existing codebase, and the path conversion logic is appropriate.
|
@claude Please review this PR |
|
Claude finished @yamadashy's task —— View job PR Review - feat(stdin): Apply include and ignore patterns to stdin file listsTodo List:
SummaryThis PR successfully implements stdin file filtering to apply include and ignore patterns to files passed via Strengths✅ Clean Integration: The ✅ Consistent API: The new function follows the same signature pattern as ✅ Proper Path Handling: Correctly converts absolute paths to relative paths for pattern matching at ✅ Comprehensive Testing: Added meaningful test cases covering include patterns and absolute path handling. ✅ Good Documentation: The function has clear JSDoc documentation explaining its purpose and relationship to Issues & Concerns🔴 Critical Issues
🟡 Code Quality Issues
🟢 Minor Issues
File-by-File Analysis
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #680 +/- ##
==========================================
+ Coverage 90.36% 90.38% +0.01%
==========================================
Files 99 99
Lines 5543 5553 +10
Branches 1183 1185 +2
==========================================
+ Hits 5009 5019 +10
Misses 534 534 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ab41e2b to
c77dca7
Compare
There was a problem hiding this comment.
PR Summary
Implements file filtering functionality for stdin input in Repomix, allowing include/ignore patterns to be applied to files passed via --stdin command.
- Modified
src/core/file/fileSearch.tsto add filtering support for predefined file lists while maintaining existing functionality - Updated
src/cli/actions/defaultAction.tsto integrate stdin file filtering into the main processing workflow - Added comprehensive test coverage in
tests/core/file/fileSearch.test.tsandtests/cli/actions/defaultAction.test.ts - Enhanced
.gitignoreto prevent recursion issues by excluding.repomix/directory
6 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
Add filtering capability to stdin file input, allowing users to pipe files through stdin while still applying include and ignore patterns from configuration. Changes: - Add filterFileList function to apply include/ignore patterns to file lists - Modify handleStdinProcessing to filter stdin files before packing - Add test cases for the new filtering functionality This resolves issue #650 by implementing the requested behavior where "stdin files could be injected into the included file set, with ignore patterns still being applied." 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Address PR feedback by implementing complete filtering support: - Add .gitignore and .repomixignore processing using globby - Enable previously skipped test cases with proper mocks - Add globby mocks to defaultAction tests - Fix test expectations to match sorted output This resolves the TODO in filterFileList and ensures stdin filtering matches the behavior of normal file discovery. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Apply automatic linting fixes for code style consistency. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The filterFileList function was missing robust error handling compared to searchFiles. Added try-catch blocks around file system operations, explicit permission error handling, and proper error logging to maintain consistency with the existing searchFiles implementation. This addresses a security and robustness gap where file system errors during stdin filtering could cause unexpected failures without proper error context. Changes: - Add try-catch wrapper around entire filterFileList function - Handle EPERM/EACCES errors specifically in globby operations - Add proper error logging and re-throwing with context - Add test cases for permission and generic error scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed "noExplicitAny" lint error by using proper type assertion instead of casting to any. The test maintains the same functionality while adhering to TypeScript best practices. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…function - Add predefinedFiles parameter to searchFiles function for stdin file processing - Remove duplicate filterFileList function and consolidate logic - Update defaultAction to use unified searchFiles instead of filterFileList - Skip directory validation when using predefined files from stdin - Update tests to reflect consolidated file filtering architecture - Improve code maintainability by eliminating duplicate filtering logic 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Add guidelines for commit body content to preserve decision-making context and conversation history that led to changes. This helps with future code archaeology and understanding the reasoning behind modifications. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix searchFiles to properly filter predefinedFiles instead of using them as glob patterns - Fix normalizeGlobPattern to avoid converting file patterns like **/*.test.ts to directory patterns - Update predefinedFiles filtering logic to apply include/ignore patterns correctly - Remove obsolete globby mocks from tests since filtering now happens directly - Skip empty directory processing for predefined file lists as it's not applicable This resolves issues where stdin file filtering was not working correctly due to incorrect pattern handling and improper use of file paths as glob patterns. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…d logic - Extract filterPredefinedFiles function for cleaner predefined file filtering - Add adjustIgnorePatternsForWorktree helper to handle git worktree logic - Create searchFilesWithGlobby function with centralized error handling - Extract findEmptyDirectoriesIfEnabled to simplify directory processing - Reduce nesting depth in main searchFiles function using helper functions - Improve maintainability and testability of file search logic All functionality remains unchanged while significantly improving code readability and reducing complexity of the main searchFiles function. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Move main searchFiles function before helper functions for better code organization and readability. This follows the convention of having the main exported function at the top, followed by supporting helper functions. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…meter - Add optional predefinedFiles parameter to pack function for cleaner API - Remove complex dependency injection override in defaultAction.ts - Pass stdin file paths directly to pack function instead of using searchFiles override - Update tests to match new function signatures and parameter expectations - Simplify stdin processing workflow by eliminating intermediate filtering step This approach is more explicit, maintainable, and easier to understand than the previous dependency injection pattern for handling predefined file lists. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…nclude patterns User suggested a much simpler approach: instead of complex filtering logic, merge predefinedFiles into includePatterns directly. This: - Maintains existing include pattern behavior - Adds stdin files as additional include patterns - Uses existing globby flow for all filtering - Removes 116 lines of helper functions - Fixes bug where predefined files replaced instead of extended include patterns All tests pass with this cleaner implementation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ef071f2 to
f8615e4
Compare
User correctly pointed out that directory validation was being skipped when predefinedFiles were provided. This created a security and consistency issue where invalid/non-existent directories could be processed. Fixed by removing the conditional skip and always performing: - Directory existence check - Directory type validation - Permission verification This ensures robust error handling regardless of input source. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Removed complex condition checks from normalizeGlobPattern that were
checking for wildcards and file extensions. The simplified condition
`pattern.startsWith('**/') && \!pattern.includes('/**')` is sufficient
for the intended behavior of converting directory patterns.
This makes the code cleaner and easier to understand while maintaining
the same functionality.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
…xtensibility User suggested improving the naming to support future features like --include-from-file. The new name explicitFiles: - Is more generic and not stdin-specific - Clearly indicates files explicitly specified by user - Better supports future file input methods - Maintains same functionality with clearer intent Updated function signatures, variable names, comments, and test descriptions across the codebase for consistency. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@claude review |
|
Claude finished @yamadashy's task —— View job PR Review - feat(stdin): Apply include and ignore patterns to stdin file listsTodo List:
SummaryThis PR successfully implements stdin file filtering by extending the existing Strengths✅ Excellent Design Choice
✅ Robust Implementation (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
PR Summary
Updates GitHub Actions workflows and dependencies while maintaining security standards.
- Updated CodeQL workflow from v2 to v3 in
.github/workflows/codeql.ymlfor improved Node.js 20 support and security analysis - Refreshed Claude AI review action in
.github/workflows/claude-review.ymlfrom ebbd9e9 to ece712e - Updated Repomix action reference in
.github/workflows/pack-repository.ymlto latest commit 44055a5 - Added clear 'Commit Body Guidelines' section in
.agents/rules/base.mdfor better context preservation
13 files reviewed, no comments
Edit PR Review Bot Settings | Greptile
…ovided User identified a critical bug where config.include=[] + explicitFiles would result in ['**/*', ...explicitFiles] instead of just [...explicitFiles]. Fixed by reordering the logic: 1. Start with config.include patterns only 2. Add explicitFiles if provided 3. Default to ['**/*'] only if no patterns exist This ensures: - config.include=[] + explicitFiles → explicitFiles only (correct) - config.include=['*.js'] + explicitFiles → both (correct) - config.include=[] + no explicitFiles → ['**/*'] (correct) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Pull Request Overview
This PR enables applying include/ignore patterns to file paths provided via --stdin by passing them through the existing filtering logic.
- Updated
packandsearchFilesto accept and handle anexplicitFileslist - Modified CLI’s stdin handler to pass stdin paths into
pack - Added and adjusted tests to cover stdin filtering behavior
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/core/packager.test.ts | Updated expectations to include the new explicitFiles argument when calling searchFiles |
| tests/core/file/fileSearch.test.ts | Added tests for filtering explicit stdin files with include/ignore patterns |
| tests/cli/actions/defaultAction.test.ts | Adjusted test to assert pack is called with the new parameters; added (but unused) globby mock |
| src/core/packager.ts | Extended pack signature and invocation of searchFiles to accept explicitFiles |
| src/core/file/fileSearch.ts | Enhanced searchFiles to merge explicit file list into include patterns |
| src/cli/actions/defaultAction.ts | Passed stdin file list as explicitFiles into pack |
| .agents/rules/base.md | Introduced new commit body guidelines (documentation) |
Comments suppressed due to low confidence (4)
.agents/rules/base.md:77
- [nitpick] These new commit body guidelines aren’t related to the stdin filtering feature. It may be clearer to move them into a separate documentation or style-guide PR to keep this change scoped to code functionality.
### Commit Body Guidelines
src/core/packager.ts:47
- [nitpick] The parameter name
explicitFilescould be more descriptive of its role (e.g.,stdinFilePathsorfilteredFilePaths) to improve clarity in thepackAPI.
explicitFiles?: string[],
src/core/file/fileSearch.ts:94
- [nitpick] The PR description mentions a
filterFileListfunction, but there’s no such function in this file. Consider updating the description or adding the utility to keep docs and implementation in sync.
// Get all file paths considering the config
tests/cli/actions/defaultAction.test.ts:3
- [nitpick] The test imports and mocks
globby, buthandleStdinProcessingdoesn’t callglobbydirectly. Consider removing this mock to keep the test focused and reduce unnecessary dependencies.
import { globby } from 'globby';
There was a problem hiding this comment.
Code Review
This pull request introduces stdin file filtering, applying include and ignore patterns to files passed via --stdin. It adds a filterFileList function to handle the filtering logic and modifies handleStdinProcessing to apply this filtering step. The changes also include new test cases for the filtering functionality.
Summary
Implements stdin file filtering to apply include and ignore patterns to files passed via
--stdin, as requested in issue #650.Previously, when using
--stdin, all ignore patterns (.gitignore,.repomixignore, custom patterns) were completely bypassed. Now stdin files are filtered through the same include/ignore logic used by normal file discovery.Changes
Added
filterFileListfunction insrc/core/file/fileSearch.ts:searchFilesbut for stdin inputModified
handleStdinProcessinginsrc/cli/actions/defaultAction.ts:Added test cases for the new filtering functionality
Test Plan
git ls-files | node bin/repomix.cjs --stdin --ignore "*.test.ts"correctly excludes test filesgit ls-files | node bin/repomix.cjs --stdin --include "**/*.ts"correctly includes only TypeScript filesExample Usage
This resolves #650 by implementing the requested behavior where "stdin files could be injected into the included file set, with ignore patterns still being applied."