HugoFara / lwt

Learn languages by reading! A language learning app stemmed from Learning with Texts (LWT).
https://hugofara.github.io/lwt/
The Unlicense
169 stars 19 forks source link

[BUG] Mecab required when not set #155

Closed ProgramComputer closed 10 months ago

ProgramComputer commented 1 year ago

Even when mecab was not set for Japanese; it would require it installed. Also changed phonetic reading to take LanguageName as that is the key used in the language table and not the language code.

HugoFara commented 10 months ago

Hi @ProgramComputer, thank you for the pull request! I implemented the manually changes, here are the details:

Have a great day!

ProgramComputer commented 10 months ago

@HugoFara

  • For MeCab, I simply implemented your changes. See 887ac35.

https://github.com/HugoFara/lwt/commit/887ac35047af534d67f2a478eeb07058d0c2a07d is a UI fix. The cause of MeCab bug is on https://github.com/HugoFara/lwt/blob/b64cd6ae7ba8eca2537c57fb144f7f3a754a41aa/inc/session_utility.php#L4810 . Apply this patch

diff --git a/inc/session_utility.php b/inc/session_utility.php
index e69c18e3..f5275e53 100644
--- a/inc/session_utility.php
+++ b/inc/session_utility.php
@@ -4807,7 +4807,9 @@ function phonetic_reading($text, $lang)
 {
     global $tbpref;
     // Many languages are already phonetic
-    if (!str_starts_with($lang, "ja") && !str_starts_with($lang, "jp")) {
+    $mecab = get_first_value('select LgRegexpWordCharacters as value from ' . $tbpref . 'languages where LgName = ' . '"Japanese"');
+
+    if (!str_starts_with($lang, "ja") && !str_starts_with($lang, "jp") && $mecab != "mecab") {
         return $text;
     }
HugoFara commented 10 months ago

That makes sense, actually I may create a new function phoneticReading($text, $lg_id), it will be better. I that case I will bundle it in the next minor release.

ProgramComputer commented 10 months ago

@HugoFara

That makes sense, actually I may create a new function phoneticReading($text, $lg_id), it will be better. I that case I will bundle it in the next minor release.

Lg_ID can work after seeing https://github.com/HugoFara/lwt/issues/103#issuecomment-1499924588 for Mecab to be enabled independent of language.

HugoFara commented 10 months ago

I added a new parameter word_parsing (on LWT_DATA.language). That should be a more flexible way to address the issue. For now I don't want to touch the TTS feature any more since it already underwent a massive change with #153, and I don't see bugs for now...

ProgramComputer commented 10 months ago

Bug not fixed yet, opened #182.