Avoid having Bing mis-translate native UI language name back as "English"

[tombogle]

Fix for #66

---

This change is 

[ermshiperete]

It's too bad that we don't have tests for that part, that makes it much harder to understand what's going on. But I finally got it 😄

Could you please add a line to CHANGELOG.md with a reference to the issue#? Otherwise

Reviewed 1 of 1 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @hatton)

[ermshiperete]

Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @hatton)

[ermshiperete]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @hatton and @tombogle)

---

CHANGELOG.md, line 19 at r2 (raw file):

## [Unreleased]

### Fixed

Actually, we already have a Fixed section just below, so the text should go there.

[tombogle]

I further enhanced the fix. I could wrap some of that logic in a testable method. Obviously, we wouldn't want the tests to actually call Bing, which is where the real issues could lie. But the tests could at least try to anticipate some of the garbage that Bing could possibly throw back at us. So that might be worthwhile.

[JohnThomson]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @hatton and @tombogle)

---

src/L10NSharp/UI/LanguageChoosingDialog.cs, line 31 at r1 (raw file):

			{
				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.

Based on the old and following code, it appears that we expect _originalMessageTemplate to contain something like {0} ({1}), where (in English) {0} is the English name of the language and {1} is the native name, like German (Deutsch). We want to give Bing German ({1}) and get back Deutsch ({1}) and THEN change {1} to "German".
I would expect this to fail because the string.Format() will choke on a string containing {1} when only given a single argument. You'd need to pass "{1}" as a second argument (or I think you could pass "{0}" as the second argument, then Bing will get German ({0}), hopefully return Deutsch ({0}, and you are all ready to insert the English).
Even if I'm wrong and string.Format gives back German ({1}) and Bing translates this to Deutsch ({1}), I think this needs more explanation. The trailing comment
on the line above, particularly, was just confusing...
I think it would be good to document what it's acceptable for _originalMessageTemplate to contain, what we want to pass to Bing for each such case, what we hope to get back, and then be clearer about happy and unhappy paths for what we do next and how we tell the difference.

[tombogle]

---

src/L10NSharp/UI/LanguageChoosingDialog.cs, line 31 at r1 (raw file):

Previously, JohnThomson (John Thomson) wrote…

Based on the old and following code, it appears that we expect _originalMessageTemplate to contain something like {0} ({1}), where (in English) {0} is the English name of the language and {1} is the native name, like German (Deutsch). We want to give Bing German ({1}) and get back Deutsch ({1}) and THEN change {1} to "German".
I would expect this to fail because the string.Format() will choke on a string containing {1} when only given a single argument. You'd need to pass "{1}" as a second argument (or I think you could pass "{0}" as the second argument, then Bing will get German ({0}), hopefully return Deutsch ({0}, and you are all ready to insert the English).
Even if I'm wrong and string.Format gives back German ({1}) and Bing translates this to Deutsch ({1}), I think this needs more explanation. The trailing comment
on the line above, particularly, was just confusing...
I think it would be good to document what it's acceptable for _originalMessageTemplate to contain, what we want to pass to Bing for each such case, what we hope to get back, and then be clearer about happy and unhappy paths for what we do next and how we tell the difference.

I did consider doing the replacement as part of the initial call to Format. (I've done that before in other apps, but it kind of feels confusing either way.) I'm doing some refactoring to bring this code under unit tests. I'll think it through and try to make it intelligible.

[tombogle]

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @ermshiperete, @hatton, and @JohnThomson)

---

CHANGELOG.md, line 19 at r2 (raw file):

Previously, ermshiperete (Eberhard Beilharz) wrote…

Actually, we already have a Fixed section just below, so the text should go there.

Done.

[tombogle]

---

src/L10NSharp/UI/LanguageChoosingDialog.cs, line 31 at r1 (raw file):

Previously, tombogle (Tom Bogle) wrote…

I did consider doing the replacement as part of the initial call to Format. (I've done that before in other apps, but it kind of feels confusing either way.) I'm doing some refactoring to bring this code under unit tests. I'll think it through and try to make it intelligible.

And, yes, you were right.

[JohnThomson]

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @ermshiperete, @hatton, and @tombogle)

---

src/L10NSharp/UI/LanguageChoosingDialog.cs, line 31 at r1 (raw file):

Previously, tombogle (Tom Bogle) wrote…

And, yes, you were right.

Catch (Exception) is dangerous!

[tombogle]

---

src/L10NSharp/UI/LanguageChoosingDialog.cs, line 31 at r1 (raw file):

Previously, JohnThomson (John Thomson) wrote…

Catch (Exception) is dangerous!

Maybe so. That is existing code (in a new file). It's probably overkill, but since the only purpose of this code is to attempt to replace the English version of the message with a language-specific one, any unhandled error is only going to make things worse. Since it's calling out to a third-party web-based delegate to get the translation it might be difficult to anticipate everything that could possibly go wrong. I had actually considered whether it might be better to display the translated message in addition to the original English one, just in case we get something back that is garbage (and also because if someone needs help, it could be hard for a support person to make sense of the problem. Searching for the translated string in our code and TMX files won't result in a hit.) In the end, though, I figured that could be left for a rainier day.

[ermshiperete]

---

src/L10NSharp/UI/LanguageChoosingDialogViewModel.cs, line 18 at r3 (raw file):

			_messageLabelFormat = messageLabelFormat;
			AcceptButtonText = _acceptButtonText = acceptButtonText;
			WindowTitle = _windowTitle = windowTitle;

What's the purpose of having an auto property WindowTitle (and AcceptButtonText) and a separate field _windowTitle? Wouldn't the auto property be sufficient?