fix(cli): ignore output path using a relative path to the target#256
Conversation
|
|
📝 WalkthroughWalkthroughThe pull request introduces modifications to the file search functionality in the Changes
Sequence DiagramsequenceDiagram
participant FileSearch as File Search Module
participant RootDir as Root Directory
participant OutputPath as Output Path
FileSearch->>OutputPath: Resolve to absolute path
OutputPath->>RootDir: Calculate relative path
FileSearch->>FileSearch: Add relative path to ignore patterns
FileSearch->>FileSearch: Log relative path
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/file/fileSearch.ts (1)
162-164: Consider referencingrootDirrather thanprocess.cwd().Using
path.resolve(process.cwd(), config.output.filePath)may not behave as expected if the user runs the command from a different working directory than the project root. If the goal is always to compute this path relative torootDir, resolving fromrootDiravoids potential inconsistencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/file/fileSearch.ts(1 hunks)
🔇 Additional comments (1)
src/core/file/fileSearch.ts (1)
165-167: Verify path separator handling on non-POSIX systems.You are adding the relative path to the ignore patterns, which is a good fix. However, because ignore patterns can vary on Windows (due to backslash separators), consider confirming that
minimatchand related ignoring mechanisms behave correctly under Windows. Adding cross-platform tests or clarifying in documentation can prevent path mismatch issues.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #256 +/- ##
=======================================
Coverage 92.26% 92.27%
=======================================
Files 44 44
Lines 2108 2110 +2
Branches 462 462
=======================================
+ Hits 1945 1947 +2
Misses 163 163 ☔ View full report in Codecov by Sentry. |
|
Hi, @massdo ! I modified the code to use Your contribution is greatly appreciated! 🙌 |
Fix: Properly Ignore Output Path When a Target Is Given
Hi @yamadashy! 👋
I think we have an issue with ignoring the output file when a target is specified.
Issue Description
-> The
custom-output.txtfile is not ignored. If we run the command multiple times, it leads to an increased output file size, etc.Why
I believe this happens because the output file added to the
ignorePatternsis the pathfoo/bar/custom-output.txt.However,
globbycompares therootDirwith this path.When a target is specified, the
rootDirbecomes/current-working-directory/foo. As a result,globbytries to ignore/foo/bar/custom-output.txtinside/cwd/foo, which it cannot find.The ignored file needs to be relative to the target, in this case
/bar/custom-output.txt.Summary
We need to use a relative path to the target to ignore the output file.
Remark
For the same reason,
.gitignoreand.repomixignoreare not used if a target is specified. I'm not sure if this is intentional—maybe? If not, we would need to traverse up the tree from the target directory to retrieve the.gitignoreand.repomixignorefiles.If needed, I’d be happy to write the code to implement this change. Just let me know! 😊
Checklist
npm run testnpm run lint