search

sillsdev / l10nsharp

A .NET localization library for Windows Forms applications.
3 stars 10 forks source link

Create LocalizationManager using more specific locale if necessary #78

Closed tombogle closed 4 years ago

tombogle commented 4 years ago

For programs that are switching from TMX-based localization to XLIFF-based, languages like Spanish (es) and Portuguese (pt) will typically use a langiage code that includes a country specifier (es-ES, pt-PT). The LocalizationManager already knows about these mappings and handles them ocrrectly when getting strings, but not when checking to see if the requested UI language is available. If the caller requests a desired UI based on a two-letter language code that is not available, but there is a mapping to the corresponding (country-)specifiic locale, use that without displaying the "UI not available" dialog.

This change is Reviewable

tombogle commented 4 years ago

I initially made the tests generic, but the TMX version failed because TMX doesn't have these mappings. That might just be coincidence, based on the test data. I didn't try to track it down, because in practice, all I care about is getting the behavior when switching from TMX to XLIFF. I could add a comment to the test (and production code) to explain that. (Or I could try to figure out why it is that way, but I don't want to.)

tombogle commented 4 years ago

src/L10NSharpTests/LocalizationManagerXliffTests.cs, line 13 at r1 (raw file):

Previously, andrew-polk wrote…
> ``` > ensures that if the UI pops up, a developer will not have time to respond > ``` 3 seconds is a long time for a dev to respond (unless it is running on a TC agent, of course). I think I would tweak this comment.

It's 3 seconds total. I tested it. 3 seconds was defnitely not enough time to respond. I didn't want to make it too short, lest it fail spuriously on a bogged down machine.

tombogle commented 4 years ago

src/L10NSharpTests/LocalizationManagerXliffTests.cs, line 17 at r1 (raw file):

Previously, andrew-polk wrote…
> ``` > "es" > ``` Don't we want two test cases here? One for "es" and one for "es-ES"?

I don't think so. At least in practice, I don't think we'll have a case where it goes the other diirection. I guess I could write the test and see if it even works.

tombogle commented 4 years ago

src/L10NSharpTests/LocalizationManagerXliffTests.cs, line 13 at r1 (raw file):

Previously, andrew-polk wrote…
I think I would still tweak it. Maybe something like: If the test set up is incorrect, we can end up with a dialog popping up and asking the user which UI Language to use. In that case, we don't want the test to hang, but rather to fail. However, we also don't want the developer to interact with the dialog which could give a false success. So we set the time out to 3 seconds, which seems to be long enough to let the test run successfully but short enough to prevent a developer from interacting with it.

Done.

tombogle commented 4 years ago

src/L10NSharp/LocalizationManagerInternal.cs, line 85 at r2 (raw file):

Previously, ermshiperete (Eberhard Beilharz) wrote…
Hmm, makes me wonder: is the method name really long because the comment is really long, or is it the other way round? :wink:

Probably the two things are related. I've written worse. This test is just a bit funky, and I figured a long comment was needed to make it make sense. I couldn't think of any good way to shorten the name and still have it be fairly meaningful if someone ever sees it fail.