[Localization] Add generator error messages #7531

ghost · 2019-12-02T21:10:45Z

I will be making multiple PRs for localization changes, hopefully one per tool. This PR is copying what android does for localization for more context see: dotnet/android#3900 and https://github.com/xamarin/monodroid/wiki/Localization

spouliot

Not sure how the whole "fix a typo" process would work ?
I suspect (hope) there's not need to update all .xlf files but, beside that, I would not know how to proceed.

src/Resources.Designer.cs

src/Resources.resx

src/generator.cs

ghost · 2019-12-02T21:35:28Z

Not sure how the whole "fix a typo" process would work ?
I suspect (hope) there's not need to update all .xlf files but, beside that, I would not know how to proceed.

It works by editing Resources.resx in visual studio. The xlf and and designeer.cs files will be updated/generated automatically. There is documentation in the android pr ( dotnet/android#3900 ) that does a better job of explaining the process . I will port the documentation eventually.

monojenkins · 2019-12-03T01:04:26Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 86 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
Generator tests/NUnit: BuildFailure

VincentDondain

Looking good 👍

Couple items we could discuss as a team:

Should we take that change to improve the error messages?
- That is make sure all sentences start with a capital letter and end with a period.
- Remove error codes from the error message (redundant)
- Always put methods and properties between ' ' (apostrophes)
This is gonna be tedious but I suggest we consider adding <comment> for most errors to tell translators what to avoid translating (e.g: "runtime", "BaseType", "binding", etc.
- Background, I dogfooded the French translations of VSMac and a lot of things were translated when they didn't need to be.
- We need guidance on that, the Android team might be facing the same questions and the IDE folks might be able to help us too.

monojenkins · 2019-12-04T00:57:56Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 86 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
Generator tests/NUnit: BuildFailure

src/Resources.Designer.cs

src/Resources.resx

src/btouch.cs

src/error.cs

monojenkins · 2019-12-04T21:47:06Z

Build failure
❌ Build failed or was aborted

🔥 Build failed 🔥

monojenkins · 2019-12-04T22:21:24Z

Build failure
❌ Build failed or was aborted

✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)

monojenkins · 2019-12-05T01:46:29Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 86 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
Generator tests/NUnit: BuildFailure

monojenkins · 2019-12-05T22:20:32Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

monojenkins · 2019-12-05T22:25:14Z

Build failure
❌ Build failed or was aborted

monojenkins · 2019-12-06T01:48:18Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 86 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
Generator tests/NUnit: BuildFailure

monojenkins · 2019-12-09T17:55:31Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 86 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
Generator tests/NUnit: BuildFailure

monojenkins · 2019-12-09T22:56:03Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

Test results

1 tests failed, 87 tests passed.

Failed tests

Generator tests/NUnit: Failed (Execution failed with exit code 4)

monojenkins · 2019-12-10T20:20:39Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

monojenkins · 2019-12-10T20:25:15Z

Build failure
❌ Build failed or was aborted

🔥 Build failed 🔥

ghost · 2019-12-10T20:28:20Z

build

rolfbjarne

Question: should we create src/bgen subdirectory and put every new file there? The src/ directory is getting crowded... this would only be for new files in this PR, but eventually all the bgen source files would go there.

src/error.cs

src/generator.cs

Co-Authored-By: Rolf Bjarne Kvinge <[email protected]>

monojenkins · 2019-12-10T21:05:33Z

Build failure
❌ Build failed or was aborted

✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

monojenkins · 2019-12-10T21:05:44Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

…alization-generator

src/generator-attribute-manager.cs

monojenkins · 2019-12-10T21:15:27Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

monojenkins · 2019-12-10T23:19:25Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
🔥 Test run failed 🔥

Test results

1 tests failed, 87 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed

src/generator-filters.cs

src/generator.cs

mandel-macaque · 2019-12-13T19:06:42Z

src/generator.cs

 			sb.Append (ParameterGetMarshalType (new MarshalInfo (this, mi) { IsAligned = aligned, EnumMode = enum_mode } ));
 		} catch (BindingException ex) {
-			throw new BindingException (ex.Code, ex.Error, ex,  "{0} in method `{1}'", ex.Message, mi.Name);
+			throw new BindingException (1078, ex.Error, ex, ex.Message, mi.Name);


Why do we go from ex.Code to 1078? Is ex.Code always the same, if it is not, we are going to get the wrong message.

src/generator.cs

tests/generator/ErrorTests.cs

src/generator.cs

prmallic · 2020-01-08T16:06:15Z

src/error.cs

+				BindingFlags.GetProperty);
+
+		var errorMessage = prop == null ? String.Format (bgen.Resources._default, errorCode) :
+					(String) prop.GetValue (null);


Suggested change

(String) prop.GetValue (null);

prop.GetValue (null) as string;

To avoid cast exceptions?

Cast exceptions are usually easier to diagnose than null reference exceptions, so I think the current code is fine.

prmallic · 2020-01-08T16:09:48Z

src/Resources.Designer.cs

+        internal Resources() {
+        }
+
+        [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)]


I think this file isn't code-formatted to our guidelines. Edit->Format->Format Document on Visual Studio does the trick

This file is auto-generated.

rolfbjarne · 2020-01-08T17:00:52Z

src/error.cs

+				BindingFlags.GetProperty);
+
+		var errorMessage = prop == null ? String.Format (bgen.Resources._default, errorCode) :
+					(String) prop.GetValue (null);


Cast exceptions are usually easier to diagnose than null reference exceptions, so I think the current code is fine.

…rator

monojenkins · 2020-01-08T21:15:16Z

Build failure
❌ Build failed or was aborted

[Localization] Add generator error messages

30f826d

ghost requested review from VincentDondain, dalexsoto and spouliot as code owners December 2, 2019 21:10

undo changes caused by copy/pasting

2058f96

spouliot reviewed Dec 2, 2019

View reviewed changes

src/Resources.Designer.cs Show resolved Hide resolved

src/Resources.resx Outdated Show resolved Hide resolved

src/generator.cs Outdated Show resolved Hide resolved

VincentDondain reviewed Dec 3, 2019

View reviewed changes

Add reflection and other updates

f439112

stephen-hawley reviewed Dec 4, 2019

View reviewed changes

src/Resources.Designer.cs Show resolved Hide resolved

spouliot suggested changes Dec 4, 2019

View reviewed changes

src/Resources.resx Outdated Show resolved Hide resolved

src/btouch.cs Show resolved Hide resolved

src/btouch.cs Show resolved Hide resolved

src/error.cs Outdated Show resolved Hide resolved

src/error.cs Show resolved Hide resolved

Waleed Chaudhry added 2 commits December 4, 2019 16:43

Update and fix remaining errors

3f4d577

Undo visibility mistake

4117cbd

Update more errors

a26a681

Waleed Chaudhry added 3 commits December 5, 2019 16:27

Fix unit tests and add nuget source

638a420

And null check for reflection

113c4d0

Remove unused Globalization library

e543307

Update XliffTasks to version 1.0.0-beta.19252.1

f7b82f7

Temporarily 'UpdateXlfOnBuild=true' to see if it fixes jenkins

fa8acae

Fix broken tests

2731e99

Update error messages

c55d966

Fix style

d29f8fe

rolfbjarne requested changes Dec 10, 2019

View reviewed changes

src/error.cs Outdated Show resolved Hide resolved

src/generator.cs Outdated Show resolved Hide resolved

Fix style

8f41e04

Co-Authored-By: Rolf Bjarne Kvinge <[email protected]>

Waleed Chaudhry added 2 commits December 10, 2019 16:12

Undo attribute check using loop

78426c3

Merge remote-tracking branch 'origin/localization-generator' into loc…

65b3153

…alization-generator

whitneyschmidt approved these changes Dec 10, 2019

View reviewed changes

src/generator-attribute-manager.cs Show resolved Hide resolved

mandel-macaque requested changes Dec 13, 2019

View reviewed changes

spouliot approved these changes Dec 19, 2019

View reviewed changes

src/generator.cs Outdated Show resolved Hide resolved

src/generator.cs Outdated Show resolved Hide resolved

dalexsoto approved these changes Jan 8, 2020

View reviewed changes

prmallic approved these changes Jan 8, 2020

View reviewed changes

rolfbjarne approved these changes Jan 8, 2020

View reviewed changes

mandel-macaque approved these changes Jan 8, 2020

View reviewed changes

Waleed Chaudhry added 2 commits January 8, 2020 14:44

Merge remote-tracking branch 'upstream/master' into localization-gene…

33588cc

…rator

Fix indentation

776b138

ghost merged commit 729e241 into dotnet:master Jan 8, 2020

rolfbjarne added the not-notes-worthy Ignore for release notes label Sep 10, 2020

This pull request was closed.

	(String) prop.GetValue (null);
	prop.GetValue (null) as string;

[Localization] Add generator error messages #7531

[Localization] Add generator error messages #7531

Uh oh!

Conversation

ghost commented Dec 2, 2019 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spouliot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Dec 2, 2019 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

monojenkins commented Dec 3, 2019

Test results

Failed tests

Uh oh!

VincentDondain left a comment

Choose a reason for hiding this comment

Uh oh!

monojenkins commented Dec 4, 2019

Test results

Failed tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

monojenkins commented Dec 4, 2019

Uh oh!

monojenkins commented Dec 4, 2019

Uh oh!

monojenkins commented Dec 5, 2019

Test results

Failed tests

Uh oh!

monojenkins commented Dec 5, 2019

Uh oh!

monojenkins commented Dec 5, 2019

Uh oh!

monojenkins commented Dec 6, 2019

Test results

Failed tests

Uh oh!

monojenkins commented Dec 9, 2019

Test results

Failed tests

Uh oh!

monojenkins commented Dec 9, 2019

Test results

Failed tests

Uh oh!

monojenkins commented Dec 10, 2019

Uh oh!

monojenkins commented Dec 10, 2019

Uh oh!

ghost commented Dec 10, 2019

Uh oh!

rolfbjarne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

monojenkins commented Dec 10, 2019

Uh oh!

monojenkins commented Dec 10, 2019

Uh oh!

Uh oh!

monojenkins commented Dec 10, 2019

Uh oh!

monojenkins commented Dec 10, 2019

Test results

Failed tests

Uh oh!

Uh oh!

ghost commented Dec 2, 2019 •

edited by ghost

Loading

ghost commented Dec 2, 2019 •

edited by ghost

Loading