-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: improve YAML parsing for custom modes to be more lenient #7139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add lenient YAML parsing with strict:false and uniqueKeys:false options - Implement multiple fallback strategies for .roomodes files (JSON, original content, partial extraction) - Add recovery mechanisms for invalid/incomplete mode configurations - Provide default values for missing required fields - Fix handling of various group formats (string, array, complex with fileRegex) - Improve error messages with helpful suggestions - Add comprehensive tests for edge cases and recovery scenarios Fixes #7138
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
private extractPartialModes(content: string): any { | ||
try { | ||
const modes: any[] = [] | ||
const lines = content.split("\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extractPartialModes()
method iterates through every line of potentially large YAML files. For very large corrupted files, this could cause performance issues. Consider adding a size limit or line count limit (e.g., max 10,000 lines) to prevent hanging on massive files.
// Try parsing the original content as JSON (not the cleaned content) | ||
return JSON.parse(content) | ||
const jsonParsed = JSON.parse(content) | ||
console.log(`[CustomModesManager] Successfully parsed ${filePath} as JSON`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, consider using the logger
utility (already imported at line 14) instead of console.log
:
console.log(`[CustomModesManager] Successfully parsed ${filePath} as JSON`) | |
logger.info(`[CustomModesManager] Successfully parsed ${filePath} as JSON`) |
strict: false, | ||
uniqueKeys: false, | ||
}) | ||
console.log(`[CustomModesManager] Successfully parsed ${filePath} with original content`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly here, consider using logger.info
for consistency:
console.log(`[CustomModesManager] Successfully parsed ${filePath} with original content`) | |
logger.info(`[CustomModesManager] Successfully parsed ${filePath} with original content`) |
// Strategy 3: Try to extract valid modes even from partially corrupted YAML | ||
const partialModes = this.extractPartialModes(content) | ||
if (partialModes && partialModes.customModes && partialModes.customModes.length > 0) { | ||
console.log( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here as well:
console.log( | |
logger.info( | |
`[CustomModesManager] Extracted ${partialModes.customModes.length} modes from partially corrupted YAML`, | |
) |
|
||
return modes.length > 0 ? { customModes: modes } : null | ||
} catch (error) { | ||
console.error("[CustomModesManager] Failed to extract partial modes:", error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After logging the error, should we return null immediately instead of continuing? The current approach might mask other issues:
console.error("[CustomModesManager] Failed to extract partial modes:", error) | |
logger.error("[CustomModesManager] Failed to extract partial modes:", error) | |
return null |
Summary
This PR fixes issue #7138 where custom modes would disappear or fail to load due to overly strict YAML parsing and validation.
Problem
Users were experiencing:
Solution
Implemented a multi-layered approach to make custom mode loading more robust:
1. Lenient YAML Parsing
strict: false
anduniqueKeys: false
options to YAML parser2. Multiple Fallback Strategies
For .roomodes files, implemented fallback chain:
3. Mode Recovery Mechanisms
4. Better Error Handling
Testing
Impact
Users should now experience:
Fixes #7138
Important
Improves YAML parsing for custom modes with lenient parsing, fallback strategies, and comprehensive testing.
parseYamlSafely()
inCustomModesManager.ts
now uses lenient YAML parsing withstrict: false
anduniqueKeys: false
..roomodes
files: JSON parsing, original content parsing, partial mode extraction.CustomModesManager.lenientParsing.spec.ts
to test lenient parsing, JSON fallback, and recovery mechanisms.CustomModesManager.yamlEdgeCases.spec.ts
to test edge cases like BOM handling, invisible characters, and complex structures.loadModesFromFile()
to handle single mode objects and invalid arrays.fixCommonModeIssues()
andrecoverInvalidMode()
for better mode validation and recovery.This description was created by
for 7bda2dc. You can customize this summary. It will automatically update as commits are pushed.