Feature: Add --force-include option #480
Conversation
ktyubeshi
force-pushed
the
feature/forceinclude
branch
from
f8bb98a to
ffef120
Compare
|
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 WalkthroughA new "force include" feature was introduced, allowing users to specify files or patterns that should always be included in file searches, overriding any ignore rules. This is implemented via a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ConfigLoader
participant FileSearch
User->>CLI: Run with --force-include pattern(s)
CLI->>ConfigLoader: Build config (merge CLI/config file)
ConfigLoader->>CLI: Return config with forceInclude
CLI->>FileSearch: searchFiles(config)
FileSearch->>FileSearch: Search with include/ignore patterns
FileSearch->>FileSearch: Search with forceInclude patterns (no ignores)
FileSearch->>CLI: Return combined file list (forceInclude overrides ignores)
Possibly related issues
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
README.md(1 hunks)src/cli/actions/defaultAction.ts(1 hunks)src/cli/cliRun.ts(1 hunks)src/cli/types.ts(1 hunks)src/config/configLoad.ts(1 hunks)src/config/configSchema.ts(2 hunks)src/core/file/fileSearch.ts(4 hunks)tests/cli/actions/defaultAction.test.ts(1 hunks)tests/cli/cliRun.test.ts(2 hunks)tests/config/configSchema.test.ts(2 hunks)tests/core/file/fileSearch.test.ts(1 hunks)tests/core/treeSitter/parseFile.solidity.test.ts(1 hunks)tests/mcp/tools/packCodebaseTool.test.ts(1 hunks)tests/testing/testUtils.ts(1 hunks)website/client/src/en/guide/command-line-options.md(2 hunks)website/client/src/en/guide/configuration.md(3 hunks)website/client/src/ja/guide/command-line-options.md(2 hunks)website/client/src/ja/guide/configuration.md(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/core/file/fileSearch.test.ts (1)
src/core/file/fileSearch.ts (1)
searchFiles(94-218)
🪛 markdownlint-cli2 (0.17.2)
website/client/src/en/guide/configuration.md
62-62: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
82-82: Files should end with a single newline character
null
(MD047, single-trailing-newline)
🔇 Additional comments (29)
src/cli/types.ts (1)
26-26: LGTM: Properly implemented new CLI option.The
forceIncludeproperty has been correctly added to theCliOptionsinterface with the appropriate type (optional string) and positioned correctly within the "Filter Options" section.README.md (1)
154-158: Documentation is clear and well-positioned.The new section explaining the
--force-includeoption provides a clear explanation of its purpose and includes a helpful example. The placement of this section in the README is logical, following the exclude patterns section and preceding the remote repository section.tests/cli/cliRun.test.ts (2)
78-79: Test mock properly updated for new config property.The
forceIncludearray property has been correctly added to the configuration mock object, which ensures tests reflect the updated configuration structure.
122-123: Test mock correctly updated for remote action.The
forceIncludearray has been consistently added to the remote action mock configuration, maintaining parity with the default action mock.tests/mcp/tools/packCodebaseTool.test.ts (1)
77-77: LGTM - Properly includes new forceInclude property in test configurationThe change correctly adds the new
forceIncludearray to the mock configuration object, which ensures test compatibility with the new feature. This is consistent with how other array properties are initialized in this mock.website/client/src/en/guide/command-line-options.md (2)
23-23: Clear documentation of the new CLI optionThe documentation for the new
--force-includeoption is clear and concise, appropriately placed in the Filter Options section.
63-65: Helpful example demonstrates practical usageGood job including a practical example that shows how to use the
--force-includeoption to include specific files that would typically be ignored, such as coverage reports and test results.tests/core/treeSitter/parseFile.solidity.test.ts (1)
28-28: LGTM - Properly includes new forceInclude property in test configurationThe change correctly adds the new
forceIncludearray to the default config object used in Solidity file parsing tests. This keeps the test configuration aligned with the updated schema.src/config/configLoad.ts (1)
112-116: LGTM - Properly implements configuration merging for forceIncludeThe implementation correctly merges the
forceIncludearrays from all three configuration sources (base, file, and CLI), following the same pattern used for theincludeproperty. The code handles potential undefined values with appropriate fallbacks.website/client/src/ja/guide/command-line-options.md (2)
23-23: Documentation accurately describes the new feature.The Japanese documentation for the new
--force-include <patterns>option clearly explains its purpose of overriding exclusion rules.
63-65: Good example of the new feature usage.The provided example effectively demonstrates how to use the
--force-includeoption with specific file patterns.src/cli/actions/defaultAction.ts (1)
110-112: Implementation correctly handles the forceInclude option.The code appropriately splits the comma-separated string, trims each pattern, and assigns it to the CLI configuration, consistent with how the
includeoption is handled.src/cli/cliRun.ts (1)
68-68: CLI option added with clear description.The
--force-include <patterns>option is correctly positioned alongside other filter options and has a clear description that explains its purpose of overriding ignore patterns.tests/testing/testUtils.ts (1)
32-32: Test utility correctly updated to support the new feature.The
createMockConfigfunction now properly handles theforceIncludeproperty, following the same pattern used for theincludeproperty, ensuring consistent testing of the new feature.website/client/src/ja/guide/configuration.md (2)
36-36: Good addition of the forceInclude configuration example.The example shows appropriate use cases for the force include feature with test-related files that are typically ignored but may need to be included in specific scenarios.
62-66: Clear priority ordering documentation for the new force include patterns.Excellent update to the exclusion pattern priority documentation. Placing "force include patterns" at the highest precedence clearly communicates to users how this new feature interacts with existing ignore rules.
tests/core/file/fileSearch.test.ts (2)
301-302: Good update to the mock implementation for the new dual globby calls.The implementation has been properly modified to handle both the initial globby call and the additional call that's made for forceInclude patterns. This ensures the test correctly simulates the behavior of the updated searchFiles function.
306-307: Appropriate change to use Set comparison for order-independent results.Using Set comparison rather than direct array comparison is a good approach since the order of results may vary when combining files from both normal search and force-included search. This makes the test more robust against implementation details.
tests/config/configSchema.test.ts (2)
81-81: Properly added forceInclude to default config test.The empty array default for forceInclude matches the pattern used for other similar fields and ensures the default configuration correctly validates with the updated schema.
171-171: Good test coverage for forceInclude in merged config.Including multiple file patterns in the test ensures that the merged configuration correctly handles various pattern types for the forceInclude option.
src/config/configSchema.ts (2)
42-42: Well-placed addition of forceInclude to the base schema.The forceInclude property is correctly defined as an optional array of strings, consistent with the include property definition above it. This maintains the schema's structure and type safety.
89-89: Appropriate default value for forceInclude in the default schema.Setting an empty array as the default value for forceInclude is consistent with how the include property is handled, ensuring that the property always exists with a sensible default.
website/client/src/en/guide/configuration.md (1)
36-36: LGTM: Clear example of the new forceInclude option.The addition of the
forceIncludefield with practical examples like coverage reports and test results is helpful and clearly demonstrates the intended use case for this feature.src/core/file/fileSearch.ts (6)
122-122: LGTM: Good addition of logging for the new feature.Adding trace logging for force include patterns is consistent with the existing logging approach for include and ignore patterns.
139-141: LGTM: Clear step labeling.Adding numbered steps in the comments makes the multi-phase filtering approach clear and easier to follow.
159-176: LGTM: Well-implemented force include logic.The implementation correctly handles force include patterns by:
- Escaping special characters in patterns
- Running a separate globby search without ignore filters
- Adding appropriate logging
This effectively ensures that force include patterns override all ignore rules as intended.
178-180: LGTM: Efficient deduplication approach.Using a Set to combine and deduplicate file paths is an efficient approach that avoids returning duplicate entries when a file matches both normal include patterns and force include patterns.
196-198: LGTM: Enhanced logging with force include statistics.The updated log message provides useful information about how many files were included via the force include mechanism, which will help with debugging.
201-201: LGTM: Proper integration with existing sorting logic.Using the sortPaths function on the combined paths ensures consistent sorting regardless of how files were included.
ktyubeshi
force-pushed
the
feature/forceinclude
branch
from
ffef120 to
68be8a0
Compare
|
Hi, @ktyubeshi ! I assume this is intended to address a similar use case as mentioned in this issue: I totally understand the motivation here — and I really appreciate the effort you've put into the implementation. Also, the use of the word "force" might create long-term challenges from a design perspective. For example, if users want to "force include" a set of files but then also ignore some of them, it becomes difficult to reason about what should happen. I’d love to explore a more flexible and conceptually cleaner solution before locking in this approach. Thanks again for opening the PR! |
|
Hi, @yamadashy Thank you for your feedback.
Regarding the choice of the word "force," my intention was to use a strong expression to clearly indicate that this would be the highest priority.
Regarding this, while an option like --force-include-ignore could be considered, I thought it might be acceptable to simply decide against adding such functionality. In my actual use cases, the targets I would specify with
The reason I added this option was that I wanted to keep repomix's default excluded files as they are. |
ktyubeshi
force-pushed
the
feature/forceinclude
branch
from
68be8a0 to
d7af036
Compare
ktyubeshi
force-pushed
the
feature/forceinclude
branch
2 times, most recently
from
1cc0ca4 to
0d07b04
Compare
ktyubeshi
force-pushed
the
feature/forceinclude
branch
from
0d07b04 to
3c29415
Compare
ktyubeshi
force-pushed
the
feature/forceinclude
branch
from
3c29415 to
aa473b0
Compare
…okenCount.test.ts Add the required forceInclude property to all RepomixConfigMerged objects in the diff token count tests to fix TypeScript compilation errors. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ktyubeshi
force-pushed
the
feature/forceinclude
branch
from
aa473b0 to
c53743f
Compare
Description:
This PR adds a
--force-includefeature to include specific files regardless of ignore patterns.Key changes:
--force-include <patterns>forceIncludeproperty to configuration filesUsage example:
repomix --force-include "coverage/summary.json,test-results/junit.xml"This feature allows including normally ignored files (e.g., coverage reports or specific log files) in the output when needed.
Documentation
Testing
Checklist
npm run testnpm run lintSummary by CodeRabbit
--force-include) and configuration files.