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

[BUG] Mecab required when not set #155

Closed
wants to merge 8 commits into from

Conversation

ProgramComputer
Copy link

@ProgramComputer ProgramComputer commented Oct 6, 2023

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.

@ProgramComputer ProgramComputer changed the base branch from master to dev December 26, 2023 15:19
HugoFara added a commit that referenced this pull request Dec 27, 2023
@HugoFara HugoFara added the bug Something isn't working label Dec 27, 2023
@HugoFara
Copy link
Owner

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

Have a great day!

@HugoFara HugoFara closed this Dec 27, 2023
@HugoFara HugoFara linked an issue Dec 27, 2023 that may be closed by this pull request
HugoFara added a commit that referenced this pull request Dec 27, 2023
Fixes/Implements #149: adds counting in japanese.
Fixes #155: MeCab was always set for japanese.
Fixes romanization with MeCab not displayed.
Typo and link fixing in documentation.
@ProgramComputer
Copy link
Author

ProgramComputer commented Dec 27, 2023

@HugoFara

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

887ac35 is a UI fix. The cause of MeCab bug is on

if (!str_starts_with($lang, "ja") && !str_starts_with($lang, "jp")) {
. 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
Copy link
Owner

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.

@HugoFara HugoFara reopened this Dec 28, 2023
@ProgramComputer
Copy link
Author

ProgramComputer commented Dec 31, 2023

@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 #103 (comment) for Mecab to be enabled independent of language.

HugoFara added a commit that referenced this pull request Jan 1, 2024
@HugoFara
Copy link
Owner

HugoFara commented Jan 1, 2024

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...

@HugoFara HugoFara closed this Jan 1, 2024
ProgramComputer pushed a commit to ProgramComputer/lwt that referenced this pull request Jan 1, 2024
HugoFara added a commit that referenced this pull request Jan 3, 2024
New databse migration strategy.
Fixes feeds (#168).
Adds missing documentation to Docker (#146, #160).
Changes in PHP and JS globals.
Fixes reading position was not set.
Read text through API (#153, #155).
Fixes word was not saved/deleted.
Fixes #170 and #69.
Updates API (#175).
Adds dependency to php-xml (#178, #181).
Updates makefile (#179).
Adds MeCab support on Mac (#135).
Adds the option to hide/show word romanization (#119).
Raises URL size limit to 2048 (#144).
@ProgramComputer
Copy link
Author

Bug not fixed yet, opened #182.

HugoFara added a commit that referenced this pull request Jan 5, 2024
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.

TTS and unrecognized language names
2 participants