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

list_keyboards sets skipTerritory, but never uses it #18

Closed
AdamWill opened this issue Aug 25, 2023 · 2 comments
Closed

list_keyboards sets skipTerritory, but never uses it #18

AdamWill opened this issue Aug 25, 2023 · 2 comments

Comments

@AdamWill
Copy link
Contributor

The list_keyboards function in langtable.py has a variable called skipTerritory that gets set if the language DB has an entry for the combined languageId, scriptId and territoryId or combined languageId and territoryId, but it is never used. We always go down the if territoryId in _territories_db: path, regardless of whether skipTerritory is True or False.

I'd send a PR for this, but I'm not sure if the desired fix would be to skip the territory path if skipTerritory is True, or remove the setting of skipTerritory entirely.

@mike-fabian mike-fabian self-assigned this Aug 28, 2023
@mike-fabian mike-fabian added this to To do in Mike’s github project board via automation Aug 28, 2023
@mike-fabian
Copy link
Owner

The list_keyboards function in langtable.py has a variable called skipTerritory that gets set if the language DB has an entry for the combined languageId, scriptId and territoryId or combined languageId and territoryId, but it is never used. We always go down the if territoryId in _territories_db: path, regardless of whether skipTerritory is True or False.

I'd send a PR for this, but I'm not sure if the desired fix would be to skip the territory path if skipTerritory is True, or remove the setting of skipTerritory entirely.

Thank you very much!

Currently that makes no difference, but it could make a difference in future.

When more specific information is already available in _languages_db for a combination of

   languageId+'_'+scriptId+'_'+territoryId
   languageId+'_'+scriptId
   languageId+'_'+territoryId

then the less specific information in _territories_db should be ignored.

For example, list_inputmethods() already used skipTerritory correctly. If I disable that like this:

diff --git a/langtable/langtable.py b/langtable/langtable.py
index 7e36c50..32eb5a3 100644
--- a/langtable/langtable.py
+++ b/langtable/langtable.py
@@ -2119,7 +2119,7 @@ def list_inputmethods(concise=True, show_weights=False, languageId = None, scrip
                     ranked_inputmethods[inputmethod] *= extra_bonus
                 ranked_inputmethods[inputmethod] *= language_bonus
     territory_bonus = 1
-    if territoryId in _territories_db and not skipTerritory:
+    if territoryId in _territories_db:# and not skipTerritory:
         for inputmethod in _territories_db[territoryId].inputmethods:
             if _territories_db[territoryId].inputmethods[inputmethod] != 0:
                 if inputmethod not in ranked_inputmethods:

then this test case fails:

**********************************************************************
File "/local/mfabian/src/langtable/test_cases.py", line 544, in __main__.dummy
Failed example:
    list_inputmethods(languageId="sd", scriptId="Arab", territoryId="IN") # doctest: +NORMALIZE_WHITESPACE
Expected:
        []
Got:
    ['ibus/m17n:hi:inscript2', 'ibus/m17n:bn:inscript2', 'ibus/m17n:te:inscript2', 'ibus/m17n:mr:inscript2', 'ibus/m17n:ta:inscript2', 'ibus/m17n:ur:phonetic', 'ibus/m17n:gu:inscript2',
us/m17n:kn:inscript2', 'ibus/m17n:ml:inscript2', 'ibus/m17n:or:inscript2', 'ibus/m17n:pa:inscript2-guru', 'ibus/m17n:as:inscript2', 'ibus/m17n:mai:inscript2', 'ibus/m17n:sat:inscript2-d
, 'ibus/m17n:ks:inscript2-deva', 'ibus/m17n:kok:inscript2-deva', 'ibus/m17n:sd:inscript2-deva', 'ibus/m17n:doi:inscript2-deva', 'ibus/m17n:mni:inscript2-beng', 'ibus/m17n:brx:inscript2-
', 'ibus/m17n:ne:inscript2-deva']
**********************************************************************

I.e. even though scriptId="Arab" was explicitely requested, the
fallback to the territory "IN" gets all the Indian input methods
listed, even though none of them currently supports Arabic script.

So the fallback to territoryId should not be used if more specific information was found earlier.

Mike’s github project board automation moved this from To do to Done Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants