From 71d474905d0ec8bfd2eb2c50558b17a4fd802a03 Mon Sep 17 00:00:00 2001 From: "tom_bogle@sil.org" Date: Fri, 5 Jul 2019 10:04:23 -0400 Subject: [PATCH 1/5] Fix for https://github.com/sillsdev/l10nsharp/issues/66 --- src/L10NSharp/UI/LanguageChoosingDialog.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/L10NSharp/UI/LanguageChoosingDialog.cs b/src/L10NSharp/UI/LanguageChoosingDialog.cs index f07d594..86760a0 100644 --- a/src/L10NSharp/UI/LanguageChoosingDialog.cs +++ b/src/L10NSharp/UI/LanguageChoosingDialog.cs @@ -27,7 +27,16 @@ void Application_Idle(object sender, EventArgs e) var translator = new BingTranslator("en", _requestedCulture.TwoLetterISOLanguageName); try { - var s = translator.TranslateText(string.Format(_originalMessageTemplate, _requestedCulture.EnglishName, _requestedCulture.NativeName)); + var s = translator.TranslateText(string.Format(_originalMessageTemplate, _requestedCulture.EnglishName)); + if (s.Contains("{1}") && s.Length > 5) // If we just get back "{1} or "({1})", we won't consider that useful. + { + // Bing will presumably have translated the English string into the native language, so now we want + // to display the English name in parentheses. (As a sanity check, we could look to see whether the + // native name is in the string, but there could be situations where it may not be an exact match.) + s = string.Format(s.Replace("{1}", "{0}"), _requestedCulture.EnglishName); + } + else + s = translator.TranslateText(string.Format(_originalMessageTemplate, _requestedCulture.EnglishName, _requestedCulture.NativeName)); if (!string.IsNullOrEmpty(s)) { _messageLabel.Text = s; From be015676b230d35112bbb05b95ee59a32a39940e Mon Sep 17 00:00:00 2001 From: "tom_bogle@sil.org" Date: Fri, 5 Jul 2019 11:40:43 -0400 Subject: [PATCH 2/5] Additional improvement to prevent displaying the identical language name twice. --- src/L10NSharp/UI/LanguageChoosingDialog.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/L10NSharp/UI/LanguageChoosingDialog.cs b/src/L10NSharp/UI/LanguageChoosingDialog.cs index 86760a0..a2d53b0 100644 --- a/src/L10NSharp/UI/LanguageChoosingDialog.cs +++ b/src/L10NSharp/UI/LanguageChoosingDialog.cs @@ -9,7 +9,7 @@ namespace L10NSharp.UI public partial class LanguageChoosingDialog : Form { private readonly L10NCultureInfo _requestedCulture; - private string _originalMessageTemplate; + private readonly string _originalMessageTemplate; public LanguageChoosingDialog(L10NCultureInfo requestedCulture, Icon icon) { @@ -17,13 +17,19 @@ public LanguageChoosingDialog(L10NCultureInfo requestedCulture, Icon icon) InitializeComponent(); this.Icon = icon; _originalMessageTemplate = _messageLabel.Text; + if (requestedCulture.EnglishName == requestedCulture.NativeName) + { + // It looks weird and stupid to display "English (English)" or any other such pair where the two strings are the same. + _originalMessageTemplate = _originalMessageTemplate.Replace(" ({1})", ""); + } _messageLabel.Text = string.Format(_originalMessageTemplate, requestedCulture.EnglishName, requestedCulture.NativeName); - Application.Idle += new EventHandler(Application_Idle); + if (requestedCulture.TwoLetterISOLanguageName != "en") + Application.Idle += Application_Idle; } void Application_Idle(object sender, EventArgs e) { - Application.Idle -= new EventHandler(Application_Idle); + Application.Idle -= Application_Idle; var translator = new BingTranslator("en", _requestedCulture.TwoLetterISOLanguageName); try { @@ -35,7 +41,7 @@ void Application_Idle(object sender, EventArgs e) // native name is in the string, but there could be situations where it may not be an exact match.) s = string.Format(s.Replace("{1}", "{0}"), _requestedCulture.EnglishName); } - else + else if (_originalMessageTemplate.Contains("{1}")) // If the language names are the same, we already weeded out the extra param. s = translator.TranslateText(string.Format(_originalMessageTemplate, _requestedCulture.EnglishName, _requestedCulture.NativeName)); if (!string.IsNullOrEmpty(s)) { From 28236825abe1a45a2ba80944c867e39eccb9795e Mon Sep 17 00:00:00 2001 From: "tom_bogle@sil.org" Date: Fri, 5 Jul 2019 11:44:40 -0400 Subject: [PATCH 3/5] Updated CHANGELOG with info about fix. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f82f60..c6508f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed +- Don't ask Bing translator to translate language names: https://github.com/sillsdev/l10nsharp/issues/66. + Also don't display the name a second time in parentheses if English and native name are identical. + ### Added - create symbol nuget package From a2402832eedf3752876053956edf83f3dcaa537e Mon Sep 17 00:00:00 2001 From: "tom_bogle@sil.org" Date: Fri, 5 Jul 2019 13:31:49 -0400 Subject: [PATCH 4/5] Moved new Fixed comments into existing Fixed section of CHANGELOG --- CHANGELOG.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6508f9..544dc71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] -### Fixed -- Don't ask Bing translator to translate language names: https://github.com/sillsdev/l10nsharp/issues/66. - Also don't display the name a second time in parentheses if English and native name are identical. - ### Added - create symbol nuget package @@ -28,6 +24,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Find TMX files in `Generated` and `User Modified` directories +- Don't ask Bing translator to translate language names: https://github.com/sillsdev/l10nsharp/issues/66. + Also don't display the name a second time in parentheses if English and native name are identical. + ## [4.0.0] - 2019-05-16 ### Changed From 5f1146bec1b2c2a487adc3d3f217cf9771d2dd3d Mon Sep 17 00:00:00 2001 From: "tom_bogle@sil.org" Date: Fri, 5 Jul 2019 15:26:01 -0400 Subject: [PATCH 5/5] Moved LanguageChoosingDialog logic into LanguageChoosingDialogViewModel and added unit tests. (And found and fixed a problem or two.) --- src/L10NSharp/L10NSharp-Designer.csproj | 1 + src/L10NSharp/UI/LanguageChoosingDialog.cs | 47 +--- .../UI/LanguageChoosingDialogViewModel.cs | 70 ++++++ .../LanguageChoosingDialogViewModelTests.cs | 213 ++++++++++++++++++ 4 files changed, 291 insertions(+), 40 deletions(-) create mode 100644 src/L10NSharp/UI/LanguageChoosingDialogViewModel.cs create mode 100644 src/L10NSharpTests/LanguageChoosingDialogViewModelTests.cs diff --git a/src/L10NSharp/L10NSharp-Designer.csproj b/src/L10NSharp/L10NSharp-Designer.csproj index a084855..6c18722 100644 --- a/src/L10NSharp/L10NSharp-Designer.csproj +++ b/src/L10NSharp/L10NSharp-Designer.csproj @@ -123,6 +123,7 @@ LanguageChoosingDialog.cs + Form diff --git a/src/L10NSharp/UI/LanguageChoosingDialog.cs b/src/L10NSharp/UI/LanguageChoosingDialog.cs index a2d53b0..0e06ee1 100644 --- a/src/L10NSharp/UI/LanguageChoosingDialog.cs +++ b/src/L10NSharp/UI/LanguageChoosingDialog.cs @@ -1,6 +1,5 @@ using System; using System.Drawing; -using System.Globalization; using System.Windows.Forms; using L10NSharp.Translators; @@ -8,55 +7,23 @@ namespace L10NSharp.UI { public partial class LanguageChoosingDialog : Form { - private readonly L10NCultureInfo _requestedCulture; - private readonly string _originalMessageTemplate; + private readonly LanguageChoosingDialogViewModel _model; public LanguageChoosingDialog(L10NCultureInfo requestedCulture, Icon icon) { - _requestedCulture = requestedCulture; InitializeComponent(); this.Icon = icon; - _originalMessageTemplate = _messageLabel.Text; - if (requestedCulture.EnglishName == requestedCulture.NativeName) - { - // It looks weird and stupid to display "English (English)" or any other such pair where the two strings are the same. - _originalMessageTemplate = _originalMessageTemplate.Replace(" ({1})", ""); - } - _messageLabel.Text = string.Format(_originalMessageTemplate, requestedCulture.EnglishName, requestedCulture.NativeName); - if (requestedCulture.TwoLetterISOLanguageName != "en") - Application.Idle += Application_Idle; + _model = new LanguageChoosingDialogViewModel(_messageLabel.Text, _OKButton.Text, Text, requestedCulture, () => { Application.Idle += Application_Idle; } ); + _messageLabel.Text = _model.Message; } void Application_Idle(object sender, EventArgs e) { Application.Idle -= Application_Idle; - var translator = new BingTranslator("en", _requestedCulture.TwoLetterISOLanguageName); - try - { - var s = translator.TranslateText(string.Format(_originalMessageTemplate, _requestedCulture.EnglishName)); - if (s.Contains("{1}") && s.Length > 5) // If we just get back "{1} or "({1})", we won't consider that useful. - { - // Bing will presumably have translated the English string into the native language, so now we want - // to display the English name in parentheses. (As a sanity check, we could look to see whether the - // native name is in the string, but there could be situations where it may not be an exact match.) - s = string.Format(s.Replace("{1}", "{0}"), _requestedCulture.EnglishName); - } - else if (_originalMessageTemplate.Contains("{1}")) // If the language names are the same, we already weeded out the extra param. - s = translator.TranslateText(string.Format(_originalMessageTemplate, _requestedCulture.EnglishName, _requestedCulture.NativeName)); - if (!string.IsNullOrEmpty(s)) - { - _messageLabel.Text = s; - // In general, we will be able to translate OK and the title bar text iff we were able to translate - // the message. This assumption saves a few processor cycles and prevents disappearing text when - // a language has not been localized (as is likely the case when we display this dialog). - _OKButton.Text = translator.TranslateText("OK"); - Text = translator.TranslateText(Text); - } - } - catch (Exception) - { - //swallow - } + _model.SetTranslator(new BingTranslator("en", _model.RequestedCultureTwoLetterISOLanguageName)); + _messageLabel.Text = _model.Message; + _OKButton.Text = _model.AcceptButtonText; + Text = _model.WindowTitle; } public string SelectedLanguage; diff --git a/src/L10NSharp/UI/LanguageChoosingDialogViewModel.cs b/src/L10NSharp/UI/LanguageChoosingDialogViewModel.cs new file mode 100644 index 0000000..1944845 --- /dev/null +++ b/src/L10NSharp/UI/LanguageChoosingDialogViewModel.cs @@ -0,0 +1,70 @@ +using System; +using L10NSharp.Translators; + +namespace L10NSharp.UI +{ + public class LanguageChoosingDialogViewModel + { + private readonly L10NCultureInfo _requestedCulture; + private readonly string _messageLabelFormat; + private readonly string _acceptButtonText; + private readonly string _windowTitle; + + public LanguageChoosingDialogViewModel(string messageLabelFormat, string acceptButtonText, string windowTitle, + L10NCultureInfo requestedCulture, Action nonEnglishUiAction) + { + _messageLabelFormat = messageLabelFormat; + AcceptButtonText = _acceptButtonText = acceptButtonText; + WindowTitle = _windowTitle = windowTitle; + _requestedCulture = requestedCulture; + if (requestedCulture.EnglishName == requestedCulture.NativeName) + { + // It looks weird and stupid to display "English (English)" or any other such pair where the two strings are the same. + _messageLabelFormat = _messageLabelFormat.Replace(" ({1})", ""); + } + Message = string.Format(_messageLabelFormat, requestedCulture.EnglishName, requestedCulture.NativeName); + if (requestedCulture.TwoLetterISOLanguageName != "en") + nonEnglishUiAction?.Invoke(); + } + + public void SetTranslator(TranslatorBase translator) + { + try + { + var s = translator.TranslateText(string.Format(_messageLabelFormat, _requestedCulture.EnglishName, "{0}")); + if (s.Contains("{0}") && s.Length > 5) // If we just get back "{0} or "({0})", we won't consider that useful. + { + // Bing will presumably have translated the English string into the native language, so now we want + // to display the English name in parentheses. (As a sanity check, we could look to see whether the + // native name is in the string, but there could be situations where it may not be an exact match.) + s = string.Format(s, _requestedCulture.EnglishName); + } + else if (_messageLabelFormat.Contains("{1}")) + { + // If we already weeded out the param (because the language names are the same), there's no need to re-try (in case it's slow). + // This is just a fall-back in case there is some rare situation where the translator chokes on the presence of a formatting param in the string. + s = translator.TranslateText(string.Format(_messageLabelFormat, _requestedCulture.EnglishName, _requestedCulture.NativeName)); + } + + if (!string.IsNullOrEmpty(s)) + { + Message = s; + // In general, we will be able to translate OK and the title bar text iff we were able to translate + // the message. This assumption saves a few processor cycles and prevents disappearing text when + // a language has not been localized (as is likely the case when we display this dialog). + AcceptButtonText = translator.TranslateText(_acceptButtonText); + WindowTitle = translator.TranslateText(_windowTitle); + } + } + catch (Exception) + { + //swallow + } + } + + public string RequestedCultureTwoLetterISOLanguageName => _requestedCulture.TwoLetterISOLanguageName; + public string Message { get; private set; } + public string AcceptButtonText { get; private set; } + public string WindowTitle { get; private set; } + } +} diff --git a/src/L10NSharpTests/LanguageChoosingDialogViewModelTests.cs b/src/L10NSharpTests/LanguageChoosingDialogViewModelTests.cs new file mode 100644 index 0000000..fc8f29b --- /dev/null +++ b/src/L10NSharpTests/LanguageChoosingDialogViewModelTests.cs @@ -0,0 +1,213 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using L10NSharp.Translators; +using L10NSharp.UI; +using NUnit.Framework; + +namespace L10NSharp.Tests +{ + [TestFixture] + class LanguageChoosingDialogViewModelTests + { + [Test] + public void Constructor_RequestedCultureEnglish_MessageOnlyHasEnglishOnceAndNonEnglishActionIsNotRun() + { + var model = new LanguageChoosingDialogViewModel("Blah {0} ({1})", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "en"), + () => throw new Exception("Non-english UI Action should not have been called")); + Assert.AreEqual("Blah English", model.Message); + // We'll go ahead and confirm that the other properties come through as expected also. + Assert.AreEqual("en", model.RequestedCultureTwoLetterISOLanguageName); + Assert.AreEqual("OK", model.AcceptButtonText); + Assert.AreEqual("Choose a Language", model.WindowTitle); + } + + [Test] + public void Constructor_RequestedCultureTypicalNonEnglish_MessageHasNativeNameAndEnglishNameAndNonEnglishActionIsRun() + { + var nonEnglishActionGotCalled = false; + var model = new LanguageChoosingDialogViewModel("Blah {0} ({1})", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "de"), + () => nonEnglishActionGotCalled = true); + Assert.AreEqual("Blah German (Deutsch)", model.Message); + Assert.IsTrue(nonEnglishActionGotCalled); + // We'll go ahead and confirm that the other properties come through as expected also. + Assert.AreEqual("de", model.RequestedCultureTwoLetterISOLanguageName); + Assert.AreEqual("OK", model.AcceptButtonText); + Assert.AreEqual("Choose a Language", model.WindowTitle); + } + + [Test] + public void Constructor_RequestedCultureWithNativeNameEqualToEnglishName_MessageOnlyLanguageNameOnceAndNonEnglishActionIsRun() + { + var nonEnglishActionGotCalled = false; + var culture = L10NCultureInfo.GetCultures(CultureTypes.AllCultures).FirstOrDefault(c => c.EnglishName == c.NativeName); + Assume.That(culture != null); + var model = new LanguageChoosingDialogViewModel("Blah {0} ({1})", "OK", "Choose a Language", + culture, + () => nonEnglishActionGotCalled = true); + Assert.AreEqual($"Blah {culture.EnglishName}", model.Message); + Assert.IsTrue(nonEnglishActionGotCalled); + // We'll go ahead and confirm that the other properties come through as expected also. + Assert.AreEqual(culture.TwoLetterISOLanguageName, model.RequestedCultureTwoLetterISOLanguageName); + Assert.AreEqual("OK", model.AcceptButtonText); + Assert.AreEqual("Choose a Language", model.WindowTitle); + } + + [Test] + public void Constructor_RequestedCultureEnglishFormatStringHasNoParam1_MessageHasEnglishSubstitutedAsAparam0() + { + var model = new LanguageChoosingDialogViewModel("Blah {0} yup", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "en"), + null); + Assert.AreEqual("Blah English yup", model.Message); + } + + [TestCase("en")] + [TestCase("fr")] + public void Constructor_FormatStringHasNoParams_MessageEqualsFormatString(string languageCode) + { + var model = new LanguageChoosingDialogViewModel("No format parameters", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == languageCode), + null); + Assert.AreEqual("No format parameters", model.Message); + } + + [TestCase("Blah {2}")] + [TestCase("Blah {-1}")] + [TestCase("Blah {a}")] + public void Constructor_BogusFormatString_Throws(string format) + { + Assert.Throws(() => new LanguageChoosingDialogViewModel(format, "OK", "Choose a Language", + L10NCultureInfo.CurrentCulture, null)); + } + + [Test] + public void SetTranslator_RequestedCultureEnglish_TranslationAppliedOnceToEachString() + { + var model = new LanguageChoosingDialogViewModel("Blah {0} ({1}) yup!", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "en"), + null); + Assert.AreEqual("Blah English yup!", model.Message); + var translator = new TestTranslatorBumpyFrog(); + model.SetTranslator(translator); + Assert.AreEqual("Bumpy frog Blah English yup!", model.Message); + Assert.AreEqual("Bumpy frog OK", model.AcceptButtonText); + Assert.AreEqual("Bumpy frog Choose a Language", model.WindowTitle); + Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "Blah English yup!", "OK", "Choose a Language" })); + } + + [Test] + public void SetTranslator_RequestedCultureGermanNormalTranslation_TranslationAppliedOnceToEachString() + { + var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "de"), + null); + Assert.AreEqual("No localization for German (Deutsch)", model.Message); + var translator = new TestTranslatorGerman(); + model.SetTranslator(translator); + Assert.AreEqual("Ich spreche No localization for Deutsch (German)", model.Message); + Assert.AreEqual("Ich spreche OK", model.AcceptButtonText); + Assert.AreEqual("Ich spreche Choose a Language", model.WindowTitle); + Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "No localization for German ({0})", "OK", "Choose a Language" })); + } + + [Test] + public void SetTranslator_ChangeTranslator_TranslationAppliedToOriginalString() + { + var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "de"), + null); + Assert.AreEqual("No localization for German (Deutsch)", model.Message); + model.SetTranslator(new TestTranslatorBumpyFrog()); + model.SetTranslator(new TestTranslatorGerman()); + Assert.AreEqual("Ich spreche No localization for Deutsch (German)", model.Message); + Assert.AreEqual("Ich spreche OK", model.AcceptButtonText); + Assert.AreEqual("Ich spreche Choose a Language", model.WindowTitle); + } + + [Test] + public void SetTranslator_RequestedCultureSpanishChokesOnFormatParam_TranslationReappliedToStringWithoutParam() + { + var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "OK", "Choose a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "es"), + null); + Assert.AreEqual("No localization for Spanish (español)", model.Message); + var translator = new TestTranslatorSpanishChokesOnFormatParam(); + model.SetTranslator(translator); + // Note: the test translator mimics Bing's behavior of replacing the English name of the requested language with the word "English" in the translation. + Assert.AreEqual("No choke No localization for English (español)", model.Message); + Assert.AreEqual("No choke OK", model.AcceptButtonText); + Assert.AreEqual("No choke Choose a Language", model.WindowTitle); + Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "No localization for Spanish ({0})", "No localization for Spanish (español)", "OK", "Choose a Language" })); + } + + [Test] + public void SetTranslator_TranslatorThrowsException_ExceptionSwallowedAndNoAttemptToTranslateOtherStrings() + { + var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "Okey-dokey", "Select a Language", + L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "es"), + null); + var translator = new TestTranslatorThrowsException(); + model.SetTranslator(translator); + Assert.AreEqual("No localization for Spanish (español)", model.Message); + Assert.AreEqual("Okey-dokey", model.AcceptButtonText); + Assert.AreEqual("Select a Language", model.WindowTitle); + Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "No localization for Spanish ({0})" })); + } + + private class TestTranslatorBase : TranslatorBase + { + public List SourceStrings { get; } + + public TestTranslatorBase() + { + SourceStrings = new List(); + } + + protected override string InternalTranslate(string srcText) + { + SourceStrings.Add(srcText); + return srcText; + } + } + + private class TestTranslatorBumpyFrog : TestTranslatorBase + { + protected override string InternalTranslate(string srcText) + { + return "Bumpy frog " + base.InternalTranslate(srcText); + } + } + + private class TestTranslatorGerman : TestTranslatorBase + { + protected override string InternalTranslate(string srcText) + { + return "Ich spreche " + base.InternalTranslate(srcText).Replace("German", "Deutsch"); + } + } + + private class TestTranslatorSpanishChokesOnFormatParam : TestTranslatorBase + { + protected override string InternalTranslate(string srcText) + { + var s = base.InternalTranslate(srcText); + if (s.Contains("{0}")) + return ""; + return "No choke " + s.Replace("Spanish", "English"); // Looks weird, but Bing actually does this! + } + } + + private class TestTranslatorThrowsException : TestTranslatorBase + { + protected override string InternalTranslate(string srcText) + { + base.InternalTranslate(srcText); + throw new Exception("This should get swallowed"); + } + } + } +}