Skip to content

fix(synced-lyrics): korean romanization#4528

Open
AARP41298 wants to merge 1 commit into
pear-devs:masterfrom
AARP41298:kor-romanization
Open

fix(synced-lyrics): korean romanization#4528
AARP41298 wants to merge 1 commit into
pear-devs:masterfrom
AARP41298:kor-romanization

Conversation

@AARP41298

@AARP41298 AARP41298 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #4526
Vibe coded

Edit:
Screenshot of the fix
image

Summary by CodeRabbit

  • Bug Fixes
    • Improved synced lyrics romanization so Hangul-to-Latin conversion works more reliably across different module setups.
    • Fixed an issue where the lyric translation step could fail if the Hanja translation helper was exposed in a different format.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

romanizeHangul in the synced-lyrics renderer is updated to resolve the translate function from the hanja module via a typed interop shim (HanjaApi) that handles both named translate and default.translate export shapes, then passes the result to esHangulRomanize.

Changes

Korean Romanization Hanja Interop Fix

Layer / File(s) Summary
HanjaApi interop and romanizeHangul update
src/plugins/synced-lyrics/renderer/utils.tsx
Adds HanjaApi type and derives hanjaTranslate by checking for a named translate export or falling back to default.translate; updates romanizeHangul to call hanjaTranslate(line, 'SUBSTITUTION') before passing to esHangulRomanize.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A bunny hopped through module land,
Where exports hid from every hand.
Named or default? Both shall do!
hanjaTranslate now sees it through.
Korean lyrics sing once more~ 🎵

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing Korean romanization in synced lyrics.
Linked Issues check ✅ Passed The change addresses #4526 by restoring Korean lyric romanization in synced lyrics via the hanja translation path.
Out of Scope Changes check ✅ Passed The diff stays focused on the romanization bug fix and does not introduce unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JellyBrick JellyBrick added the bug Something isn't working label Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: regression korean romanized lyrics

2 participants