Allow for passing in multiple directories #338
Conversation
📝 WalkthroughWalkthroughThe changes update several functions and tests across the codebase to handle multiple directories instead of a single directory string. The Sequence Diagram(s)sequenceDiagram
participant CLI
participant CLI_Run
participant DefaultAction
participant Packager
participant OutputGen
CLI->>CLI_Run: Pass directories array, cwd, and options
CLI_Run->>DefaultAction: executeAction(directories[], cwd, options)
DefaultAction->>Packager: runDefaultAction(directories[], cwd, options)
Packager->>Packager: Resolve and search files for each directory
Packager->>OutputGen: generateOutput(directories[], config, ...)
OutputGen-->>Packager: Return processed output
Packager-->>DefaultAction: Return aggregated result
DefaultAction-->>CLI_Run: Return final result
CLI_Run-->>CLI: Display output
Possibly related PRs
✨ Finishing Touches
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: 1
🔭 Outside diff range comments (1)
tests/cli/actions/defaultAction.test.ts (1)
60-71: Add test cases for multiple directories functionality.While the test has been updated to use an array, it only tests with a single directory. Consider adding the following test cases to validate the new multi-directory functionality:
- Test with multiple valid directories
- Test with an empty array
- Test with invalid directory paths
Example test case:
it('should handle multiple directories successfully', async () => { const options: CliOptions = { output: 'custom-output.txt', verbose: true, }; await runDefaultAction(['path/to/foo', 'path/to/bar'], process.cwd(), options); expect(configLoader.loadFileConfig).toHaveBeenCalled(); expect(configLoader.mergeConfigs).toHaveBeenCalled(); expect(packager.pack).toHaveBeenCalled(); }); it('should handle empty directory array', async () => { const options: CliOptions = {}; await expect(runDefaultAction([], process.cwd(), options)) .rejects .toThrow('At least one directory must be specified'); }); it('should handle invalid directory paths', async () => { const options: CliOptions = {}; await expect(runDefaultAction(['non/existent/path'], process.cwd(), options)) .rejects .toThrow('Directory not found'); });
🧹 Nitpick comments (7)
src/core/packager.ts (1)
47-51: Simplify array flattening usingArray.prototype.flat()The current code uses
reducewithconcatto flatten the array of collected files. This can be simplified by usingflat(), improving readability and efficiency.Apply this diff to simplify the code:
const rawFiles = ( await Promise.all( filePathsByDir.map(({ rootDir, filePaths }) => deps.collectFiles(filePaths, rootDir, progressCallback) ) ) - ).reduce((acc: RawFile[], curr: RawFile[]) => acc.concat(...curr), []); + ).flat();tests/core/output/outputStyles/xmlStyle.test.ts (1)
26-26: Add test cases for multiple directoriesTo fully validate the updated functionality of handling multiple directories, consider adding test cases where
generateOutputreceives multiple directory paths. This will ensure the function behaves as expected with various inputs.tests/core/output/outputStyles/plainStyle.test.ts (1)
26-26: Consider adding test cases for multiple directoriesSince
generateOutputnow handles multiple directories, enhancing the test to include scenarios with multiple directory inputs will ensure comprehensive test coverage and verify correct functionality across different cases.tests/core/output/outputGenerate.test.ts (1)
25-25: LGTM! Consider adding test cases for multiple directories.The changes correctly update the
generateOutputfunction calls to accept an array of directories. However, all test cases only test with a single directory ([process.cwd()]).Consider adding test cases that verify the behavior with multiple directories to ensure the new functionality works as expected. For example:
const output = await generateOutput([process.cwd(), 'path/to/second/dir'], mockConfig, mockProcessedFiles, []);Also applies to: 51-51, 86-86
tests/core/packager.test.ts (1)
72-72: LGTM! Consider expanding test coverage.The changes correctly update the test to handle arrays of directories. However, like the previous file, the test only covers the single directory case.
Consider adding test cases that verify:
- Multiple directories being packed together
- Edge cases like empty array of directories
- Duplicate directories in the array
Also applies to: 84-84
src/cli/cliRun.ts (1)
46-46: LGTM! Consider updating the CLI description.The changes correctly implement support for multiple directories. The default value of
['.']maintains backward compatibility.Consider updating the program description to explicitly mention the support for multiple directories. For example:
- .description('Repomix - Pack your repository into a single AI-friendly file') + .description('Repomix - Pack one or more directories into a single AI-friendly file')Also applies to: 76-76, 84-84, 105-105
src/cli/actions/defaultAction.ts (1)
107-182: Improve code consistency in buildCliConfig.The refactoring of the buildCliConfig function improves code consistency by using the spread operator uniformly. This makes the code more maintainable and easier to read.
Consider extracting the repeated pattern into a helper function to reduce duplication:
+const updateOutput = (cliConfig: RepomixConfigCli, key: string, value: unknown): void => { + cliConfig.output = { + ...cliConfig.output, + [key]: value, + }; +}; const buildCliConfig = (options: CliOptions): RepomixConfigCli => { const cliConfig: RepomixConfigCli = {}; // ... other code ... if (options.topFilesLen !== undefined) { - cliConfig.output = { - ...cliConfig.output, - topFilesLength: options.topFilesLen, - }; + updateOutput(cliConfig, 'topFilesLength', options.topFilesLen); } // Apply similar changes to other output updates return repomixConfigCliSchema.parse(cliConfig); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/cli/actions/defaultAction.ts(4 hunks)src/cli/actions/remoteAction.ts(1 hunks)src/cli/cliRun.ts(3 hunks)src/core/output/outputGenerate.ts(4 hunks)src/core/packager.ts(4 hunks)tests/cli/actions/defaultAction.test.ts(22 hunks)tests/cli/cliRun.test.ts(11 hunks)tests/core/output/outputGenerate.test.ts(3 hunks)tests/core/output/outputStyles/plainStyle.test.ts(1 hunks)tests/core/output/outputStyles/xmlStyle.test.ts(1 hunks)tests/core/packager.test.ts(2 hunks)tests/integration-tests/packager.test.ts(2 hunks)
🔇 Additional comments (13)
src/core/packager.ts (1)
24-24: Update function parameter to accept multiple directoriesChanging the
packfunction'srootDirparameter torootDirs: string[]appropriately enables processing multiple directories. This change is correctly reflected in the function signature and facilitates the intended functionality.tests/core/output/outputStyles/xmlStyle.test.ts (1)
26-26: Update test to pass multiple directories togenerateOutputChanging the argument to
[process.cwd()]correctly aligns with the updatedgenerateOutputfunction signature that now expects an array of directories.tests/core/output/outputStyles/plainStyle.test.ts (1)
26-26: Update test to pass multiple directories togenerateOutputChanging the argument to
[process.cwd()]correctly reflects the updated function signature ofgenerateOutputthat now accepts an array of directories.src/cli/actions/remoteAction.ts (1)
50-50: LGTM! The change maintains compatibility with the new interface.The update correctly wraps
tempDirPathin an array to match the newrunDefaultActionsignature while maintaining the existing remote repository processing logic.tests/integration-tests/packager.test.ts (1)
77-77: LGTM! Test updated to support multiple directories.The test has been correctly updated to pass the input directory as an array, aligning with the new multi-directory support feature.
src/cli/actions/defaultAction.ts (3)
23-27: LGTM! Function signature updated to support multiple directories.The runDefaultAction function now accepts an array of directories, aligning with the PR's objective to support multiple input directories.
46-46: LGTM! Path resolution handles multiple directories.The code efficiently maps over the directories array to resolve all paths in parallel.
54-54: LGTM! Pack function call updated for multiple directories.The pack function is now called with the array of target paths, maintaining consistency with the multi-directory support.
src/core/output/outputGenerate.ts (3)
7-7: LGTM! Improved type safety with explicit type import.Added FileSearchResult type import to enhance type safety.
114-116: LGTM! Function signatures updated for multiple directories.Both generateOutput and buildOutputGeneratorContext functions now accept an array of root directories, aligning with the multi-directory support feature.
Also applies to: 134-136
154-162: LGTM! Efficient parallel directory search implementation.The code efficiently searches multiple directories in parallel using Promise.all and reduces the results into a single FileSearchResult. This implementation maintains good performance while supporting multiple directories.
tests/cli/cliRun.test.ts (1)
128-130: LGTM! Tests updated to support multiple directories.All test cases have been consistently updated to pass directory arguments as arrays, maintaining comprehensive test coverage while supporting the new multi-directory feature.
Also applies to: 134-136, 140-141, 147-148, 154-156, 165-169, 177-181, 191-195, 203-207, 215-219, 227-231, 239-243, 251-257, 265-271, 279-285
tests/cli/actions/defaultAction.test.ts (1)
73-459: LGTM!The feature-specific tests have been consistently updated to use the new array syntax while maintaining their focus on verifying the configuration options. Since these tests are primarily concerned with validating feature behavior rather than directory handling, using a single directory is acceptable.
rchatrath7
force-pushed
the
rc/multiple-dir-paths
branch
from
2e57844 to
e6834d9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/core/packager.ts (1)
47-51: Simplify the file collection logic.The nested Promise.all with reduce makes the code harder to read. Consider extracting the reduction logic into a separate function for better readability.
- const rawFiles = ( - await Promise.all( - filePathsByDir.map(({ rootDir, filePaths }) => deps.collectFiles(filePaths, rootDir, progressCallback)), - ) - ).reduce((acc: RawFile[], curr: RawFile[]) => acc.concat(...curr), []); + const collectFilesFromDir = async ({ rootDir, filePaths }: { rootDir: string; filePaths: string[] }) => + deps.collectFiles(filePaths, rootDir, progressCallback); + + const rawFilesArrays = await Promise.all(filePathsByDir.map(collectFilesFromDir)); + const rawFiles = rawFilesArrays.flat();src/core/output/outputGenerate.ts (1)
154-161: Simplify the empty directory search reduction.The nested reduction logic is complex and could be simplified for better readability.
- emptyDirPaths = (await Promise.all(rootDirs.map((rootDir) => searchFiles(rootDir, config)))).reduce( - (acc: FileSearchResult, curr: FileSearchResult) => - ({ - filePaths: [...acc.filePaths, ...curr.filePaths], - emptyDirPaths: [...acc.emptyDirPaths, ...curr.emptyDirPaths], - }) as FileSearchResult, - { filePaths: [], emptyDirPaths: [] }, - ).emptyDirPaths; + const searchResults = await Promise.all(rootDirs.map((rootDir) => searchFiles(rootDir, config))); + emptyDirPaths = searchResults.flatMap(result => result.emptyDirPaths);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/cli/actions/defaultAction.ts(4 hunks)src/cli/actions/remoteAction.ts(1 hunks)src/cli/cliRun.ts(3 hunks)src/core/output/outputGenerate.ts(4 hunks)src/core/packager.ts(4 hunks)tests/cli/actions/defaultAction.test.ts(22 hunks)tests/cli/cliRun.test.ts(11 hunks)tests/core/output/outputGenerate.test.ts(3 hunks)tests/core/output/outputStyles/plainStyle.test.ts(1 hunks)tests/core/output/outputStyles/xmlStyle.test.ts(1 hunks)tests/core/packager.test.ts(2 hunks)tests/integration-tests/packager.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/core/output/outputStyles/xmlStyle.test.ts
- tests/core/output/outputStyles/plainStyle.test.ts
- tests/core/output/outputGenerate.test.ts
- tests/integration-tests/packager.test.ts
- src/cli/actions/remoteAction.ts
- tests/cli/actions/defaultAction.test.ts
- tests/cli/cliRun.test.ts
- tests/core/packager.test.ts
🔇 Additional comments (3)
src/core/packager.ts (1)
39-44: Handle potential errors during file search across multiple directories.Using
Promise.allassumes that allsearchFilesoperations will succeed. IfsearchFilesfails for any directory, it will reject the entirePromise.all, potentially causing unhandled exceptions.src/cli/cliRun.ts (1)
46-46: LGTM! Good use of variadic arguments.The change to use variadic arguments with
[directories...]and the default value['.']is a clean way to handle multiple directory inputs while maintaining backward compatibility.Also applies to: 76-76
src/cli/actions/defaultAction.ts (1)
107-182: LGTM! Consistent use of spread operator.The refactoring to consistently use the spread operator for config building improves code readability and maintainability.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #338 +/- ##
==========================================
- Coverage 90.38% 90.11% -0.27%
==========================================
Files 48 48
Lines 2631 2682 +51
Branches 542 546 +4
==========================================
+ Hits 2378 2417 +39
- Misses 253 265 +12 ☔ View full report in Codecov by Sentry. |
yamadashy
force-pushed
the
rc/multiple-dir-paths
branch
from
e6834d9 to
8f54212
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cli/cliRun.ts (1)
84-90: Consider enhancing directory validation logging.While the current logging is good, consider adding validation logging to help users identify issues with directory paths early.
Add directory existence and permission checks with detailed logging:
export const executeAction = async (directories: string[], cwd: string, options: CliOptions) => { logger.setVerbose(options.verbose || false); logger.trace('directories:', directories); logger.trace('cwd:', cwd); logger.trace('options:', options); + + // Early validation of directories + for (const dir of directories) { + logger.trace(`Validating directory: ${dir}`); + // Note: Actual validation would be implemented in a separate utility + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cli/cliRun.ts(3 hunks)
🔇 Additional comments (2)
src/cli/cliRun.ts (2)
46-46: LGTM! Well-structured CLI argument definition.The change to support multiple directories is implemented correctly using variadic arguments, with a sensible default value that maintains backward compatibility.
76-76: LGTM! Action handler correctly updated.The action handler signature properly reflects the new directories array parameter, with defaults appropriately moved to the argument definition level.
|
hi, @rchatrath7 ! The code looks perfect with just one small commit I added! I'll merge this now. Thank you! |
|
@rchatrath7 Thank you again for your valuable contribution! 🙏 |
|
Nice feature! |
On large projects, I will have files that are contextually related but in separate sub directories that I would like to bundle together. However, due to token limits, I don't necessarily want to find the closest common parent directory and bundle the entire thing. What I really wanted was an option to be able to pass in multiple directories as arguments and have repomix bundle them all together automatically, something like
This PR adds support for having the
directoryargument be an arbitrary number of directories instead of a single string and collects all of that into one output.yields
Checklist
npm run testnpm run lint