From 53e3064f243190dfc01c4ba6bcdf29b6092fd050 Mon Sep 17 00:00:00 2001 From: Hasso Date: Fri, 3 Mar 2023 14:48:16 -0600 Subject: [PATCH 1/9] Fix StackOverfloweException in obsolete Create method --- src/L10NSharp/LocalizationManager.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/L10NSharp/LocalizationManager.cs b/src/L10NSharp/LocalizationManager.cs index 2d9568df..141d5238 100644 --- a/src/L10NSharp/LocalizationManager.cs +++ b/src/L10NSharp/LocalizationManager.cs @@ -101,7 +101,14 @@ public static ILocalizationManager Create(TranslationMemory kind, string desired string relativeSettingPathForLocalizationFolder, Icon applicationIcon, string emailForSubmissions, params string[] namespaceBeginnings) { - return Create(kind, desiredUiLangId, + if (kind != TranslationMemory.XLiff) + { + throw new ArgumentException($@"Unknown translation memory kind {kind}. Only XLiff + is supported.", + nameof(kind)); + } + + return Create(desiredUiLangId, appId, appName, appVersion, directoryOfInstalledFiles, relativeSettingPathForLocalizationFolder, applicationIcon, emailForSubmissions, From f9c8de78ca1383903dfd6fe9824ed4cfc7449927 Mon Sep 17 00:00:00 2001 From: Hasso Date: Fri, 3 Mar 2023 17:14:03 -0600 Subject: [PATCH 2/9] WIP: #109: find specific languages when generics are specified If a client tries to initialize a LocalizationManager with only a language code, and we have a language available with that code plus extra information, use that language without prompting the user. This addresses https://github.com/sillsdev/l10nsharp/issues/109 This addresses https://jira.sil.org/browse/LT-21232 --- .../XLiffUtils/XLiffLocalizedStringCache.cs | 8 +- .../LocalizationManagerTestsBase.cs | 105 ++++++++++++++---- .../XLiffLocalizedStringCacheTests.cs | 12 +- 3 files changed, 97 insertions(+), 28 deletions(-) diff --git a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs index f998181d..37fbe6cc 100644 --- a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs +++ b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs @@ -172,10 +172,12 @@ private void LoadXliffAndUpdateExistingLanguageMap(string langId) // If we're asked to try to load the xliff for es-ES and don't find one, // Try loading the one for es. var pieces = langId.Split('-'); - if (pieces.Length <= 1) - return; - if (!_unloadedXliffDocuments.TryRemove(pieces[0], out file)) + if (pieces.Length == 1 || !_unloadedXliffDocuments.TryRemove(pieces[0], out file)) + { return; + // Another possibility is that the lang folder is "es-ES" but the client is requesting only "es". + // TODO: actual implementation here: + } } var xliffDoc = XLiffDocument.Read(file); diff --git a/src/L10NSharpTests/LocalizationManagerTestsBase.cs b/src/L10NSharpTests/LocalizationManagerTestsBase.cs index dcfb8110..260d08bb 100644 --- a/src/L10NSharpTests/LocalizationManagerTestsBase.cs +++ b/src/L10NSharpTests/LocalizationManagerTestsBase.cs @@ -326,17 +326,22 @@ public string GetDynamicStringOrEnglish(string langId, string uiLangId) } } - //NOTE: the TestName parameter is only here to work around an NUnit bug in which - //NUnit doesn't run all the test cases when some differ only by the values in an array parameter - //cases where we expect to get back the english in the code - [TestCase(new[] { "en" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_Works_1")] - [TestCase(new[] { "en", "fr" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_Works_2")] - [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_Works_3")] // our arabic doesn't have a translation of 'blah', so fall to the code's English - [TestCase(new[] { "zz", "en", "fr" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_Works_4")] - //cases where we expect to get back the French - [TestCase(new[] { "fr" }, "blahInFrench", "fr", TestName = "GetString_OverloadThatTakesListOfLanguages_Works_5")] - [TestCase(new[] { "fr", "en" }, "blahInFrench", "fr", TestName = "GetString_OverloadThatTakesListOfLanguages_Works_6")] - [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "GetString_OverloadThatTakesListOfLanguages_Works_7")] // our arabic doesn't have a translation of 'blah', so fall to French + /// + /// Tests that GetString returns the first language in order of preference with an available string + /// + /// + /// NOTE: the TestName parameter is only here to work around an NUnit bug in which + /// NUnit doesn't run all the test cases when some differ only by the values in an array parameter. + /// + //cases where we expect to get back the english in the code: + [TestCase(new[] { "en" }, "blahInEnglishCode", "en", TestName = "en; finds en")] + [TestCase(new[] { "en", "fr" }, "blahInEnglishCode", "en", TestName = "en,fr; finds en")] + [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "ar,en; finds en")] // our arabic doesn't have a translation of 'blah', so fall to the code's English + [TestCase(new[] { "zz", "en", "fr" }, "blahInEnglishCode", "en", TestName = "zz,en,fr; finds en")] + //cases where we expect to get back the French: + [TestCase(new[] { "fr" }, "blahInFrench", "fr", TestName = "fr; finds fr")] + [TestCase(new[] { "fr", "en" }, "blahInFrench", "fr", TestName = "fr,en; finds fr")] + [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "ar,fr,en; finds fr")] // our arabic doesn't have a translation of 'blah', so fall to French public void GetString_OverloadThatTakesListOfLanguages_Works(IEnumerable preferredLangIds, string expectedResult, string expectedLanguage) { using(var folder = new TempFolder()) @@ -420,17 +425,22 @@ public void GetDynamicStringInEnglish_NoDefault_FindsEnglishWithFolders() } } - //NOTE: the TestName parameter is only here to work around an NUnit bug in which - //NUnit doesn't run all the test cases when some differ only by the values in an array parameter - //cases where we expect to get back the english in the code - [TestCase(new[] { "en" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_WorksWithFolders_1")] - [TestCase(new[] { "en", "fr" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_WorksWithFolders_2")] - [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_WorksWithFolders_3")] // our arabic doesn't have a translation of 'blah', so fall to the code's English - [TestCase(new[] { "zz", "en", "fr" }, "blahInEnglishCode", "en", TestName = "GetString_OverloadThatTakesListOfLanguages_WorksWithFolders_4")] - //cases where we expect to get back the French - [TestCase(new[] { "fr" }, "blahInFrench", "fr", TestName = "GetString_OverloadThatTakesListOfLanguages_WorksWithFolders_5")] - [TestCase(new[] { "fr", "en" }, "blahInFrench", "fr", TestName = "GetString_OverloadThatTakesListOfLanguages_WorksWithFolders_6")] - [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "GetString_OverloadThatTakesListOfLanguages_WorksWithFolders_7")] // our arabic doesn't have a translation of 'blah', so fall to French + /// + /// Tests that GetString returns the first language in order of preference with an available string + /// + /// + /// NOTE: the TestName parameter is only here to work around an NUnit bug in which + /// NUnit doesn't run all the test cases when some differ only by the values in an array parameter. + /// + //cases where we expect to get back the english in the code: + [TestCase(new[] { "en" }, "blahInEnglishCode", "en", TestName = "en; finds en")] + [TestCase(new[] { "en", "fr" }, "blahInEnglishCode", "en", TestName = "en,fr; finds en")] + [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "ar,en; finds en")] // our arabic doesn't have a translation of 'blah', so fall to the code's English + [TestCase(new[] { "zz", "en", "fr" }, "blahInEnglishCode", "en", TestName = "zz,en,fr; finds en")] + //cases where we expect to get back the French: + [TestCase(new[] { "fr" }, "blahInFrench", "fr", TestName = "fr; finds fr")] + [TestCase(new[] { "fr", "en" }, "blahInFrench", "fr", TestName = "fr,en; finds fr")] + [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "ar,fr,en; finds fr")] // our arabic doesn't have a translation of 'blah', so fall to French public void GetString_OverloadThatTakesListOfLanguages_WorksWithFolders(IEnumerable preferredLangIds, string expectedResult, string expectedLanguage) { LocalizationManager.UseLanguageCodeFolders = true; @@ -568,6 +578,29 @@ private void AddSpanishTranslation(string folderPath) spanishDoc.Save(Path.Combine(folderPath, LocalizationManager.GetTranslationFileNameForLanguage(AppId, "es"))); } + private void AddChineseChineseTranslation(string folderPath) + { + var chineseDoc = CreateNewDocument(null, "en", "zh-CN"); + // first unit + var tu = CreateTransUnit("theId", false, + CreateTransUnitVariant("en", "from English Translation"), + CreateTransUnitVariant("zh-CN", "from Chinese (China) Translation"), + "Tzh-CNt", TranslationStatus.Approved); + chineseDoc.AddTransUnit(tu); + // second unit + var tu2 = CreateTransUnit("notUsedId", false, + CreateTransUnitVariant("en", "no longer used English text"), + CreateTransUnitVariant("zh-CN", "no longer used Chinese (China) text"), + null, TranslationStatus.Approved); + chineseDoc.AddTransUnit(tu2); + // third unit + var tu3 = CreateTransUnit("blahId", false, + CreateTransUnitVariant("en", "blah"), + CreateTransUnitVariant("zh-CN", "中文")); + chineseDoc.AddTransUnit(tu3); + chineseDoc.Save(Path.Combine(folderPath, LocalizationManager.GetTranslationFileNameForLanguage(AppId, "zh-CN"))); + } + protected void AddRandomTranslation(string langId, string folderPath) { var doc = CreateNewDocument(null, "en", langId); @@ -929,5 +962,33 @@ public void TestMappingLanguageCodesToAvailable() Assert.AreEqual("es-ES", languageIdUsed); } } + + [Test] + public void TestMappingLanguageCodesToAvailable_FindsSpecificGivenGeneric() + { + LocalizationManager.SetUILanguage("en", true); + LocalizationManagerInternal.LoadedManagers.Clear(); + using (var folder = new TempFolder()) + { + var installedFolder = Path.Combine(folder.Path, "installed"); + AddEnglishTranslation(installedFolder, null); + AddChineseChineseTranslation(installedFolder); + var manager = LocalizationManager.Create("zh", AppId, AppName, AppVersion, installedFolder, + $"Temp/{Path.GetFileName(folder.Path)}/user", null, null, new string[] { }); + LocalizationManagerInternal.LoadedManagers[AppId] = (ILocalizationManagerInternal)manager; + + var langs = LocalizationManager.GetAvailableLocalizedLanguages(); + Assert.That(langs, Is.EquivalentTo(new[] { "en", "zh-CN" })); + + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh"), Is.True, "zh should find zh-CN"); + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh-CN"), Is.True, "zh-CN should find zh-CN"); + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "en"), Is.True, "en should find en"); + + // Check asking for a specific form of the language when we have only a different specific form. + var str = LocalizationManager.GetString("theId", ".", "", new []{ "zh" }, out var languageIdUsed); + Assert.AreEqual("from Chinese (China) Translation", str); + Assert.AreEqual("zh-CN", languageIdUsed); + } + } } } diff --git a/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs b/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs index 9df03712..271b9690 100644 --- a/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs +++ b/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs @@ -1,4 +1,4 @@ -using L10NSharp.XLiffUtils; +using L10NSharp.XLiffUtils; using NUnit.Framework; namespace L10NSharp.Tests @@ -28,7 +28,7 @@ public class XLiffLocalizedStringCacheTests [TestCase(1, "\u0632\u0020\"{\u200E\"{0\u200F.", false, TestName="CheckSubstitutionMarkers_17")] [TestCase(3, "{\u09E6} \u09A7\u09B0\u09A3\u09BE '{1}' \u09AC\u09B9\u09BE\u09B0 {\u09E8}pt.", false, TestName="CheckSubstitutionMarkers_18")] - public void CheckStringsForValidSubstitionMarkers(int markerCount, string formatting, bool isValid) + public void CheckStringsForValidSubstitutionMarkers(int markerCount, string formatting, bool isValid) { Assert.That(XLiffLocalizedStringCache.CheckForValidSubstitutionMarkers(markerCount, formatting, "a.b"), Is.EqualTo(isValid)); @@ -60,7 +60,7 @@ public void TryToFixBrokenSubstitutionMarkers(string badFormat, string goodForma Assert.That(XLiffLocalizedStringCache.CheckForValidSubstitutionMarkers(3, result, "a.b"), Is.EqualTo(true)); } - // This checks for a wider range of substition marker numbers. + // This checks for a wider range of substitution marker numbers. [Test] [TestCase("\u0645 '{\u200E'{10 \u0627", "\u0645 \u200E'{10}'\u200F \u0627", TestName = "FixBrokenSubstitution_Works_1")] [TestCase("\u0647 '{\u200E'{11\u0646\u0627", "\u0647 \u200E'{11}'\u200F\u0646\u0627", TestName = "FixBrokenSubstitution_Works_2")] @@ -79,5 +79,11 @@ public void FixBrokenSubstitutionMarkersOnly(string badFormat, string goodFormat var result = XLiffLocalizedStringCache.FixBrokenFormattingString(badFormat); Assert.That(result, Is.EqualTo(goodFormat)); } + + [Test] + public void TryGetDocument() + { + //var sut = new XLiffLocalizedStringCache(); + } } } From ccc16ab7737d51fb0c2c0a103c04641ba9dfd975 Mon Sep 17 00:00:00 2001 From: Hasso Date: Mon, 6 Mar 2023 16:48:30 -0600 Subject: [PATCH 3/9] Address code review comments, write failing tests --- src/L10NSharp/LocalizationManagerInternal.cs | 27 ++++-- .../LocalizationManagerTestsBase.cs | 93 ++++++++++++++++--- .../XLiffLocalizedStringCacheTests.cs | 6 -- 3 files changed, 97 insertions(+), 29 deletions(-) diff --git a/src/L10NSharp/LocalizationManagerInternal.cs b/src/L10NSharp/LocalizationManagerInternal.cs index 8cc2ecac..68594c7e 100644 --- a/src/L10NSharp/LocalizationManagerInternal.cs +++ b/src/L10NSharp/LocalizationManagerInternal.cs @@ -13,6 +13,7 @@ using System.Windows.Forms; using L10NSharp.UI; using L10NSharp.XLiffUtils; +// ReSharper disable StaticMemberInGenericType - these static fields are parameter-independent namespace L10NSharp { @@ -35,10 +36,24 @@ internal static class LocalizationManagerInternal /// internal static ConcurrentDictionary MapToExistingLanguage = new ConcurrentDictionary(); - // If documents are loaded lazily, this lock must be held while loading one, or while using MapToExistingLanguage - // in a way that might cause loading. + /// + /// If documents are loaded lazily, this lock must be held while loading one, or while using MapToExistingLanguage + /// in a way that might cause loading. + /// internal static object LazyLoadLock = new object(); + /// + /// Function to choose a fallback language during construction. Overridable by unit tests. + /// + internal static Func ChooseFallbackLanguage = (desiredUiLangId, icon) => + { + using (var dlg = new LanguageChoosingDialog(L10NCultureInfo.GetCultureInfo(desiredUiLangId), icon)) + { + dlg.ShowDialog(); + return dlg.SelectedLanguage; + } + }; + private static readonly Dictionary> s_loadedManagers = new Dictionary>(); @@ -50,7 +65,7 @@ private static ILocalizationManager Create(string desiredUiLangId, string appId, if (string.IsNullOrEmpty(relativeSettingPathForLocalizationFolder)) relativeSettingPathForLocalizationFolder = appName; else if (Path.IsPathRooted(relativeSettingPathForLocalizationFolder)) - throw new ArgumentException("Relative (non-rooted) path expected", nameof(relativeSettingPathForLocalizationFolder)); + throw new ArgumentException(@"Relative (non-rooted) path expected", nameof(relativeSettingPathForLocalizationFolder)); var directoryOfWritableTranslationFiles = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), relativeSettingPathForLocalizationFolder, "localizations"); @@ -72,11 +87,7 @@ private static ILocalizationManager Create(string desiredUiLangId, string appId, if (!IsDesiredUiCultureAvailable(desiredUiLangId)) { - using (var dlg = new LanguageChoosingDialog(L10NCultureInfo.GetCultureInfo(desiredUiLangId), applicationIcon)) - { - dlg.ShowDialog(); - desiredUiLangId = dlg.SelectedLanguage; - } + desiredUiLangId = ChooseFallbackLanguage(desiredUiLangId, applicationIcon); } LocalizationManager.SetUILanguage(desiredUiLangId, false); diff --git a/src/L10NSharpTests/LocalizationManagerTestsBase.cs b/src/L10NSharpTests/LocalizationManagerTestsBase.cs index 260d08bb..16f16ae9 100644 --- a/src/L10NSharpTests/LocalizationManagerTestsBase.cs +++ b/src/L10NSharpTests/LocalizationManagerTestsBase.cs @@ -336,12 +336,12 @@ public string GetDynamicStringOrEnglish(string langId, string uiLangId) //cases where we expect to get back the english in the code: [TestCase(new[] { "en" }, "blahInEnglishCode", "en", TestName = "en; finds en")] [TestCase(new[] { "en", "fr" }, "blahInEnglishCode", "en", TestName = "en,fr; finds en")] - [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "ar,en; finds en")] // our arabic doesn't have a translation of 'blah', so fall to the code's English + [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "ar,en; finds en")] // our arabic doesn't have a translation of 'blah', so fall back to the code's English [TestCase(new[] { "zz", "en", "fr" }, "blahInEnglishCode", "en", TestName = "zz,en,fr; finds en")] //cases where we expect to get back the French: [TestCase(new[] { "fr" }, "blahInFrench", "fr", TestName = "fr; finds fr")] [TestCase(new[] { "fr", "en" }, "blahInFrench", "fr", TestName = "fr,en; finds fr")] - [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "ar,fr,en; finds fr")] // our arabic doesn't have a translation of 'blah', so fall to French + [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "ar,fr,en; finds fr")] // our arabic doesn't have a translation of 'blah', so fall back to French public void GetString_OverloadThatTakesListOfLanguages_Works(IEnumerable preferredLangIds, string expectedResult, string expectedLanguage) { using(var folder = new TempFolder()) @@ -433,14 +433,14 @@ public void GetDynamicStringInEnglish_NoDefault_FindsEnglishWithFolders() /// NUnit doesn't run all the test cases when some differ only by the values in an array parameter. /// //cases where we expect to get back the english in the code: - [TestCase(new[] { "en" }, "blahInEnglishCode", "en", TestName = "en; finds en")] - [TestCase(new[] { "en", "fr" }, "blahInEnglishCode", "en", TestName = "en,fr; finds en")] - [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "ar,en; finds en")] // our arabic doesn't have a translation of 'blah', so fall to the code's English - [TestCase(new[] { "zz", "en", "fr" }, "blahInEnglishCode", "en", TestName = "zz,en,fr; finds en")] + [TestCase(new[] { "en" }, "blahInEnglishCode", "en", TestName = "en; finds en in folders")] + [TestCase(new[] { "en", "fr" }, "blahInEnglishCode", "en", TestName = "en,fr; finds en in folders")] + [TestCase(new[] { "ar", "en" }, "blahInEnglishCode", "en", TestName = "ar,en; finds en in folders")] // our arabic doesn't have a translation of 'blah', so fall back to the code's English + [TestCase(new[] { "zz", "en", "fr" }, "blahInEnglishCode", "en", TestName = "zz,en,fr; finds en in folders")] //cases where we expect to get back the French: - [TestCase(new[] { "fr" }, "blahInFrench", "fr", TestName = "fr; finds fr")] - [TestCase(new[] { "fr", "en" }, "blahInFrench", "fr", TestName = "fr,en; finds fr")] - [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "ar,fr,en; finds fr")] // our arabic doesn't have a translation of 'blah', so fall to French + [TestCase(new[] { "fr" }, "blahInFrench", "fr", TestName = "fr; finds fr in folders")] + [TestCase(new[] { "fr", "en" }, "blahInFrench", "fr", TestName = "fr,en; finds fr in folders")] + [TestCase(new[] { "ar", "fr", "en" }, "blahInFrench", "fr", TestName = "ar,fr,en; finds fr in folders")] // our arabic doesn't have a translation of 'blah', so fall back to French public void GetString_OverloadThatTakesListOfLanguages_WorksWithFolders(IEnumerable preferredLangIds, string expectedResult, string expectedLanguage) { LocalizationManager.UseLanguageCodeFolders = true; @@ -578,14 +578,14 @@ private void AddSpanishTranslation(string folderPath) spanishDoc.Save(Path.Combine(folderPath, LocalizationManager.GetTranslationFileNameForLanguage(AppId, "es"))); } - private void AddChineseChineseTranslation(string folderPath) + private void AddChineseOfChinaTranslation(string folderPath) { var chineseDoc = CreateNewDocument(null, "en", "zh-CN"); // first unit var tu = CreateTransUnit("theId", false, CreateTransUnitVariant("en", "from English Translation"), CreateTransUnitVariant("zh-CN", "from Chinese (China) Translation"), - "Tzh-CNt", TranslationStatus.Approved); + "Test", TranslationStatus.Approved); chineseDoc.AddTransUnit(tu); // second unit var tu2 = CreateTransUnit("notUsedId", false, @@ -596,11 +596,34 @@ private void AddChineseChineseTranslation(string folderPath) // third unit var tu3 = CreateTransUnit("blahId", false, CreateTransUnitVariant("en", "blah"), - CreateTransUnitVariant("zh-CN", "中文")); + CreateTransUnitVariant("zh-CN", "中文(中国) blah")); chineseDoc.AddTransUnit(tu3); chineseDoc.Save(Path.Combine(folderPath, LocalizationManager.GetTranslationFileNameForLanguage(AppId, "zh-CN"))); } + private void AddChineseOfTaiwanTranslation(string folderPath) + { + var chineseDoc = CreateNewDocument(null, "en", "zh-TW"); + // first unit + var tu = CreateTransUnit("theId", false, + CreateTransUnitVariant("en", "from English Translation"), + CreateTransUnitVariant("zh-TW", "from Chinese (Taiwan) Translation"), + "Test", TranslationStatus.Approved); + chineseDoc.AddTransUnit(tu); + // second unit + var tu2 = CreateTransUnit("notUsedId", false, + CreateTransUnitVariant("en", "no longer used English text"), + CreateTransUnitVariant("zh-TW", "no longer used Chinese (Taiwan) text"), + null, TranslationStatus.Approved); + chineseDoc.AddTransUnit(tu2); + // third unit + var tu3 = CreateTransUnit("blahId", false, + CreateTransUnitVariant("en", "blah"), + CreateTransUnitVariant("zh-TW", "中文(Taiwan) blah")); + chineseDoc.AddTransUnit(tu3); + chineseDoc.Save(Path.Combine(folderPath, LocalizationManager.GetTranslationFileNameForLanguage(AppId, "zh-TW"))); + } + protected void AddRandomTranslation(string langId, string folderPath) { var doc = CreateNewDocument(null, "en", langId); @@ -972,7 +995,9 @@ public void TestMappingLanguageCodesToAvailable_FindsSpecificGivenGeneric() { var installedFolder = Path.Combine(folder.Path, "installed"); AddEnglishTranslation(installedFolder, null); - AddChineseChineseTranslation(installedFolder); + AddChineseOfChinaTranslation(installedFolder); + LocalizationManagerInternal.ChooseFallbackLanguage = (langTag, icon) => + throw new NotImplementedException($"Expected to find a match for {langTag}"); var manager = LocalizationManager.Create("zh", AppId, AppName, AppVersion, installedFolder, $"Temp/{Path.GetFileName(folder.Path)}/user", null, null, new string[] { }); LocalizationManagerInternal.LoadedManagers[AppId] = (ILocalizationManagerInternal)manager; @@ -986,8 +1011,46 @@ public void TestMappingLanguageCodesToAvailable_FindsSpecificGivenGeneric() // Check asking for a specific form of the language when we have only a different specific form. var str = LocalizationManager.GetString("theId", ".", "", new []{ "zh" }, out var languageIdUsed); - Assert.AreEqual("from Chinese (China) Translation", str); - Assert.AreEqual("zh-CN", languageIdUsed); + Assert.That(str, Is.EqualTo("from Chinese (China) Translation")); + Assert.That(languageIdUsed, Is.EqualTo("zh-CN")); + } + } + + [Test] + public void TestMappingLanguageCodesToAvailable_AmbiguousOptions_PromptsUser([Values("zh-CN", "zh-TW")] string choice) + { + LocalizationManager.SetUILanguage("en", true); + LocalizationManagerInternal.LoadedManagers.Clear(); + using (var folder = new TempFolder()) + { + var installedFolder = Path.Combine(folder.Path, "installed"); + AddEnglishTranslation(installedFolder, null); + AddChineseOfChinaTranslation(installedFolder); + AddChineseOfTaiwanTranslation(installedFolder); + var userPromptCount = 0; + LocalizationManagerInternal.ChooseFallbackLanguage = (langTag, icon) => + { + userPromptCount++; + Assert.That(langTag, Is.EqualTo("zh")); + return choice; + }; + var manager = LocalizationManager.Create("zh", AppId, AppName, AppVersion, installedFolder, + $"Temp/{Path.GetFileName(folder.Path)}/user", null, null, new string[] { }); + LocalizationManagerInternal.LoadedManagers[AppId] = (ILocalizationManagerInternal)manager; + + var langs = LocalizationManager.GetAvailableLocalizedLanguages(); + Assert.That(langs, Is.EquivalentTo(new[] { "en", "zh-CN", "zh-TW" })); + + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh"), Is.True, "zh should find the chosen language"); + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh-CN"), Is.True, "zh-CN should find zh-CN"); + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh-TW"), Is.True, "zh-TW should find zh-TW"); + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "en"), Is.True, "en should find en"); + + // Check asking for a specific form of the language when we have only a different specific form. + var str = LocalizationManager.GetString("theId", ".", "", new []{ "zh" }, out var languageIdUsed); + Assert.That(str, Is.EqualTo("from Chinese (China) Translation")); + Assert.That(languageIdUsed, Is.EqualTo(choice)); + Assert.That(userPromptCount, Is.EqualTo(1)); } } } diff --git a/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs b/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs index 271b9690..d76780c1 100644 --- a/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs +++ b/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs @@ -79,11 +79,5 @@ public void FixBrokenSubstitutionMarkersOnly(string badFormat, string goodFormat var result = XLiffLocalizedStringCache.FixBrokenFormattingString(badFormat); Assert.That(result, Is.EqualTo(goodFormat)); } - - [Test] - public void TryGetDocument() - { - //var sut = new XLiffLocalizedStringCache(); - } } } From 446291a4793d4142375eef8043528f2b8d418715 Mon Sep 17 00:00:00 2001 From: Hasso Date: Mon, 6 Mar 2023 17:08:25 -0600 Subject: [PATCH 4/9] Update changelog; fix tests --- CHANGELOG.md | 4 +++ README.md | 11 ++++++-- src/L10NSharp/LocalizationManager.cs | 3 +++ .../XLiffUtils/XLiffLocalizedStringCache.cs | 27 ++++++++++++++----- .../LocalizationManagerTestsBase.cs | 12 +++------ 5 files changed, 40 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08b83bcb..efd1b684 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `LocalizationManager.Create` methods without `TranslationMemory kind` parameter +### Fixed + +- `LocalizationManager.Create("es"` loads `es-ES` if it is the best match + ### Deprecated - `LocalizationManager.Create` methods with `TranslationMemory kind` parameter diff --git a/README.md b/README.md index 7e2babb3..ed60372b 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ To use L10NSharp in your application, simply call the `Create` method on `Locali passing the location of the translation memory files and some other information: ```csharp -using (var lm = LocalizationManager.Create(TranslationMemory.XLiff, lang, "SampleApp", +using (var lm = LocalizationManager.Create(lang, "SampleApp", "SampleApp", productVersion, directoryOfInstalledXliffFiles, "MyCompany/L10NSharpSample", icon, "sample@example.com", "SampleApp") { @@ -33,9 +33,16 @@ By default, `directoryOfInstalledFiles` contains files named `Whatever.lang.xlf` are the language tags. These names must match the target-language declared in the XLF for lazy loading to work properly. If the target-language is a multi-part tag (like `es-ES`), the lang component in the file path may be either the full tag (`Whatever.es-ES.xlf` or -`es-ES/Whatever.xlf`) or its first component, the actual language tag (`Whatever.es.xlf` +`es-ES/Whatever.xlf`) or its first component, the bare language tag (`Whatever.es.xlf` or `es/Whatever.xlf`). +### Selecting a Language + +If an exact match for the requested language is not available, L10NSharp will try to find the best available language. For example, if the client +requests `es` but only `Whatever.es-ES.xlf` is available, `Whatever.es-ES.xlf` will be loaded automatically, and vise versa. However, if the client +requests `es` and both `Whatever.es-ES.xlf` and `Whatever.es-MX.xlf` are available, or if no `Whatever.es[-details].xlf` is available, a dialog will +inform the user that the selected language is not available and prompt the user to select from the available languages. + ## Thread safety In general, L10NSharp is not written with thread safety in mind; callers should ensure diff --git a/src/L10NSharp/LocalizationManager.cs b/src/L10NSharp/LocalizationManager.cs index 141d5238..69dea087 100644 --- a/src/L10NSharp/LocalizationManager.cs +++ b/src/L10NSharp/LocalizationManager.cs @@ -308,7 +308,10 @@ public static string UILanguageId // The current version of Mono does not define a CultureInfo for "zh", so // it tends to throw exceptions when we try to use just plain "zh". if (s_uiLangId == "zh-CN") + { + Debug.Fail("hooray; it's tested!"); return s_uiLangId; + } } // Otherwise, we want the culture.neutral version. int i = s_uiLangId.IndexOf('-'); diff --git a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs index 37fbe6cc..11f24402 100644 --- a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs +++ b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs @@ -169,14 +169,23 @@ private void LoadXliffAndUpdateExistingLanguageMap(string langId) { // Often an xliff in a plain lang folder (like "es") contains // a target-language that is more specific (like "es-ES"). - // If we're asked to try to load the xliff for es-ES and don't find one, - // Try loading the one for es. + // Another possibility is that the lang folder is "es-ES" but the client is requesting only "es". In either case, try to find a + // sensible match automatically before prompting the user. If, however, there is more than one match and no clear best, + // allow the user to choose. var pieces = langId.Split('-'); + // If we're asked to try to load the xliff for es-ES and don't find one, try loading the one for es. if (pieces.Length == 1 || !_unloadedXliffDocuments.TryRemove(pieces[0], out file)) { - return; - // Another possibility is that the lang folder is "es-ES" but the client is requesting only "es". - // TODO: actual implementation here: + // Either we were already trying to find 'es' or we could find neither 'es-ES' nor 'es'. Look for 'es-*'. + var available = _unloadedXliffDocuments.Keys.Where(key => key.Split('-')[0] == pieces[0]).ToList(); + if (available.Count == 1) + { + _unloadedXliffDocuments.TryRemove(available[0], out file); + } + else + { + return; + } } } @@ -210,6 +219,11 @@ private void LoadXliffAndUpdateExistingLanguageMap(string langId) } } + if (langId == LocalizationManager.kDefaultLang) + { + return; + } + XliffDocuments.TryAdd(targetLang, xliffDoc); var defunctUnits = new List(); foreach (var tu in xliffDoc.File.Body.TransUnitsUnordered.ToList()) // need a list here because we may modify it while enumerating @@ -218,8 +232,7 @@ private void LoadXliffAndUpdateExistingLanguageMap(string langId) // We assume the default language Xliff has only current IDs, and therefore don't look for orphans in that case. // This guards against cases such as recently occurred in Bloom, where a dynamic ID EditTab.AddPageDialog.Title // was regarded as an obsolete id for PublishTab.Upload.Title - if (langId != LocalizationManager.kDefaultLang && - DefaultXliffDocument.GetTransUnitForId(tu.Id) == null && + if (DefaultXliffDocument.GetTransUnitForId(tu.Id) == null && !tu.Id.EndsWith(kToolTipSuffix) && !tu.Id.EndsWith(kShortcutSuffix)) { //if we couldn't find it, maybe the id just changed and then if so re-id it. diff --git a/src/L10NSharpTests/LocalizationManagerTestsBase.cs b/src/L10NSharpTests/LocalizationManagerTestsBase.cs index 16f16ae9..50987a95 100644 --- a/src/L10NSharpTests/LocalizationManagerTestsBase.cs +++ b/src/L10NSharpTests/LocalizationManagerTestsBase.cs @@ -997,7 +997,7 @@ public void TestMappingLanguageCodesToAvailable_FindsSpecificGivenGeneric() AddEnglishTranslation(installedFolder, null); AddChineseOfChinaTranslation(installedFolder); LocalizationManagerInternal.ChooseFallbackLanguage = (langTag, icon) => - throw new NotImplementedException($"Expected to find a match for {langTag}"); + throw new NotImplementedException($"{langTag} shouldn't have stumped us"); var manager = LocalizationManager.Create("zh", AppId, AppName, AppVersion, installedFolder, $"Temp/{Path.GetFileName(folder.Path)}/user", null, null, new string[] { }); LocalizationManagerInternal.LoadedManagers[AppId] = (ILocalizationManagerInternal)manager; @@ -1036,21 +1036,17 @@ public void TestMappingLanguageCodesToAvailable_AmbiguousOptions_PromptsUser([Va }; var manager = LocalizationManager.Create("zh", AppId, AppName, AppVersion, installedFolder, $"Temp/{Path.GetFileName(folder.Path)}/user", null, null, new string[] { }); + Assert.That(userPromptCount, Is.EqualTo(1)); LocalizationManagerInternal.LoadedManagers[AppId] = (ILocalizationManagerInternal)manager; var langs = LocalizationManager.GetAvailableLocalizedLanguages(); Assert.That(langs, Is.EquivalentTo(new[] { "en", "zh-CN", "zh-TW" })); + Assert.That(LocalizationManager.UILanguageId, Is.EqualTo(choice)); - Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh"), Is.True, "zh should find the chosen language"); + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh"), Is.False, "zh is ambiguous"); Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh-CN"), Is.True, "zh-CN should find zh-CN"); Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh-TW"), Is.True, "zh-TW should find zh-TW"); Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "en"), Is.True, "en should find en"); - - // Check asking for a specific form of the language when we have only a different specific form. - var str = LocalizationManager.GetString("theId", ".", "", new []{ "zh" }, out var languageIdUsed); - Assert.That(str, Is.EqualTo("from Chinese (China) Translation")); - Assert.That(languageIdUsed, Is.EqualTo(choice)); - Assert.That(userPromptCount, Is.EqualTo(1)); } } } From 6bd225051ad10ae59d8ef29a601fda8f93522c57 Mon Sep 17 00:00:00 2001 From: Hasso Date: Tue, 7 Mar 2023 16:27:15 -0600 Subject: [PATCH 5/9] Update comments +semver: major I don't expect this to break anything, but it does change behaviour for existing clients --- README.md | 2 +- src/L10NSharp/LocalizationManagerInternal.cs | 3 ++ .../XLiffUtils/XLiffLocalizedStringCache.cs | 5 +- .../LocalizationManagerTestsBase.cs | 51 +++++++++---------- 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index ed60372b..9ba722c8 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ or `es/Whatever.xlf`). ### Selecting a Language If an exact match for the requested language is not available, L10NSharp will try to find the best available language. For example, if the client -requests `es` but only `Whatever.es-ES.xlf` is available, `Whatever.es-ES.xlf` will be loaded automatically, and vise versa. However, if the client +requests `es` but only `Whatever.es-ES.xlf` is available, `Whatever.es-ES.xlf` will be loaded automatically, and vice versa. However, if the client requests `es` and both `Whatever.es-ES.xlf` and `Whatever.es-MX.xlf` are available, or if no `Whatever.es[-details].xlf` is available, a dialog will inform the user that the selected language is not available and prompt the user to select from the available languages. diff --git a/src/L10NSharp/LocalizationManagerInternal.cs b/src/L10NSharp/LocalizationManagerInternal.cs index 68594c7e..d312feaa 100644 --- a/src/L10NSharp/LocalizationManagerInternal.cs +++ b/src/L10NSharp/LocalizationManagerInternal.cs @@ -620,6 +620,9 @@ public static string GetDynamicStringOrEnglish(string appId, string id, string e /// not yet contain data about this language. In that case, get the lock for /// loading xliff docs, load any relevant ones, and try again. /// + /// + /// must load "es-ES" before "es" will map to "es-ES". + /// internal static string MapToExistingLanguageIfPossible(string langId) { if (string.IsNullOrEmpty(langId)) diff --git a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs index 11f24402..addb7df6 100644 --- a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs +++ b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs @@ -162,6 +162,7 @@ private void MergeXliffFilesIntoCache(IEnumerable xliffFiles) /// (or its primary language, if different) and update MapToExistingLanguage according /// to what we find. /// Use only from TryGetDocument. Should hold LazyLoadLock. + /// Tries to find partial matches, similar to /// private void LoadXliffAndUpdateExistingLanguageMap(string langId) { @@ -171,7 +172,7 @@ private void LoadXliffAndUpdateExistingLanguageMap(string langId) // a target-language that is more specific (like "es-ES"). // Another possibility is that the lang folder is "es-ES" but the client is requesting only "es". In either case, try to find a // sensible match automatically before prompting the user. If, however, there is more than one match and no clear best, - // allow the user to choose. + // allow the user to choose. (https://github.com/sillsdev/l10nsharp/issues/109) var pieces = langId.Split('-'); // If we're asked to try to load the xliff for es-ES and don't find one, try loading the one for es. if (pieces.Length == 1 || !_unloadedXliffDocuments.TryRemove(pieces[0], out file)) @@ -184,6 +185,8 @@ private void LoadXliffAndUpdateExistingLanguageMap(string langId) } else { + // Loading more than one partial match at this point would arbitrarily map the bare language ID to the first available, + // which might not be correct return; } } diff --git a/src/L10NSharpTests/LocalizationManagerTestsBase.cs b/src/L10NSharpTests/LocalizationManagerTestsBase.cs index 50987a95..a474503e 100644 --- a/src/L10NSharpTests/LocalizationManagerTestsBase.cs +++ b/src/L10NSharpTests/LocalizationManagerTestsBase.cs @@ -245,7 +245,7 @@ public void GetDynamicString_EnglishAndArabicHaveDifferentValuesOfEnglishString_ { using(var folder = new TempFolder()) { - SetupManager(folder, "en"); + SetupManager(folder); //This was the original assertion, and it worked: // Assert.AreEqual("from English Translation", LocalizationManager.GetDynamicString(AppId, "theId", "some default")); //However, later I decided, I don't care what is in the English Translation, either. If the c# code just gave a new @@ -267,7 +267,7 @@ public void GetDynamicString_EnglishTranslationHasDIfferentStringThanParamater_P { using(var folder = new TempFolder()) { - SetupManager(folder, "en"); + SetupManager(folder); Assert.AreEqual("from c# code", LocalizationManager.GetDynamicString(AppId, "theId", "from c# code")); } } @@ -287,7 +287,7 @@ public void GetDynamicStringInEnglish_NoDefault_FindsEnglish() { using (var folder = new TempFolder()) { - SetupManager(folder, "en"); + SetupManager(folder); Assert.That(LocalizationManager.GetDynamicString(AppId, "blahId", null), Is.EqualTo("blah"), "With no default supplied, should find saved English"); } } @@ -297,7 +297,7 @@ public void GetDynamicStringOrEnglish_LmDisposed_GivesUsefulException() { using (var folder = new TempFolder()) { - SetupManager(folder, "en"); + SetupManager(folder); Assert.That(LocalizationManager.GetDynamicString(AppId, "blahId", null), Is.EqualTo("blah"), "With no default supplied, should find saved English"); using (var extra = CreateLocalizationManager("nonsense", "more nonsense", "1.0")) { @@ -402,11 +402,6 @@ public void GetUiLanguages_FindsAllWithFolders() } } - int compareCultureTags(CultureInfo first, CultureInfo second) - { - return first.IetfLanguageTag.CompareTo(second.IetfLanguageTag); - } - [Test] public void GetDynamicStringInEnglish_NoDefault_FindsEnglishWithFolders() { @@ -471,7 +466,7 @@ public void GetUiLanguages_AzeriHasHackedNativeName() /// /// - "Installs" English, Arabic, and French /// - Sets the UI language - /// - Constructs a LocalizationManager and adds it to LocalizationManagerInternal.LoadedManagers[AppId] + /// - Constructs a LocalizationManager and adds it to LocalizationManagerInternal<T>.LoadedManagers[AppId] /// public void SetupManager(TempFolder folder, string uiLanguageId = LocalizationManager.kDefaultLang) { @@ -737,7 +732,7 @@ public void GetAvailableUILanguageTags_FindsThreeLanguages() { using (var folder = new TempFolder()) { - SetupManager(folder, "en"); + SetupManager(folder); var lm = LocalizationManagerInternal.LoadedManagers.Values.First(); var tags = lm.GetAvailableUILanguageTags().ToArray(); Assert.That(tags.Length, Is.EqualTo(4)); @@ -951,38 +946,38 @@ public void TestMappingLanguageCodesToAvailable() // Check that we return the provided string for English. var str = LocalizationManager.GetString("theId", "This is a test!", "This is only a test?", new[]{ "en", "fr", "ar" }, out var languageIdUsed); - Assert.AreEqual("This is a test!", str); - Assert.AreEqual("en", languageIdUsed); + Assert.That(str, Is.EqualTo("This is a test!"), "seeking en"); + Assert.That(languageIdUsed, Is.EqualTo("en"), "seeking en"); // Check that asking for a specific form of English still returns the provided string. str = LocalizationManager.GetString("theId", "This is a test!", "This is only a test?", new []{ "en-US", "es-ES", "fr-FR" }, out languageIdUsed); - Assert.AreEqual("This is a test!", str); - Assert.AreEqual("en", languageIdUsed); + Assert.That(str, Is.EqualTo("This is a test!"), "seeking en-US"); + Assert.That(languageIdUsed, Is.EqualTo("en"), "seeking en-US"); // Check that we return the string from the second language when the first language doesn't have the string. str = LocalizationManager.GetString("theId", "This is a test!", "This is only a test?", new[]{ "fr", "ar", "es" }, out languageIdUsed); - Assert.AreEqual("inArabic", str); - Assert.AreEqual("ar", languageIdUsed); + Assert.That(str, Is.EqualTo("inArabic"), "seeking fr, ar, es"); + Assert.That(languageIdUsed, Is.EqualTo("ar"), "seeking fr, ar, es"); // Check that we return the string from the first language when it exists. str = LocalizationManager.GetString("theId", "This is a test!", "This is only a test?", new []{ "es-ES", "en", "fr" }, out languageIdUsed); - Assert.AreEqual("from Spanish Translation", str); - Assert.AreEqual("es-ES", languageIdUsed); + Assert.That(str, Is.EqualTo("from Spanish Translation"), "seeking es-ES"); + Assert.That(languageIdUsed, Is.EqualTo("es-ES"), "seeking es-ES"); // Check asking for the general form of the language when we have only a specific form. str = LocalizationManager.GetString("theId", "This is a test!", "This is only a test?", new []{ "es", "en", "fr" }, out languageIdUsed); - Assert.AreEqual("from Spanish Translation", str); - Assert.AreEqual("es-ES", languageIdUsed); + Assert.That(str, Is.EqualTo("from Spanish Translation"), "seeking es"); + Assert.That(languageIdUsed, Is.EqualTo("es-ES"), "seeking es"); // Check asking for a specific form of the language when we have only the general form. str = LocalizationManager.GetString("theId", "This is a test!", "This is only a test?", new []{ "ar-AR", "en", "fr" }, out languageIdUsed); - Assert.AreEqual("inArabic", str); - Assert.AreEqual("ar", languageIdUsed); + Assert.That(str, Is.EqualTo("inArabic"), "seeking ar-AR"); + Assert.That(languageIdUsed, Is.EqualTo("ar"), "seeking ar-AR"); // Check asking for a specific form of the language when we have only a different specific form. str = LocalizationManager.GetString("theId", "This is a test!", "This is only a test?", new []{ "es-MX", "en-GB", "fr-FR" }, out languageIdUsed); - Assert.AreEqual("from Spanish Translation", str); - Assert.AreEqual("es-ES", languageIdUsed); + Assert.That(str, Is.EqualTo("from Spanish Translation"), "seeking es-MX"); + Assert.That(languageIdUsed, Is.EqualTo("es-ES"), "seeking es-MX"); } } @@ -1007,6 +1002,7 @@ public void TestMappingLanguageCodesToAvailable_FindsSpecificGivenGeneric() Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh"), Is.True, "zh should find zh-CN"); Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh-CN"), Is.True, "zh-CN should find zh-CN"); + Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "zh-TW"), Is.True, "zh-TW should find zh-CN"); Assert.That(LocalizationManager.GetIsStringAvailableForLangId("theId", "en"), Is.True, "en should find en"); // Check asking for a specific form of the language when we have only a different specific form. @@ -1024,6 +1020,9 @@ public void TestMappingLanguageCodesToAvailable_AmbiguousOptions_PromptsUser([Va using (var folder = new TempFolder()) { var installedFolder = Path.Combine(folder.Path, "installed"); + // ReSharper disable once AssignNullToNotNullAttribute + var userRelativeFolder = Path.Combine("Temp", Path.GetFileName(Path.GetDirectoryName(folder.Path)), + Path.GetFileName(folder.Path), "user"); AddEnglishTranslation(installedFolder, null); AddChineseOfChinaTranslation(installedFolder); AddChineseOfTaiwanTranslation(installedFolder); @@ -1035,7 +1034,7 @@ public void TestMappingLanguageCodesToAvailable_AmbiguousOptions_PromptsUser([Va return choice; }; var manager = LocalizationManager.Create("zh", AppId, AppName, AppVersion, installedFolder, - $"Temp/{Path.GetFileName(folder.Path)}/user", null, null, new string[] { }); + userRelativeFolder, null, null, new string[] { }); Assert.That(userPromptCount, Is.EqualTo(1)); LocalizationManagerInternal.LoadedManagers[AppId] = (ILocalizationManagerInternal)manager; From 5d4b70031b863c1c1fc6aefafb7cd24bf0cff6a9 Mon Sep 17 00:00:00 2001 From: Hasso Date: Tue, 7 Mar 2023 16:31:57 -0600 Subject: [PATCH 6/9] Fix capitalization of Xliff in an internal class --- src/CheckOrFixXliff/Program.cs | 4 ++-- src/ExtractXliff/Program.cs | 2 +- src/L10NSharp/LocalizationManagerInternal.cs | 2 +- src/L10NSharp/XLiffUtils/XLiffBody.cs | 6 +++--- src/L10NSharp/XLiffUtils/XLiffLocalizationManager.cs | 6 +++--- src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs | 4 ++-- src/L10NSharp/XLiffUtils/XLiffTransUnitUpdater.cs | 4 ++-- src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/CheckOrFixXliff/Program.cs b/src/CheckOrFixXliff/Program.cs index 56872600..3ca7ed45 100644 --- a/src/CheckOrFixXliff/Program.cs +++ b/src/CheckOrFixXliff/Program.cs @@ -137,7 +137,7 @@ private static void FixBrokenTransUnit(XElement tu, XmlNamespaceManager namespac var targetValue = target.Value; if (!string.IsNullOrWhiteSpace(targetValue)) { - var targetFixed = XLiffLocalizedStringCache.FixBrokenFormattingString(targetValue); + var targetFixed = XliffLocalizedStringCache.FixBrokenFormattingString(targetValue); if (targetFixed != target.Value) target.SetValue(targetFixed); } @@ -219,7 +219,7 @@ private static ErrorState CheckFormatStringMarkers(string filename) var okay = CheckForExactlyMatchingSubstitutionMarkers(tu.Id, dictSourceMarkers, dictTargetMarkers); if (!okay && retval == ErrorState.Okay) retval = ErrorState.Warning; - if (!XLiffLocalizedStringCache.CheckForValidSubstitutionMarkers(dictSourceMarkers.Count, tu.Target.Value, tu.Id, _quiet)) + if (!XliffLocalizedStringCache.CheckForValidSubstitutionMarkers(dictSourceMarkers.Count, tu.Target.Value, tu.Id, _quiet)) { _mangledTargets.Add(tu.Id); retval = ErrorState.Error; diff --git a/src/ExtractXliff/Program.cs b/src/ExtractXliff/Program.cs index 185b5968..4a00f9c4 100644 --- a/src/ExtractXliff/Program.cs +++ b/src/ExtractXliff/Program.cs @@ -121,7 +121,7 @@ private static void Main(string[] args) // to feed into the constructor the LocalizedStringCache that does some heavy lifting for us in // creating the XliffDocument from the newly extracted localized strings. var lm = new XLiffLocalizationManager(_fileOriginal, _fileOriginal, _fileProductVersion); - var stringCache = new XLiffLocalizedStringCache(lm, false); + var stringCache = new XliffLocalizedStringCache(lm, false); foreach (var locInfo in localizedStrings) stringCache.UpdateLocalizedInfo(locInfo); diff --git a/src/L10NSharp/LocalizationManagerInternal.cs b/src/L10NSharp/LocalizationManagerInternal.cs index d312feaa..606e0b3e 100644 --- a/src/L10NSharp/LocalizationManagerInternal.cs +++ b/src/L10NSharp/LocalizationManagerInternal.cs @@ -621,7 +621,7 @@ public static string GetDynamicStringOrEnglish(string appId, string id, string e /// loading xliff docs, load any relevant ones, and try again. /// /// - /// must load "es-ES" before "es" will map to "es-ES". + /// must load "es-ES" before "es" will map to "es-ES". /// internal static string MapToExistingLanguageIfPossible(string langId) { diff --git a/src/L10NSharp/XLiffUtils/XLiffBody.cs b/src/L10NSharp/XLiffUtils/XLiffBody.cs index ac5e8713..dc1c0646 100644 --- a/src/L10NSharp/XLiffUtils/XLiffBody.cs +++ b/src/L10NSharp/XLiffUtils/XLiffBody.cs @@ -83,7 +83,7 @@ public ListWrapper TransUnitsForXml get { var result = TransUnitsUnordered.ToList(); - result.Sort(XLiffLocalizedStringCache.TuComparer); + result.Sort(XliffLocalizedStringCache.TuComparer); return new ListWrapper(result, this); } } @@ -119,10 +119,10 @@ internal XLiffTransUnit GetTransUnitForId(string id) /// internal XLiffTransUnit GetTransUnitForOrphan(XLiffTransUnit orphan, XLiffBody source) { - var terminalIdToMatch = XLiffLocalizedStringCache.GetTerminalIdPart(orphan.Id); + var terminalIdToMatch = XliffLocalizedStringCache.GetTerminalIdPart(orphan.Id); var defaultTextToMatch = GetDefaultVariantValue(orphan); return TransUnitsUnordered.FirstOrDefault(tu => - XLiffLocalizedStringCache.GetTerminalIdPart(tu.Id) == + XliffLocalizedStringCache.GetTerminalIdPart(tu.Id) == terminalIdToMatch // require last part of ID to match && GetDefaultVariantValue(tu) == defaultTextToMatch // require text to match && source?.GetTransUnitForId(tu.Id) == null); // and translation does not already have an element for this diff --git a/src/L10NSharp/XLiffUtils/XLiffLocalizationManager.cs b/src/L10NSharp/XLiffUtils/XLiffLocalizationManager.cs index 2dc76c6e..5d0c3a83 100644 --- a/src/L10NSharp/XLiffUtils/XLiffLocalizationManager.cs +++ b/src/L10NSharp/XLiffUtils/XLiffLocalizationManager.cs @@ -136,7 +136,7 @@ internal XLiffLocalizationManager(string appId, string origExtension, string app ComponentCache = new Dictionary(); ToolTipCtrls = new Dictionary(); - StringCache = new XLiffLocalizedStringCache(this); + StringCache = new XliffLocalizedStringCache(this); LocalizableComponents = new Dictionary>(); } @@ -209,7 +209,7 @@ private void CreateOrUpdateDefaultXliffFileIfNecessary( var fileStream = File.Open(DefaultStringFilePath, FileMode.Create, FileAccess.ReadWrite, FileShare.None); fileStream.Close(); - var stringCache = new XLiffLocalizedStringCache(this, false); + var stringCache = new XliffLocalizedStringCache(this, false); var extractedInfo = ExtractStringsFromCode(Name, additionalLocalizationMethods, namespaceBeginnings); if (extractedInfo != null) @@ -578,7 +578,7 @@ public void SaveIfDirty(ICollection langIdsToForceCreate) { try { - ((XLiffLocalizedStringCache)StringCache).SaveIfDirty(langIdsToForceCreate); + ((XliffLocalizedStringCache)StringCache).SaveIfDirty(langIdsToForceCreate); } catch (IOException e) { diff --git a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs index addb7df6..6aa274c6 100644 --- a/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs +++ b/src/L10NSharp/XLiffUtils/XLiffLocalizedStringCache.cs @@ -13,7 +13,7 @@ namespace L10NSharp.XLiffUtils { /// ---------------------------------------------------------------------------------------- - internal class XLiffLocalizedStringCache : LocalizedStringCache, ILocalizedStringCache + internal class XliffLocalizedStringCache : LocalizedStringCache, ILocalizedStringCache { private readonly XLiffTransUnitUpdater _tuUpdater; @@ -40,7 +40,7 @@ internal class XLiffLocalizedStringCache : LocalizedStringCache, ILocalizedStrin /// Loads the string cache from all the specified Xliff files /// /// ------------------------------------------------------------------------------------ - internal XLiffLocalizedStringCache(ILocalizationManager owningManager, bool loadAvailableXliffFiles = true) + internal XliffLocalizedStringCache(ILocalizationManager owningManager, bool loadAvailableXliffFiles = true) { OwningManager = (XLiffLocalizationManager)owningManager; if (loadAvailableXliffFiles) diff --git a/src/L10NSharp/XLiffUtils/XLiffTransUnitUpdater.cs b/src/L10NSharp/XLiffUtils/XLiffTransUnitUpdater.cs index dea7263a..02b656b9 100644 --- a/src/L10NSharp/XLiffUtils/XLiffTransUnitUpdater.cs +++ b/src/L10NSharp/XLiffUtils/XLiffTransUnitUpdater.cs @@ -19,13 +19,13 @@ internal class XLiffTransUnitUpdater // with the value of kOSNewline. internal string _literalNewline = "\\n"; - private readonly XLiffLocalizedStringCache _stringCache; + private readonly XliffLocalizedStringCache _stringCache; private readonly string _defaultLang; private bool _updated; /// ------------------------------------------------------------------------------------ - internal XLiffTransUnitUpdater(XLiffLocalizedStringCache cache) + internal XLiffTransUnitUpdater(XliffLocalizedStringCache cache) { _stringCache = cache; _defaultLang = LocalizationManager.kDefaultLang; diff --git a/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs b/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs index d76780c1..0b75a97e 100644 --- a/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs +++ b/src/L10NSharpTests/XLiffLocalizedStringCacheTests.cs @@ -30,7 +30,7 @@ public class XLiffLocalizedStringCacheTests [TestCase(3, "{\u09E6} \u09A7\u09B0\u09A3\u09BE '{1}' \u09AC\u09B9\u09BE\u09B0 {\u09E8}pt.", false, TestName="CheckSubstitutionMarkers_18")] public void CheckStringsForValidSubstitutionMarkers(int markerCount, string formatting, bool isValid) { - Assert.That(XLiffLocalizedStringCache.CheckForValidSubstitutionMarkers(markerCount, + Assert.That(XliffLocalizedStringCache.CheckForValidSubstitutionMarkers(markerCount, formatting, "a.b"), Is.EqualTo(isValid)); } @@ -54,10 +54,10 @@ public void CheckStringsForValidSubstitutionMarkers(int markerCount, string form [TestCase("\u0632 0}{{. \u0631", "\u0632 \u200E{0}\u200F. \u0631", TestName = "FixBrokenFormattingString_Works_14")] public void TryToFixBrokenSubstitutionMarkers(string badFormat, string goodFormat) { - var result = XLiffLocalizedStringCache.FixBrokenFormattingString(badFormat); + var result = XliffLocalizedStringCache.FixBrokenFormattingString(badFormat); Assert.That(result, Is.EqualTo(goodFormat)); // Check for the maximum number of possible substitution markers: unused arguments don't matter for validity. - Assert.That(XLiffLocalizedStringCache.CheckForValidSubstitutionMarkers(3, result, "a.b"), Is.EqualTo(true)); + Assert.That(XliffLocalizedStringCache.CheckForValidSubstitutionMarkers(3, result, "a.b"), Is.EqualTo(true)); } // This checks for a wider range of substitution marker numbers. @@ -76,7 +76,7 @@ public void TryToFixBrokenSubstitutionMarkers(string badFormat, string goodForma [TestCase("\u0632 21}{{. \u0631", "\u0632 \u200E{21}\u200F. \u0631", TestName = "FixBrokenSubstitution_Works_12")] public void FixBrokenSubstitutionMarkersOnly(string badFormat, string goodFormat) { - var result = XLiffLocalizedStringCache.FixBrokenFormattingString(badFormat); + var result = XliffLocalizedStringCache.FixBrokenFormattingString(badFormat); Assert.That(result, Is.EqualTo(goodFormat)); } } From c42b806e65fdb6f72e32065478de64f644dc4f58 Mon Sep 17 00:00:00 2001 From: Hasso Date: Tue, 7 Mar 2023 16:37:49 -0600 Subject: [PATCH 7/9] Accept autocorrections to the solution --- L10NSharp.sln | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/L10NSharp.sln b/L10NSharp.sln index c809df46..244b42c3 100644 --- a/L10NSharp.sln +++ b/L10NSharp.sln @@ -6,7 +6,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "L10NSharp", "src\L10NSharp\ EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "L10NSharpTests", "src\L10NSharpTests\L10NSharpTests.csproj", "{BCE5B569-057C-4D7E-832E-E44A2DA705AC}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SampleApp", "src\SampleApp\SampleApp.csproj", "{58923B30-FD84-4BCC-85E0-607DCA7E7C95}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SampleApp", "src\SampleApp\SampleApp.csproj", "{58923B30-FD84-4BCC-85E0-607DCA7E7C95}" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ExtractXliff", "src\ExtractXliff\ExtractXliff.csproj", "{E0666C78-B8DC-4232-952C-753940D54921}" EndProject @@ -18,10 +18,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "SolutionFolder", "SolutionF .gitattributes = .gitattributes .gitignore = .gitignore CHANGELOG.md = CHANGELOG.md - GitVersion.yml = GitVersion.yml - README.md = README.md Directory.Build.props = Directory.Build.props Directory.Build.targets = Directory.Build.targets + GitVersion.yml = GitVersion.yml + README.md = README.md EndProjectSection EndProject Global @@ -54,6 +54,17 @@ Global {BCE5B569-057C-4D7E-832E-E44A2DA705AC}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU {BCE5B569-057C-4D7E-832E-E44A2DA705AC}.Release|Mixed Platforms.Build.0 = Release|Any CPU {BCE5B569-057C-4D7E-832E-E44A2DA705AC}.Release|x86.ActiveCfg = Release|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Any CPU.Build.0 = Debug|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|x86.ActiveCfg = Debug|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|x86.Build.0 = Debug|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Any CPU.ActiveCfg = Release|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Any CPU.Build.0 = Release|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|x86.ActiveCfg = Release|Any CPU {E0666C78-B8DC-4232-952C-753940D54921}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {E0666C78-B8DC-4232-952C-753940D54921}.Debug|Any CPU.Build.0 = Debug|Any CPU {E0666C78-B8DC-4232-952C-753940D54921}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU @@ -78,15 +89,6 @@ Global {E4BB984D-DFB8-42EF-860D-5A038FF85B1C}.Release|Mixed Platforms.Build.0 = Release|Any CPU {E4BB984D-DFB8-42EF-860D-5A038FF85B1C}.Release|x86.ActiveCfg = Release|Any CPU {E4BB984D-DFB8-42EF-860D-5A038FF85B1C}.Release|x86.Build.0 = Release|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Any CPU.Build.0 = Debug|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Any CPU.ActiveCfg = Release|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Any CPU.Build.0 = Release|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|Mixed Platforms.Build.0 = Release|Any CPU - {58923B30-FD84-4BCC-85E0-607DCA7E7C95}.Release|x86.ActiveCfg = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE From fb8642f8b9413a83f11da3c22802c4183c3bcd1a Mon Sep 17 00:00:00 2001 From: Hasso Date: Tue, 7 Mar 2023 16:55:42 -0600 Subject: [PATCH 8/9] no, it's not. Should it be? What about zh-TW? --- src/L10NSharp/LocalizationManager.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/L10NSharp/LocalizationManager.cs b/src/L10NSharp/LocalizationManager.cs index 69dea087..141d5238 100644 --- a/src/L10NSharp/LocalizationManager.cs +++ b/src/L10NSharp/LocalizationManager.cs @@ -308,10 +308,7 @@ public static string UILanguageId // The current version of Mono does not define a CultureInfo for "zh", so // it tends to throw exceptions when we try to use just plain "zh". if (s_uiLangId == "zh-CN") - { - Debug.Fail("hooray; it's tested!"); return s_uiLangId; - } } // Otherwise, we want the culture.neutral version. int i = s_uiLangId.IndexOf('-'); From 297ffc9593553897be0ec21850eb4e5f67c785e7 Mon Sep 17 00:00:00 2001 From: Hasso Date: Wed, 8 Mar 2023 11:04:38 -0600 Subject: [PATCH 9/9] clarify changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efd1b684..5b39677e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed -- `LocalizationManager.Create("es"` loads `es-ES` if it is the best match +- `LocalizationManager.Create("es"` loads `es-ES` if it is the best match (previously, this resulted in a dialog making the user choose) ### Deprecated