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

Add multilingual typing #565

Merged
merged 11 commits into from
Mar 21, 2022

Conversation

Helium314
Copy link
Contributor

@Helium314 Helium314 commented Mar 12, 2022

This is a very basic implementation of multilingual typing, but it is working.

A second dictionary is added in and queried for suggestions, so that suggestions of both languages show up. The secondary language has lower weight, so mostly suggestions of the main language are shown. If more than two consecutive typed words exist only in the secondary language, main and secondary language switch roles.

This is only a draft PR, because a lot of stuff needs to be done:

  • secondary language is currently hardcoded, but in the end must be selectable (or disabled) by user
  • the secondary dictionary is only loaded once, and can't ever be changed or closed
  • the current main language is not stored, so it will reset whenever the app is closed
  • spell checker (if enabled) only works for the initial main language and does not switch (not sure if this can actually be changed)
  • current main language should be shown somewhere (on space bar?)
  • some tuning of suggestion weights main vs seccondary language

Before I continue working on this I would like some feedback on whether the way I did it is acceptable at all, and maybe some general comments on what should be taken care of.

fixes #167
fixes #7

revert switch between main and secondary locale
tune suggestion weights for each locale based on mConfidence
more preparations for making secondary dictionary selectable
isValidSpellingWord now uses both dictionaries (called only by spell checker)
@Helium314
Copy link
Contributor Author

Small update:

  • There are some places where the dictionary is reset if locale is not as expected, so I removed the switching of main and secondary dictionary.
  • Tuned the weights somewhat, maybe a bit too aggressively towards showing both languages
  • Allow not using a secondary dictionary by setting to null (though currently it's never null and still can't be changed).
  • Call functions like clearUserHistoryDictionary or closeDictionaries on main and secondary dictionary.
  • Use both dictionaries for isValidSpellingWord. This is used by the spell checker only (which didn't work before because I didn't realize it has it's own instance of DictionaryFacilitatorImpl). So spell checker works, but always adds new words to the main locale

(initial to-do still applies, with the update regarding spell checker)

@Helium314
Copy link
Contributor Author

Helium314 commented Mar 17, 2022

@MajeurAndroid
I had a few toughts about selecting secondary locale.

  • The secondary locale needs to have a dictionary
  • The main and secondary locales need to have the same script

So to offer a list of secondary locales to select from, we need to get a list of locales with dictionaries that have the same script as the current main locale.
Can we conveniently get such a list? Otherwise my plan is to simply put some hardcoded list that needs to be updated manually.

Then there is the question how to select secondary locales: Should it be one secondary locale? Or one for each main locale? Or one for each script?
Currently I would go for one secondary locale, as it's the most simple way... and it can still be improved later.

@Helium314
Copy link
Contributor Author

Helium314 commented Mar 20, 2022

Current state:

  • Secondary locale can be selected in advanced settings / experimental, and selection is limited to languages with dictionary and with the same script as system locale
    • properly working dictionary detection would need Compress dictionaries and allow users to add more #569
    • the script limitation is a workaround as I would expect the user to type mostly in script of the system locale. Ideally there should be a secondary locale allowed for each input locale, but I don't know where to get enabled input locales. Help is highly appreciated!
      Anyway, I think the current behavior is acceptable for an experimental feature.
  • Language display on space bar works
  • Locale weights for getting suggestions in the currently used locale seem ok to me

I think the PR is mostly ready (once #569 is done), only the spell checker needs some work:

  • something in the recent 3 commits broke checking in secondary locale
  • add to dictionary should allow selecting to which locale the word should be added

@Helium314
Copy link
Contributor Author

Spell check in secondary locale works again, but I see no way of setting the locale that is used when adding to personal dictionary...

Secondary locale selection is moved to its own screen, with one secondary locale per enabled keyboard locale.
Some small bugs still need fixing.

@Helium314
Copy link
Contributor Author

Bugs fixed, so I think this is almost ready.

Only problem with actually using it: currently detection of existing dicitonaries works only when manually putting a dicitonary in /data/data/org.dslul.openboard.inputmethod.latin/files/dicts/<locale>/*.dict (requires root file manager). This will happen automatically after #569.

@Helium314 Helium314 marked this pull request as ready for review March 21, 2022 09:03
@MajeurAndroid
Copy link
Member

I quickly reviewed your changes, they seem good and I think you covered all the fundamentals of it.
This is a big feature, and I'm having troubles to test and debug this with only a PR and an APK. I'll create a new branch called "feature-multi-locale" so everybody would be able to contribute to this, and we can make sure it's ready enough before merging it to master.
Is it ok for you ? I'm not sure if you can modify PR's target branch.

Else, I tested the APK and I am having a bit of hard times to setup a secondary locale:

  • There is an issue when default locale is set to "use system language".
  • I'm having a force close (IndexArrayOutOfBound I guess on some locale list) when selecting a locale).

This will be far simpler to work and debug on a common branch.
Thanks a lot for all your contributions !

@MajeurAndroid MajeurAndroid changed the base branch from master to feature-multi-locale March 21, 2022 11:31
@Helium314
Copy link
Contributor Author

Thanks, so I'll look into the issues once I'm (mostly) done with #569, as the way I arranged it this will be required for working detection of dicitonaries.

And I definitely agree some longer test will be needed... like I never even thought about trying with default option "use system language".

@Helium314
Copy link
Contributor Author

I added a hardcoded locale list for easier testing, and fixed the problem for default language.

Do you have more details about the crash?

@MajeurAndroid
Copy link
Member

I think we should do one thing at a time, one branch for multi-locale, one for external-dicts. Then when both will be finished, we'll look into compatibility between multi-locale.

@MajeurAndroid MajeurAndroid merged commit e4f89af into openboard-team:feature-multi-locale Mar 21, 2022
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.

Multi-language dictionary on same keyboard multilanguage: autodetect typing language
2 participants