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

Conversion problem with toRomaji function #18

Closed
Moseco opened this issue Jun 4, 2022 · 16 comments · Fixed by #24
Closed

Conversion problem with toRomaji function #18

Moseco opened this issue Jun 4, 2022 · 16 comments · Fixed by #24
Assignees
Labels
bug Something isn't working

Comments

@Moseco
Copy link

Moseco commented Jun 4, 2022

When the toRomaji function is given katakana with the pattern "*ヮー" it throws the exception Null check operator used on a null value on line 59 of the conversions.dart file

Two examples I encountered the problem with シークヮーサー and シマウヮー

@Moseco
Copy link
Author

Moseco commented Jun 4, 2022

I created a (hopefully) simple fix. I added a check above line 59 of the conversions.dart.

// Ensure 'ヮー' => 'ゎー' become 'waa'
if (previousKana == 'ゎ' && char == 'ー') {
  hiraChars.removeLast();
  hiraChars.addAll(['わ', 'あ']);
  continue;
}

It now converts 'シークヮーサー' to 'shiikuwaasaa' and 'シマウヮー' to 'shimauwaa' which seems correct to me (and for what it's worth Google Translate's pronunciation also does an extended vowel wa).

I tested this on my own and it doesn't seem to mess with any other conversions. Although I wasn't able to run the test suite because pub is having trouble resolving the dependencies.

@jeroen-meijer What do you think of this change? If it looks okay I can create a PR. If not, do you have a different solution in mind?

@jeroen-meijer
Copy link
Owner

Hey there,

Thanks for opening this issue. Looks great! I'll quickly try it myself later today, and fix the pub CI pipelines and tests in the meantime 👍🏻

@jeroen-meijer jeroen-meijer self-assigned this Jun 4, 2022
@jeroen-meijer jeroen-meijer added the bug Something isn't working label Jun 4, 2022
Moseco added a commit to Moseco/kana_kit that referenced this issue Jun 6, 2022
@tdondich
Copy link

Just a ping to see if this will make it in.

@HighLiuk
Copy link

HighLiuk commented Sep 22, 2024

Hey @jeroen-meijer how about this?
This bug is preventing the JM Dict package from working

And by the way for those who just simply can't wait for the PR to be merged, just add this to your pubspec.yaml:

dependency_overrides:
  kana_kit:
    git:
      url: https://github.com/Moseco/kana_kit.git
      ref: master

@jeroen-meijer
Copy link
Owner

@HighLiuk Hi there, thanks for pinging me on this. Apologies to @Moseco for not reviewing this in time.

I'll add a test for the edge case that this PR accounts for, then merge and push a new release.
Love that there's more people in the community working on Japanese content :)

@HighLiuk
Copy link

Thank you @jeroen-meijer! Sure, I love Japanese :)
I'm working on a personal non-commercial project of a flutter app in which you can read your favourite manga with support for text detection and OCR, furigana completion, dictionary and so on.
I already use your package and it's great. But for the dictionary feature I need the JM Dict package to work, so that's why we're here :)

Thank you for the reply. I was worried this project was abandoned.

@jeroen-meijer
Copy link
Owner

That's so cool! Let me know when it's out or if you have any updates :)
HMU @ [email protected]

@jeroen-meijer
Copy link
Owner

@HighLiuk I'm almost done preparing the PR with various fixes and updates, including this one.

Do you have two or three test cases I can use to add tests for this edge-case? Wanna make sure I get these right. Thx.

@HighLiuk
Copy link

@jeroen-meijer as far as I know, the test cases provided by @Moseco are the only (rare) cases:
シークヮーサー
シマウヮー

@jeroen-meijer
Copy link
Owner

@HighLiuk The PR is submitted (#24), but the fix made by @Moseco doesn't actually perform the translation correctly.

I can't work on this for the rest of the day, but feel free to have a look at the PR and propose a fix I can implement instead.

(I added two tests for this edge-case so just run the test suite to see if you fixed it. Good luck!)

@Moseco
Copy link
Author

Moseco commented Sep 24, 2024

@jeroen-meijer I am not sure why it is not working now in your release branch. I will try to investigate today and push a fix.

Edit: initial comment was incorrect about the problem.

@Moseco
Copy link
Author

Moseco commented Sep 24, 2024

@jeroen-meijer I quickly looked at the problem. it seems to correctly convert the rare kana when it is going from katakana to romaji but it does not do it correctly when going from hiragana to romaji. What would the best fix be? To change the hepburn map to account for 'ゎー'?

@jeroen-meijer
Copy link
Owner

@Moseco Done. See #24

@jeroen-meijer
Copy link
Owner

This issue should now be fixed in version 2.1.0, which has just released.

@jeroen-meijer
Copy link
Owner

CC @HighLiuk

@14h4i
Copy link

14h4i commented Oct 14, 2024

@jeroen-meijer I can't update to 2.1.0 because this issue #25

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
5 participants