Skip to content
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

Remove <ruby> furigana before generating readings #19

Merged
merged 5 commits into from
Oct 7, 2022

Conversation

ahlec
Copy link
Collaborator

@ahlec ahlec commented Oct 6, 2022

Before we run the current field through mecab.reading to attach furigana, line 60 correctly removes the bracket notation furigana already in the input string. This prevents clicking the button multiple times from compounding the readings. However, <ruby> markup is not removed before sending for generation, which compounds additional readings:
image

To fix this, I extracted the code from the "delete furigana" button into a reusable utility function removeFurigana. We then use this to remove readings before sending the text through generation.

In doing this, I fixed a couple of bugs with the furigana deletion logic:

  • Decoupled from current config. Every string is run through both <ruby> removal and bracket notation removal. Use cases:
    • User has changed their config setting and would like to generate in the new style
    • User used another plugin to generate the original content (in bracket notation) and wants to use this tool to replace it with new style (<ruby>)
  • Removes reliance on <rp> tag. The original implementation required a format of <ruby>日本語<rp>... in order to work. If you didn't include the <rp> tag (which is optional in the HTML specification), it would throw errors. It worked in the general flow because this tool always included the <rp> tag. I've reimplemented this to decouple from a particular structure and to just remove <rp> and <rt> tags to arrive at the body instead.

I've added some unit tests to test these behaviors.

I've tested this in both 2.1.54 (M1 Silicon) and 2.1.49 (Mac Intel) and it loads and works as expected.

@obynio
Copy link
Owner

obynio commented Oct 7, 2022

A very nice work as always. I never paid attention to this behaviour, thank you for the fix and the unit tests. The code is much cleaner this way ! I will merge this and release a 1.3.2 update.

@obynio obynio merged commit 117ab41 into obynio:master Oct 7, 2022
@ahlec ahlec deleted the strip-furigana-before-reading branch October 7, 2022 18:05
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.

None yet

2 participants