Localization #96

moljac · 2020-08-25T10:32:08Z

No description provided.

brendanzagaeski

Thanks a bunch for setting this up! I'll leave a first few comments for this initial draft in case they'll help with the next steps for the PR.

At the moment, the Azure Pipelines build on Windows is failing with:

The nuget command failed with exit code(1) and error(NU1101: Unable to find package XliffTasks. No packages exist with this id in source(s): NuGetOrg
NU1101: Unable to find package XliffTasks. No packages exist with this id in source(s): NuGetOrg
Errors in D:\a\1\s\src\Xamarin.Android.Tools.AndroidSdk\Xamarin.Android.Tools.AndroidSdk.csproj
    NU1101: Unable to find package XliffTasks. No packages exist with this id in source(s): NuGetOrg
Errors in D:\a\1\s\tests\Xamarin.Android.Tools.AndroidSdk-Tests\Xamarin.Android.Tools.AndroidSdk-Tests.csproj
    NU1101: Unable to find package XliffTasks. No packages exist with this id in source(s): NuGetOrg)

One way to resolve this is to specify the feedsToUse and nugetConfigPath inputs for the NuGetCommand@2 Azure Pipelines task:

diff --git a/azure-pipelines.yaml b/azure-pipelines.yaml
index 6b2a147..7a55b27 100644
--- a/azure-pipelines.yaml
+++ b/azure-pipelines.yaml
@@ -31,6 +31,8 @@ jobs:
     displayName: 'NuGet Restore'
     inputs:
       restoreSolution: Xamarin.Android.Tools.sln
+      feedsToUse: config
+      nugetConfigPath: NuGet.config
   - task: MSBuild@1
     displayName: 'Build solution Xamarin.Android.Tools.sln'
     inputs:

This seems like a nice approach for this case.

Another option would be to specify the full NuGet command to run, like in the latest version of xamarin-android's setup-test-environment.yaml.

workflow/Localization.md

brendanzagaeski · 2020-08-28T18:26:50Z

src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj


+  <PropertyGroup>
+    <XlfLanguages>cs;de;es;fr;it;ja;ko;pl;pt-BR;ru;tr;zh-Hans;zh-Hant</XlfLanguages>
+    <UpdateXlfOnBuild Condition="'$(RunningOnCI)' != 'true'">true</UpdateXlfOnBuild>


The xamarin-android-tools-ci pipeline did not yet have the RunningOnCI variable defined under the Variables button, so I have just added it to align with the convention followed by the pipelines for the other xamarin-android repositories like xamarin-android itself, Java.Interop, and monodroid.

No changes are needed in the PR to account for this. I just wanted to mention it here for reference.

src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj

jonpryor · 2020-09-15T16:16:49Z

src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs

 				throw new ArgumentNullException (nameof (homePath));
 			if (!Directory.Exists (homePath))
-				throw new ArgumentException ("Not a directory", nameof (homePath));
+				throw new ArgumentException ("XamarinAndroidTools_XAT0008", nameof (homePath));


Shouldn't this use Resources.ResourceManager.GetString("XamarinAndroidTools_XAT0008")

Added one more which was missed.

moljac · 2020-09-20T07:51:44Z

/azp run

azure-pipelines · 2020-09-20T07:51:53Z

Azure Pipelines successfully started running 1 pipeline(s).

moljac · 2020-09-20T07:58:45Z

/azp run

azure-pipelines · 2020-09-20T07:58:54Z

Azure Pipelines successfully started running 1 pipeline(s).

jonpryor · 2020-09-23T16:29:10Z

@brendanzagaeski's comment raises a pre-existing-yet-unanswered question around error code management: How should they be managed?

The interaction between xamarin/Java.Interop & xamarin/xamarin-android has the "same" problem/scenario: Xamarin.Android.Build.Tasks.dll (xamarin-android) uses Xamarin.Android.Tools.Bytecode.dll (Java.Interop). If the latter throws an exception, how should we manage the error codes?

Similar-but-different, should the "leaf methods" know about error codes in the first place? Xamarin.Android.Tools.Bytecode.dll is used by both Xamarin.Android.Build.Tasks.dll and class-parse.exe. Should these different contexts share the same error "codes", wherein the code includes a "prefix" such as XA1234? Should class-parse emit a XA1234 error and not a CP1001 error?

With this "philosophical thinking" in place, I think the xamarin-android-tools repo shouldn't know, care, or otherwise worry about error codes. I think that should be handled by the callers, e.g. the <ResolveJdkJvmPath/> task.

However, this poses a different question: different errors should get different codes. How should this "difference" be communicated?

In the case of xamarin-android-tools, there is no difference; there is only one use of TraceLevel.Error:

% git grep TraceLevel.Error  
src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs:                 case TraceLevel.Error:
src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs:                                    logger (TraceLevel.Error, "Could not load monodroid configuration file");

That doesn't answer how things should work in e.g. Java.Interop, but for here-and-now, we don't need to worry about error codes at all.

jpobst · 2020-09-25T16:59:38Z

Note that currently this PR adds several NRT warnings to the build: https://dev.azure.com/xamarin/public/_build/results?buildId=27103&view=results.

jonpryor · 2020-09-25T23:19:56Z

Context: dotnet/android@0342fe5

…ed_localization This merge is largely to address some of the nullable reference type warnings that were being emitted.

Instead of `Resources.ResourceManager.GetString("NAME")`, use `Resources.NAME`. This promotes type safety, ensuring that we don't "typo" resource names, and use resources that (should) exist. Review and replace the `<comment/>` values within `Resourdces.resx`, so that they're (hopefully) useful values for the translators. Re-run `/t:UpdateXlf` so that the `Resources.*.xlf` files have all the strings from `Resources.resx`. Open Visual Studio for Mac and "update" `Resources.resx` to cause it to regenerate `Resource.Designer.cs`, as it was missing the `XamarinAndroidTools_XAT0017` resource. Remove `XamarinAndroidTools_XAT0012`, as it's not for localization: https://github.com/xamarin/xamarin-android-tools/pull/96/files#r493056022

Addresses: https://github.com/xamarin/xamarin-android-tools/pull/96/files#r493033574

jonpryor · 2020-09-26T00:12:36Z

@brendanzagaeski: Question: how do we make the generated Resource.Designer.cs emit an internal type? Xamarin.Android.Build.Tasks.dll has an internal type, but the one in this repo is public.

jonpryor · 2020-09-26T00:16:12Z

Commit message for squash-and-merge review:

Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1009374/
Context: https://github.com/xamarin/xamarin-android/commit/0342fe5698b86e21e36c924732ff135b9a87e4af

Localize error, warning, and exception messages produced by
`Xamarin.Android.Tools.AndroidSdk.dll`.

We will be following the [.NET Resource Localization pattern][0] and
generating satellite assemblies using [`.resx` files][1], in particular
`src/Xamarin.Android.Tools.AndroidSdk/Properties/Resources.resx`.

`Resources.resx` is an XML file, and will contain `/root/data`
elements in which `//data/@name` will start with the
xamarin-android-tools error or warning code, and `//data/value` will
be the error or warning message:

	<root>
	  <data name="XamarinAndroidTools_XAT0001" xml:space="preserve">
	    <value>App manifest does not have 'manifest' root element</value>
	    <comment>The following are literal names and should not be translated: manifest</comment>
	  </data>
	</root>

An optional `//data/comment` element may be provided to describe the
meaning within the `//data/value` element to translators.

When using Visual Studio or Visual Studio for Mac, changes to
`Resrouces.resx` will cause `Resources.Designer.cs` to be updated:

	namespace Xamarin.Android.Tools.AndroidSdk.Properties {
	  internal partial class Resources {
	    internal static string XamarinAndroidTools_XAT0001 {
	      get => ...
	    }
	  }
	}

The `Resources` members should be used to obtain all strings for use
in `logger()` and `throw new …()` calls:

	logger (TraceLevel.Warning, string.Format (Resources.XamarinAndroidTools_XAT0013, path, locator, e.Message));
	throw new ArgumentException (Resources.XamarinAndroidTools_XAT0001, nameof (doc));

When an MSBuild error or warning code is used with more than one
output string, then a semantically meaningful suffix should be used
to distinguish between the two.

Note that this infrastructure does not interoperate with C#6 string
interpolation.  Any error or warning messages currently using C#6
string interpolation will need to use .NET 1.0-style format strings.

Our translation team doesn't work directly with `.resx` files.
Instead, the translation team works with [XLIFF files][2].
`Resources.resx` is converted into a set of
`src/Xamarin.Android.Build.Tasks/Properties/xlf/Resources.*.xlf`
files via `XliffTasks.targets` from the [dotnet/xliff-tasks][3] repo.
The `Resources.*.xlf` files should be automatically updated whenever
`Resources.resx` is updated.


[0]: https://docs.microsoft.com/dotnet/framework/resources/index
[1]: https://docs.microsoft.com/dotnet/framework/resources/creating-resource-files-for-desktop-apps#resources-in-resx-files
[2]: http://docs.oasis-open.org/xliff/v1.2/os/xliff-core.html
[3]: https://github.com/dotnet/xliff-tasks

brendanzagaeski · 2020-09-26T00:19:23Z

To make the generated members internal, adjust src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj:

-<Generator>PublicResXFileCodeGenerator</Generator>
+<Generator>ResXFileCodeGenerator</Generator>

Remove no-longer used translation strings, as of f1719a3. Fix code formatting. Rename `XamarinAndroidTools_XAT0013` to `InvalidJdkDirectory`.

brendanzagaeski

The two other logger() calls mentioned previously are not yet localizable:

src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs:235:                                        logger (TraceLevel.Error, "Could not load monodroid configuration file");
src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkWindows.cs:117:                                     logger (TraceLevel.Warning, $"The directory `{path}`, via locator `{locator}`, is not a valid JDK directory: {e.Message}");

I believe it will make sense to include those in this PR since they are surfaced as MSBuild messages in the same way as the logger() call in JdkInfo. Thanks!

Wording brainstorming:

For:

src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs:235:                                        logger (TraceLevel.Error, "Could not load monodroid configuration file");

I'll suggest something like:

<data name="InvalidJdkPathConfigFile" xml:space="preserve">
  <value>An exception occurred while loading the Java SDK path configuration file '{0}'. Ensure that the file is a valid XML file. Exception: {1}</value>
  <comments>{0} - The path of the invalid file
{1} - The exception message of the associated exception</comments>
</data>

For:

src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkWindows.cs:117:                                     logger (TraceLevel.Warning, $"The directory `{path}`, via locator `{locator}`, is not a valid JDK directory: {e.Message}");

This one can use the same Resources.InvalidJdkDirectory resource as used in JdkInfo.

Thanks!

src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs

src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs

src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs

src/Xamarin.Android.Tools.AndroidSdk/Properties/Resources.resx

Context: #96 (comment)

Addresses: * #96 (comment) * #96 (comment) * #96 (comment)

Context: #96 (review) Add the following error messages: * `FailedToChangeFileSystemOwnership` * `InvalidMonodroidConfigFile_path_message` Additionally, rename `InvalidJdkDirectory` to `InvalidJdkDirectory_path_locator_message` as a "trial balloon": one of my "foibles" with localization is that the localizations frequently use string formats -- {0}, {1}, etc. -- but it's not obvious how many parameters a message takes unless the message is in front of you. Thus, an idea: what if we "Hungarian-notationed" things so that the number of parameters was encoded into the descriptor? This could be e.g. a `_3` suffix -- "takes 3 parameters" -- but that's ugly. We can build on this and instead encode *semantics* for what each parameter is. Thus, `InvalidJdkDirectory_path_locator_message`: it takes three parameters, a *path*, a *locator*, and a *message*: logger (TraceLevel.Warning, string.Format (Resources.InvalidJdkDirectory, path, locator, e.Message)); // vs. logger (TraceLevel.Warning, string.Format (Resources.InvalidJdkDirectory_path_locator_message, path, locator, e.Message)) I think this makes things a bit more obvious.

Add the automatic changes that Visual Studio makes when saving the resource files so there won't be any need to worry about them getting included unintentionally as part of an unrelated future change to the `.resx` file.

src/Xamarin.Android.Tools.AndroidSdk/Properties/Resources.resx

Context: #96 (comment) Context: #96 (comment)

brendanzagaeski · 2020-10-02T23:33:25Z

/azp run

azure-pipelines · 2020-10-02T23:33:32Z

No pipelines are associated with this pull request.

brendanzagaeski

Approved! I pushed one more small commit to update the generated files, so those should be all set now.

It'll probably be good to get the build pipeline to run once more on this PR to be careful before merging, but it looks good locally, so I think that'll build without problems if we can get it to trigger.

brendanzagaeski · 2020-10-03T00:11:33Z

I manually triggered the pipeline for this branch via the Azure Pipelines UI. That seemed to allow it to run correctly, including having $(RunningOnCi)=true so that the validity of the .xlf files was checked as expected. In short, I think this is all set to merge now.

Oops.

initial commit of files

f4ab3ad

moljac self-assigned this Aug 25, 2020

moljac added do-not-merge Do not merge this PR enhancement labels Aug 25, 2020

jonpryor requested a review from brendanzagaeski August 27, 2020 18:52

brendanzagaeski reviewed Aug 28, 2020

View reviewed changes

moljac added 13 commits August 31, 2020 12:44

settings for nuget restore on CI windows bots

7f9a629

removed doc files (workflow from xamarin-android)

21b70af

Resources.Designer.cs generated for Resources.resx localization

78c77ab

AndroidAppManifest localization

c23fb61

AndroidSdkInfo localization

62d001a

resources generated

2288326

PrivateAssets all for XliffTasks

4af5a5c

AndroidSdkInfo localization fixes

44948ae

AndroidVersion localization fixes

73b3bd7

JdkInfo localization fixes

d9d37bf

OS localizations

24cbd96

ProcessUtilities localization

a3f6870

Generated code for localization

339967e

moljac requested a review from brendanzagaeski September 11, 2020 14:39

moljac removed the do-not-merge Do not merge this PR label Sep 14, 2020

jonpryor reviewed Sep 15, 2020

View reviewed changes

moljac added 3 commits September 16, 2020 10:19

fix Resources.ResourceManager.GetString

970cc20

localized missed message (Could not determine Java version)

82bfc15

/t:UpdateXlf added

9878448

order of steps changed for Xlf update

5fc833c

jonpryor added 3 commits September 25, 2020 20:00

Merge remote-tracking branch 'upstream/master' into moljac_master_bas…

f5e0a2a

…ed_localization This merge is largely to address some of the nullable reference type warnings that were being emitted.

Remove msbuild /t:UpdateXlf invocations

7aa2c6f

Addresses: https://github.com/xamarin/xamarin-android-tools/pull/96/files#r493033574

jonpryor and others added 3 commits September 25, 2020 22:03

Make the Resources type internal, not public.

c1d07ee

Exceptions excluded from localization (for now)

f1719a3

Cleanup!

d1d69e0

Remove no-longer used translation strings, as of f1719a3. Fix code formatting. Rename `XamarinAndroidTools_XAT0013` to `InvalidJdkDirectory`.

brendanzagaeski suggested changes Sep 29, 2020

View reviewed changes

jonpryor added 2 commits October 1, 2020 18:45

Reword InvalidJdkDirectory error message

a3c0d81

Context: #96 (comment)

More cleanup

5578a9b

Addresses: * #96 (comment) * #96 (comment) * #96 (comment)

jonpryor requested a review from brendanzagaeski October 1, 2020 23:44

jonpryor force-pushed the moljac_master_based_localization branch from fd23187 to 0cb78c0 Compare October 1, 2020 23:47

Tidy generated resource files

cf8bf25

Add the automatic changes that Visual Studio makes when saving the resource files so there won't be any need to worry about them getting included unintentionally as part of an unrelated future change to the `.resx` file.

brendanzagaeski reviewed Oct 2, 2020

View reviewed changes

src/Xamarin.Android.Tools.AndroidSdk/Properties/Resources.resx Outdated Show resolved Hide resolved

src/Xamarin.Android.Tools.AndroidSdk/Properties/Resources.resx Outdated Show resolved Hide resolved

jonpryor and others added 2 commits October 2, 2020 15:29

Update to address comments:

4aa4f09

Context: #96 (comment) Context: #96 (comment)

Tidy generated resource files

2ecf9e6

brendanzagaeski approved these changes Oct 2, 2020

View reviewed changes

Use {1}, not {2}.

08c0381

Oops.

jonpryor merged commit 1878e43 into master Oct 3, 2020

pjcollins deleted the moljac_master_based_localization branch June 17, 2024 16:08

Localization #96

Localization #96

Uh oh!

Conversation

moljac commented Aug 25, 2020

Uh oh!

brendanzagaeski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brendanzagaeski Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonpryor Sep 15, 2020

Choose a reason for hiding this comment

Uh oh!

moljac Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

moljac Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

moljac Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

moljac commented Sep 20, 2020

Uh oh!

azure-pipelines bot commented Sep 20, 2020

Uh oh!

moljac commented Sep 20, 2020

Uh oh!

azure-pipelines bot commented Sep 20, 2020

Uh oh!

jonpryor commented Sep 23, 2020

Uh oh!

jpobst commented Sep 25, 2020

Uh oh!

jonpryor commented Sep 25, 2020

Uh oh!

jonpryor commented Sep 26, 2020

Uh oh!

jonpryor commented Sep 26, 2020

Uh oh!

brendanzagaeski commented Sep 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brendanzagaeski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brendanzagaeski commented Oct 2, 2020

Uh oh!

azure-pipelines bot commented Oct 2, 2020

Uh oh!

brendanzagaeski left a comment

Choose a reason for hiding this comment

Uh oh!

brendanzagaeski commented Oct 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

brendanzagaeski commented Sep 26, 2020 •

edited

Loading