Skip to content

Improve Edict2 pinyin (romanization)#678

Merged
ilius merged 8 commits into
ilius:masterfrom
benfl:pinyin-fixes
Oct 12, 2025
Merged

Improve Edict2 pinyin (romanization)#678
ilius merged 8 commits into
ilius:masterfrom
benfl:pinyin-fixes

Conversation

@benfl

@benfl benfl commented Oct 12, 2025

Copy link
Copy Markdown
Contributor

Some words were incorrectly romanized when they contained a 'ü', replacing it with a 'v' instead. This happened if the ü occurred with a neutral ("fifth") tone (the only current example of which is 閨女 guī​nü), or if it was part of the 'üe' diphthong. This fix ensures that the correct ü is always used.

The romanization process was also converting words to lowercase in order to simplify adding tone diacritics. This restores the original case.

The romanization also previously omitted apostrophes where they should have been included. For example, 企鵝 qǐ​'é was rendered as qǐ​é.

Finally, this adds spaces to separate capitalized names. So 孫中山 is now correctly rendered as Sūn​ Zhōng​shān instead of sūn​zhōng​shān.

benfl added 4 commits October 12, 2025 12:31
Ensure that ü is rendered properly when it has a neutral tone or occurs
in the diphthong üe. Previously, it would be rendered as a v in these
cases.
This ensures that correct capitalization is preserved for proper names.
Put spaces between proper names and include apostrophes where required.
Comment thread pyglossary/plugins/edict2/conv.py Outdated
def render_syllables_no_color(
hf: T_htmlfile,
syllables: Sequence[str],
_tones: Sequence[str],

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ prefix is for unused args

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed!

Comment thread pyglossary/plugins/edict2/conv.py Outdated


def render_syllables(
color: bool, hf: T_htmlfile, syllables: Sequence[str], tones: Sequence[str]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comma at the end and ruff format to break into separate lines.
And move color to the end of args, with default True value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@ilius

ilius commented Oct 12, 2025

Copy link
Copy Markdown
Owner

Please test it.

@benfl

benfl commented Oct 12, 2025

Copy link
Copy Markdown
Contributor Author

I converted the CEdict file from here to an HTML directory using both simplified headwords with color, and traditional headwords without color. It looks good to me; here are screenshots showing the new features.

trad-nocolor simp-color simp2 trad2

@ilius

ilius commented Oct 12, 2025

Copy link
Copy Markdown
Owner

Can you upload a small subset of that glossary?
Something smaller than 32 KB in original format that can cover these cases.
So that I can add it for a full conversion test.

@benfl

benfl commented Oct 12, 2025

Copy link
Copy Markdown
Contributor Author

cedict_samples_utf-8_mdbg.txt

Here's a file with a few representative words. I would want to keep an eye specifically on the following:

  • "Zhong1 guo2 Xiao3 shuo1 Shi3 lu:e4" should become "Zhōng​guó​ Xiǎo​shuō​ Shǐ​lüè​", with that spacing and the correct ü character.
  • "bu4 er4 guo4" and "qi3 e2" should become "bù'èrguò" and "qǐ​'é", with the apostrophes.
  • "gui1 nu:5" should become "guī​nü" with the correct ü.
  • "ang1" should become "āng" with no apostrophe.
    There are a few additional words, including some random roman letters as well as some erhua syllables that shouldn't pose a problem, but might still be good to check.

@ilius

ilius commented Oct 12, 2025

Copy link
Copy Markdown
Owner

Thanks.

@ilius ilius merged commit d5ef4a3 into ilius:master Oct 12, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants