fix(config): set file extension based on style parameter#947
fix(config): set file extension based on style parameter#947yamadashy merged 5 commits intoyamadashy:mainfrom
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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR updates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
Summary of ChangesHello @Ahmad8864, 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 refines the configuration merging process to ensure greater consistency in how output file paths are determined. The core change ensures that the output file extension aligns with the chosen style by default, while still respecting any explicitly defined file paths from either the configuration file or command-line arguments. This makes the system more intuitive by linking the output format directly to the file extension, unless overridden. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully updates the configuration merging logic to automatically set the output file extension based on the selected style, which is a great improvement for user experience. The new tests are comprehensive and correctly verify the intended behavior across different scenarios. I've identified an opportunity to simplify the new logic in src/config/configLoad.ts by removing some redundant conditions and dead code, which will enhance the code's readability and maintainability.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the configuration merging logic to automatically update the output file extension to match the selected style when no explicit file path is provided.
Key Changes:
- Modified
mergeConfigsfunction to intelligently adjust output file paths based on style selection - Added logic to detect whether a file path was explicitly set versus inherited from defaults
- Added comprehensive test coverage for various style and file path combination scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/config/configLoad.ts | Enhanced mergeConfigs to automatically set file extension based on style when filePath is not explicitly provided, while preserving explicit user-specified paths |
| tests/config/configLoad.test.ts | Added four new test cases covering scenarios: style-only via CLI, explicit filePath with style, file config filePath with CLI style, and style in file config without filePath |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/config/configLoad.test.ts (1)
305-343: Excellent test coverage for the new file path adjustment behavior!The four new tests comprehensively validate the core scenarios:
- Auto-adjusting filePath when only style is provided (CLI or file config)
- Preserving explicit filePaths regardless of style changes
- Correct handling of precedence between CLI and file config
The test assertions correctly match the
defaultFilePathMapvalues and verify that user-provided paths are never overridden.Optional enhancement: Consider adding a test where both
styleandfilePathare provided in file config (not CLI) to explicitly verify that explicit file config paths are preserved. While this is implicitly covered by thefilePathExplicitlogic, it would make the test suite even more complete.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/config/configLoad.ts(1 hunks)tests/config/configLoad.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/config/configLoad.test.ts (1)
src/config/configLoad.ts (1)
mergeConfigs(170-243)
src/config/configLoad.ts (2)
src/config/configSchema.ts (1)
defaultFilePathMap(9-14)src/shared/logger.ts (1)
logger(89-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (2)
src/config/configLoad.ts (2)
198-199: Clean implementation of explicit filePath detection.The
filePathExplicitflag correctly identifies when the user has provided a filePath in either source (file config or CLI), preventing unwanted auto-adjustments. The style computation with fallback ensures we always have a valid style value.
204-210: Smart conditional logic for automatic filePath adjustment.The implementation correctly identifies when a filePath should be auto-adjusted to match the style:
- Verifies the user didn't explicitly provide a path
- Confirms the current path came from base config defaults (not from elsewhere)
- Only adjusts when the desired path differs
This preserves user intent while providing convenient automatic file extension updates.
|
@yamadashy Hi there, just wanted to check if I needed to do anything else to get a review. |
|
Hi, @Ahmad8864! It looks like there was a regression introduced in #923. I really appreciate you catching this and fixing it! The code looks good. I just added a comment to explain the logic. I'll merge this now. Thanks again! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #947 +/- ##
==========================================
+ Coverage 89.62% 89.68% +0.05%
==========================================
Files 111 111
Lines 7839 7843 +4
Branches 1501 1504 +3
==========================================
+ Hits 7026 7034 +8
+ Misses 813 809 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add release notes documenting: - Bug fix for output file extension not matching style parameter (#947) - Package size reduction by replacing tree-sitter-wasms (#942) - Dependency updates (#944) This release fixes an issue introduced in v1.9.0 where the output file extension didn't automatically adjust when using --style option.
Summary
This PR updates the logic in mergeConfigs to handle output file paths more consistently. The output file name now automatically matches the selected style, unless a specific file path is set in the config or via the CLI.
Changes
Updated mergeConfigs in src/config/configLoad.ts so that when only a style is provided, the default output file path updates to match that style. If a file path is explicitly set, it stays unchanged.
Added new tests in tests/config/configLoad.test.ts to cover these scenarios and verify that the path updates correctly when it should, and stays the same when a file path is explicitly defined.
Checklist
npm run testnpm run lint