fix: Allow markdown as valid option in configuration #126
Conversation
Running `repopack` with `output.style` set to `markdown` generates the following error: ``` Error: Invalid configuration in /Users/rdh/repos/playground/exalata_repopack/repopack.config.json: output.style must be either "plain" or "xml" ```
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve an update to the Changes
Possibly related PRs
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: 0
🧹 Outside diff range and nitpick comments (1)
src/config/configValidate.ts (1)
35-36: LGTM! Consider using an array for valid styles.The changes correctly implement the new 'markdown' option for
output.styleand update the error message accordingly. This aligns well with the PR objective.For future maintainability, consider using an array to store valid styles:
const validStyles = ['plain', 'xml', 'markdown']; if (!validStyles.includes(style)) { throw new RepopackConfigValidationError(`output.style must be one of: ${validStyles.join(', ')}`); }This approach would make it easier to add or remove styles in the future without modifying the condition and error message separately.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
tests/config/configValidate.test.ts (1)
Line range hint
1-51: Suggestion: Add a test case for the "markdown" optionWhile the error message has been updated to include "markdown" as a valid option, there's no specific test case to verify that a configuration with
output.styleset to "markdown" passes validation. Consider adding a test case to ensure this new option is properly handled.Here's a suggested test case to add:
test('should pass for a valid config with markdown output style', () => { const validConfig = { output: { filePath: 'test.md', style: 'markdown' }, ignore: { useDefaultPatterns: true }, }; expect(() => validateConfig(validConfig)).not.toThrow(); });Additionally, the existing test cases look comprehensive and cover various scenarios. They test for:
- Valid configurations
- Invalid input types
- Invalid values for different configuration options
- Edge cases like non-object configs
This coverage appears to be thorough and well-structured.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- tests/config/configValidate.test.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
tests/config/configValidate.test.ts (1)
49-49: LGTM! The test case has been updated correctly.The error message in the test case has been updated to include "markdown" as a valid option for
output.style, which aligns with the PR objective of allowing markdown as a valid option in the configuration.To ensure consistency across the codebase, let's verify if similar changes have been made in the implementation file:
✅ Verification successful
Implementation Update Verified
The implementation in
src/config/configValidate.tsnow correctly includes "markdown" as a valid option foroutput.style, matching the updated test case.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the implementation file has been updated to include "markdown" as a valid option. # Test: Search for the validation of output.style in the implementation file. # Expect: The validation logic should include "markdown" as a valid option. rg --type typescript 'output\.style.*(?:plain|xml|markdown)' src/Length of output: 102
Script:
#!/bin/bash # Description: Retry checking if the implementation file has been updated to include "markdown" as a valid option. # Test: Search for the validation of output.style in TypeScript implementation files. # Expect: The validation logic should include "markdown" as a valid option. rg --glob '*.ts' --glob '*.tsx' 'output\.style.*(?:plain|xml|markdown)' src/Length of output: 216
|
@r-dh I can't believe I missed that. You're absolutely right - when we added the Markdown style, I forgot to update the configuration validation. Please feel free to contribute more in the future. Thanks again for your help in improving Repopack! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #126 +/- ##
=======================================
Coverage 76.40% 76.40%
=======================================
Files 36 36
Lines 1653 1653
Branches 266 266
=======================================
Hits 1263 1263
Misses 390 390 ☔ View full report in Codecov by Sentry. |
|
@r-dh I'm happy to let you know that I've just released version 0.1.43 which includes your fix. You can check out the release notes here: https://github.com/yamadashy/repopack/releases/tag/v0.1.43 Thanks again for your help, and please feel free to contribute more in the future if you spot any other issues or have ideas for improvements! |
Running
repopackwithoutput.styleset tomarkdowngenerates the following error: