Merge pull request xbmc#27530 from CrystalP/fix-langcodeexpander

[i18n] Fix LangCodeExpander Bugs

Author: CrystalP

xbmc/utils/LangCodeExpander.cpp

@@ -148,10 +148,32 @@ bool CLangCodeExpander::ConvertToISO6392B(const std::string& strCharCode,
     std::string charCode(strCharCode);
     StringUtils::ToLower(charCode);
 
-    if (std::ranges::binary_search(LanguageCodesByIso639_2b, charCode, {}, &ISO639::iso639_2b) ||
-        (checkWin32Locales &&
-         std::ranges::binary_search(LanguageCodesByWin_Id, charCode, {}, &ISO639::win_id)) ||
-        CIso3166_1::ContainsAlpha3(charCode))
+    if (std::ranges::binary_search(LanguageCodesByIso639_2b, charCode, {}, &ISO639::iso639_2b))
+    {
+      strISO6392B = charCode;
+      return true;
+    }
+
+    if (const auto bCode{CIso639_2::TCodeToBCode(charCode)}; bCode.has_value())
+    {
+      strISO6392B = bCode.value();
+      return true;
+    }
+
+    if (checkWin32Locales)
+    {
+      const auto it =
+          std::ranges::lower_bound(LanguageCodesByWin_Id, charCode, {}, &ISO639::win_id);
+      if (it != LanguageCodesByWin_Id.end() && it->win_id == charCode)
+      {
+        strISO6392B = it->iso639_2b;
+        return true;
+      }
+    }
+
+    // Match against country last to avoid stealing possible matches from previous conditions
+    //! @todo what's this legacy logic for?
+    if (CIso3166_1::ContainsAlpha3(charCode))
     {
       strISO6392B = charCode;
       return true;
@@ -173,9 +195,12 @@ bool CLangCodeExpander::ConvertToISO6392B(const std::string& strCharCode,
     }
 
     // Try search on language addons
-    strISO6392B = g_langInfo.ConvertEnglishNameToAddonLocale(strCharCode);
-    if (!strISO6392B.empty())
+    if (const std::string addonLang = g_langInfo.ConvertEnglishNameToAddonLocale(strCharCode);
+        !addonLang.empty())
+    {
+      strISO6392B = addonLang;
       return true;
+    }
   }
   return false;
 }
@@ -360,29 +385,25 @@ bool CLangCodeExpander::ReverseLookup(const std::string& desc, std::string& code
     }
   }
 
-  std::string_view name = descTmp;
+  if (const auto ret = CIso639_1::LookupByName(descTmp); ret.has_value())
   {
-    auto ret = CIso639_1::LookupByName(name);
-    if (ret)
-    {
-      code = *ret;
-      return true;
-    }
+    code = *ret;
+    return true;
   }
+
+  if (const auto ret = CIso639_2::LookupByName(descTmp); ret.has_value())
   {
-    auto ret = CIso639_2::LookupByName(name);
-    if (ret)
-    {
-      code = *ret;
-      return true;
-    }
+    code = *ret;
+    return true;
   }
 
   // Find on language addons
-  code = g_langInfo.ConvertEnglishNameToAddonLocale(descTmp);
-  if (!code.empty())
+  if (const std::string addonLang = g_langInfo.ConvertEnglishNameToAddonLocale(descTmp);
+      !addonLang.empty())
+  {
+    code = addonLang;
     return true;
-
+  }
   return false;
 }
 

xbmc/utils/test/TestLangCodeExpander.cpp

@@ -51,24 +51,36 @@ TEST(TestLangCodeExpander, ConvertToISO6392B)
   std::string refstr;
   std::string varstr;
 
+  // ISO 639-2 with identical B and T forms
   refstr = "eng";
   EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("en", varstr));
   EXPECT_EQ(refstr, varstr);
 
   EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("eng", varstr));
   EXPECT_EQ(refstr, varstr);
 
+  // ISO 639-2/B
+  refstr = "fre";
+  EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("fre", varstr));
+  EXPECT_EQ(refstr, varstr);
+
+  // ISO 639-2/T
+  refstr = "cze";
+  EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("ces", varstr));
+  EXPECT_EQ(refstr, varstr);
+
   // win_id != iso639_2b
-  refstr = "fra";
+  refstr = "fre";
   EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("fra", varstr, true));
   EXPECT_EQ(refstr, varstr);
 
+  //! \todo analyze, v.suspicious. What old situation required matching languages with regions?
   // Region code
   refstr = "bol";
   EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("bol", varstr));
   EXPECT_EQ(refstr, varstr);
 
-  // non existent
+  // non-existent or non-convertible
   refstr = "invalid";
   varstr = "invalid";
   EXPECT_FALSE(g_LangCodeExpander.ConvertToISO6392B("ac", varstr));
@@ -77,6 +89,9 @@ TEST(TestLangCodeExpander, ConvertToISO6392B)
   EXPECT_FALSE(g_LangCodeExpander.ConvertToISO6392B("aaa", varstr));
   EXPECT_EQ(refstr, varstr);
 
+  EXPECT_FALSE(g_LangCodeExpander.ConvertToISO6392B("en-US", varstr));
+  EXPECT_EQ(refstr, varstr);
+
   // Full english name, case insensitive
   refstr = "eng";
   EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("English", varstr, true));
@@ -85,11 +100,6 @@ TEST(TestLangCodeExpander, ConvertToISO6392B)
   refstr = "eng";
   EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("english", varstr, true));
   EXPECT_EQ(refstr, varstr);
-
-  // Existing bug
-  //refstr = "ger";
-  //EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392B("deu", varstr));
-  //EXPECT_EQ(refstr, varstr);
 }
 
 TEST(TestLangCodeExpander, ConvertToISO6391)
@@ -166,13 +176,7 @@ TEST(TestLangCodeExpander, ConvertToISO6392T)
   EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392T("deu", varstr, true));
   EXPECT_EQ(refstr, varstr);
 
-  // Should return deu instead of failing. Side effect of failure in Convert but maybe happens for historical reasons and something
-  // relies on that.
-  // For some reason, the function allows the language code to match with the region 'deu',
-  // but since it's not a valid ISO639-2/B code, it cannot be translated to its ISO-639-2/T code
-  refstr = "invalid";
-  varstr = "invalid";
-  EXPECT_FALSE(g_LangCodeExpander.ConvertToISO6392T("deu", varstr));
+  EXPECT_TRUE(g_LangCodeExpander.ConvertToISO6392T("deu", varstr));
   EXPECT_EQ(refstr, varstr);
 }