fix(#837): filter umlaut-e, umlaut-a and * #839
Conversation
📝 WalkthroughWalkthroughTitle-casing regex logic updated to exclude capitalizing words after characters ö, é, ā, or *. Item data corrected for proper casing and diacritics. Item counts updated for several entries. Tests expanded to validate character handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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 (2)
build/parser.mjs (1)
27-32: Regex fix correctly prevents capitalization after special characters.The updated pattern properly addresses issue #837 by excluding é, ā, and * from the negative lookbehind. This ensures proper title-casing for names like "La Cathédrale Scene" (not "La CathéDrale Scene") and "Gaming Btches Glyph" (not "Gaming BTches Glyph").
The character class syntax is correct—the asterisk is literal inside
[öéā*]and doesn't require escaping.📝 Optional: Add explanatory comment for maintainability
As the excluded character set grows (ö from #833, now é/ā/* from #837), a brief comment would help future maintainers understand why these specific characters prevent capitalization:
const title = (str = '') => str .toLowerCase() + // Exclude capitalizing after diacritics/special chars to preserve proper names + // e.g., "Cathédrale" not "CathéDrale", "B*tches" not "B*Tches" .replace(/(?<![öéā*])\b\w/g, (l) => l.toUpperCase()) .replace(/'/gi, "'") .replace(/'S /gi, "'s ");test/utilities/title.spec.mjs (1)
10-17: Test coverage looks good!The new test cases correctly verify that letters following
*andéare not capitalized, which aligns with the fix for issue #837.However, the issue and regex pattern also mention the
ācharacter (a with macron), but there's no test case covering it. Consider adding a test for this character to ensure complete coverage of all special characters in the negative lookbehind pattern.Optional: Add test case for ā character
If an item name with
āexists in the data, consider adding a third test case:const cathedraleScene = find.findItem(cathedraleSceneUName); assert.equal(cathedraleScene.name, 'La Cathédrale Scene'); + + // Example - adjust uniqueName and expected name based on actual data + const itemWithMacronUName = '/Lotus/Types/Items/...'; // Replace with actual path + const itemWithMacron = find.findItem(itemWithMacronUName); + assert.equal(itemWithMacron.name, '...ā...'); // Expected name with ā followed by lowercase
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
build/parser.mjsdata/json/All.jsondata/json/Glyphs.jsondata/json/Misc.jsondata/json/Primary.jsondata/json/Resources.jsondata/json/i18n.jsontest/utilities/title.spec.mjs
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
🧬 Code graph analysis (1)
test/utilities/title.spec.mjs (1)
test/utilities/find.spec.mjs (1)
find(7-7)
🔇 Additional comments (3)
data/json/All.json (3)
322477-322477: Name corrections look good!The capitalization fixes for "Gaming Btches Glyph" (lowercase 't' after asterisk) and "La Cathédrale Scene" (lowercase 'd' after 'é') correctly implement the regex fix from issue #837. The updated parser logic now properly avoids capitalizing words that follow special characters (, é, ö, ā).
Also applies to: 461728-461728
423593-423593: Verify itemCount changes are intentional.Several itemCount values were updated during the data regeneration:
- Karkina Antenna: 3 → 20
- Lunar Pitcher: 6 → 50
- Moonlight Dragonlily: 6 → 10
- Sharrac Teeth: 5 → 20
- Sunlight Dragonlily: 4 → 10
These changes appear unrelated to the regex fix for special characters. Please confirm these are legitimate game data updates and not accidental side effects of the regeneration process.
Also applies to: 530210-530210, 629147-629147, 895474-895474, 966923-966923
942173-942173: Verify status_chance change is intentional.The status_chance value was updated from 32 to 50. This appears unrelated to the regex fix for special characters. Please confirm this is a legitimate game data update and not an accidental change during regeneration.
|
@coderabbitai you should consider using backticks instead of quotes for strings that might contain markdown-formatted text, as asterisks will make some of the text incorrectly italicized |
|
Tip For best results, initiate chat on the files or code changes. @TobiTenno: Thank you for the feedback! You're absolutely right. Using backticks for strings containing special markdown characters like asterisks prevents unintended formatting. For example, I'll make sure to use backticks for code identifiers, file paths, and any strings that contain markdown special characters like ✏️ Learnings added
🧠 Learnings used |
|
🎉 This PR is included in version 1.1272.99 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
What did you fix?
closes #837
Reproduction steps
See tests
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.