-
Notifications
You must be signed in to change notification settings - Fork 64
[ApiXmlAdjuster, Bytecode] Make error/warning strings localizable. #693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
brendanzagaeski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. I'll add a few small ideas for wording changes that might be handy for the localization team.
| <resheader name="writer"> | ||
| <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value> | ||
| </resheader> | ||
| <data name="ApiXmlAdjuster_0001" xml:space="preserve"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For simplicity in the scope of this initial localizability work, I'd say all the strings for ApiXmlAdjuster can be skipped and omitted from the .resx file for now. Since at the moment all of the ApiXmlAdjuster messages are reported as just log output in MSBuild rather than warnings or errors, that will allow deferring any decisions about how to handle the Warning: and Error: prefixes on these strings as well as decisions about whether any of the messages should be promoted to be MSBuild warnings or errors. That work can be considered again in the future.
| <comment>{0} - File name.</comment> | ||
| </data> | ||
| <data name="Bytecode_0003" xml:space="preserve"> | ||
| <value>Failed to match expression '{0}', expected {1} parameter(s) but got {2} parameter(s).</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking parameter(s) might be tricky to localize, so maybe:
Failed to match expression '{0}'. Number of parameters expected: {1}. Number of parameters provided: {2}.
| {1}, {2} - Integer.</comment> | ||
| </data> | ||
| <data name="Bytecode_0004" xml:space="preserve"> | ||
| <value>No parameter match for '{0}.{1}'. (expression: '{2}')</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid interpretation as something like "No parameters match for...", maybe:
<value>No parameter match was found for '{0}.{1}'. (expression: '{2}')</value>
| {1} - Exception.</comment> | ||
| </data> | ||
| <data name="Bytecode_0006" xml:space="preserve"> | ||
| <value>Differing number of 'throws' declarations on '{0}{1}'.</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the target localization languages would all have standard ways to handle this as it is, but maybe if it's accurate:
<value>Found an unexpected number of 'throws' declarations on '{0}{1}'.</value>
| } catch (Exception e) { | ||
| Log.Warning (0, "class-parse: warning: Could not load .jar entry '{0}': {1}", | ||
| entry.Name, e); | ||
| Log.Warning (0, Java.Interop.Localization.Resources.Bytecode_0001, entry.Name, e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these Bytecode errors and warnings are propagated to MSBuild errors and warnings, I think if it won't be too messy or complicated to do, it'd be great to add message code numbers for them as part of this PR (probably with new little Bytecode.Log.LogCodedError() and Bytecode.Log.LogCodedWarning() methods?). That'll align them with the generator messages and the Xamarin.Android.Build.Tasks messages, and it will provide the possibility to gather some telemetry about which codes users see, which might be handy.
|
We decided we are only going to focus on localizing MSBuild error/warnings at this time. |
Updates
Xamarin.Android.Tools.ApiXmlAdjusterandXamarin.Android.Tools.Bytecodeto be localizable, building on work done in #689.