feat(synced-lyrics): Add Simplified/Traditional Chinese converter and improve Romanization to display tone #4111
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds Chinese character conversion capabilities (Simplified ↔ Traditional) and improves Chinese romanization (Pinyin) to display tones. The changes replace the tiny-pinyin library with pinyin-pro for better tone support and add the chinese-conv library for character conversion.
Key changes:
- Added a new configuration option to convert between Simplified and Traditional Chinese characters
- Replaced
tiny-pinyinwithpinyin-profor improved Pinyin romanization with tone marks - Added
chinese-convlibrary for bidirectional Chinese character conversion
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/plugins/synced-lyrics/types.ts | Added convertChineseCharacter configuration option to the plugin config type |
| src/plugins/synced-lyrics/renderer/utils.tsx | Added convertChineseCharacter function and updated romanizeChinese to use pinyin-pro library |
| src/plugins/synced-lyrics/renderer/components/SyncedLine.tsx | Integrated Chinese character conversion into the synced lyrics display |
| src/plugins/synced-lyrics/renderer/components/PlainLyrics.tsx | Integrated Chinese character conversion into the plain lyrics display |
| src/plugins/synced-lyrics/menu.ts | Added menu options for selecting Chinese character conversion mode |
| src/menu.ts | Improved code formatting for language menu label (unrelated to main feature) |
| src/i18n/resources/en.json | Added English translations for new Chinese character conversion menu options |
| pnpm-lock.yaml | Updated dependencies: removed tiny-pinyin, added pinyin-pro and chinese-conv |
| package.json | Updated dependencies: removed tiny-pinyin, added pinyin-pro and chinese-conv |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| chinese-conv: | ||
| specifier: ^4.0.0 | ||
| version: 4.0.0 |
There was a problem hiding this comment.
The chinese-conv@4.0.0 package requires Node.js ^20.19.0 || >=22.12.0, but the project's package.json specifies engines.node as ">=22". This means versions like 22.0.0 through 22.11.x would meet the project's requirement but fail the dependency's requirement. Consider updating the project's Node.js requirement to ">=22.12.0" to ensure consistency.
There was a problem hiding this comment.
Code-wise this LGTM, I am not a native chinese speaker and I've only studied for 2 years before I gave up, so I can't really make the decision on which pinyin library is better.
I'll leave that up to you @JellyBrick
3 participants
With this changes, Chinese romanization (Pinyin) will display the tone. And, users will be able to see the Chinese lyrics in their preferred Chinese character (Simplified or Traditional character).
As a Chinese learner myself, I think this would help a lot!