integrate strip-json-comments for JSON configuration parsing#214
integrate strip-json-comments for JSON configuration parsing#214yamadashy merged 5 commits intoyamadashy:mainfrom
Conversation
|
|
📝 WalkthroughWalkthroughThe pull request introduces a new dependency Changes
Possibly related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
🧹 Nitpick comments (1)
src/config/configLoad.ts (1)
71-71: Consider enhancing error handling for comment stripping.While the implementation is correct, we could improve error handling by catching potential errors from
stripJsonComments.Consider this enhancement:
- const config = JSON.parse(stripJsonComments(fileContent)); + let strippedContent; + try { + strippedContent = stripJsonComments(fileContent); + } catch (error) { + throw new RepomixError(`Error stripping comments from config file ${filePath}: ${error.message}`); + } + const config = JSON.parse(strippedContent);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)src/config/configLoad.ts(2 hunks)
🔇 Additional comments (2)
package.json (1)
72-73: LGTM! Dependency addition looks good.
The strip-json-comments package is correctly added as a production dependency with an appropriate version constraint.
src/config/configLoad.ts (1)
3-3: LGTM! Clean import statement.
The import is properly placed and uses the correct import style for the package.
|
OMG I need this!! |
# Conflicts: # package-lock.json # package.json
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
biome.json (1)
40-43: Consider documenting this configuration change.Since this is a significant change in how JSON files are processed, it would be helpful to document this capability in the project's documentation.
Consider adding a comment in the README or documentation about JSON files supporting comments, along with examples of valid comment syntax. I can help draft this documentation if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
biome.json(1 hunks)package.json(1 hunks)repomix.config.json(1 hunks)src/config/configLoad.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- repomix.config.json
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/config/configLoad.ts
🔇 Additional comments (1)
biome.json (1)
40-43: LGTM! Verify impact on included JSON files.
The JSON parser configuration is correctly structured and aligns with the PR's objective of supporting commented JSON configurations.
Let's verify the impact on all JSON files in the project:
✅ Verification successful
Enabling JSON comments is safe and already in use
The verification shows that:
- There are 8 JSON files in the project that will be processed by Biome
- Comments are already being used in configuration files:
repomix.config.jsonhas line commentstsconfig.jsonandtsconfig.build.jsoncontain include patterns (which are not actually comments, these were false positives)
Let's verify if there are any multi-line comments as well, with a more precise pattern:
Enabling JSON comments is safe and already in use
The verification confirms that:
- Comments are already being used in
repomix.config.jsonwith single-line comments - No multi-line comments are currently used (the matches in tsconfig files were false positives)
- The configuration change aligns with existing usage in the codebase
- This change will support the current commenting practice and allow for more flexible JSON configuration files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check all JSON files that might be affected by the new parser configuration
# Find all JSON files mentioned in the include section
echo "Checking JSON files that will be processed by Biome:"
fd -e json --exec echo "Found JSON file: {}"
# Look for existing comments in JSON files
echo -e "\nChecking for existing comments in JSON files:"
rg -U '(?m)^\s*//|/\*[\s\S]*?\*/' --type json
Length of output: 893
Script:
#!/bin/bash
# Check specifically for JSON comments with a more precise pattern
echo "Checking for single-line comments (//):"
rg '^\s*//.*$' --type json
echo -e "\nChecking for multi-line comments (/* */):"
rg '/\*(?s).*?\*/' --type json
Length of output: 417
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
=======================================
Coverage 92.01% 92.01%
=======================================
Files 44 44
Lines 2041 2041
Branches 450 450
=======================================
Hits 1878 1878
Misses 163 163 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (2)
375-375: Enhance the JSON5 example and improve syntax highlightingConsider these improvements for better documentation:
- Use
jsoninstead ofjson5for wider syntax highlighting support- Add more JSON5 features to showcase its advantages, such as:
- Trailing commas
- Single quotes
- Multi-line strings
Example enhancement:
-```json5 +```json { "output": { - "filePath": "repomix-output.xml", + "filePath": 'repomix-output.xml', "style": "xml", - "headerText": "Custom header information for the packed file.", + "headerText": "Custom header information \ + for the packed file.", "removeComments": false, "removeEmptyLines": false, "showLineNumbers": false, "copyToClipboard": true, "topFilesLength": 5, - "includeEmptyDirectories": false + "includeEmptyDirectories": false, // trailing comma },
392-392: Enhance the comment with more contextThe comment about
.repomixignorecould be more informative by explaining the relationship between configuration-based patterns and.repomixignore.Example enhancement:
- // Patterns can also be specified in .repomixignore + // These patterns complement those in .repomixignore file. + // See "Ignore Patterns" section for priority order. "customPatterns": ["additional-folder", "**/*.log"]tests/config/configLoad.test.ts (1)
87-99: Consider adding more comment edge cases
Testing single-line and multi-line comments here is excellent. To further increase reliability, you could include edge cases such as comments at the end of lines or empty lines containing only comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)tests/config/configLoad.test.ts(1 hunks)
🔇 Additional comments (2)
tests/config/configLoad.test.ts (2)
86-86: Good descriptive test name for comment-based parsing
This test's name precisely clarifies its purpose, aligning with the PR's objective of supporting JSON files containing comments.
100-107: Test ensures correctness of final parsed structure
Confirming that the returned object matches the expected result validates that comments are properly stripped out and do not interfere with JSON parsing. This test effectively verifies the integration of the strip-json-comments dependency.
|
Hi, @ivanionut ! I've added some tests and README updates - the changes look great. |
3 participants
Integrated strip-json-comments to clean JSON configuration files by removing comments before parsing.
Checklist
npm run test:npm run lint: