[LiveLogger ] Localize strings #8682

rokonec · 2023-04-20T12:42:48Z

Partly Fixes #8391

Context

Ensure all new strings used in the logger are localizable

Changes Made

introduce strings in resx
using such resource strings
modify code to allow better localization (from write(part1); write(part2); write(part3) into str = formastringfromresources(); write(str)

Testing

local
unit tests

Notes

Rework some areas to enable localization.

Forgind

Looks good! And I appreciate all the comments on the new strings.

Forgind · 2023-04-20T15:36:12Z

src/MSBuild/LiveLogger/LiveLogger.cs

    {
        public override string ToString()
        {
+            string duration = Stopwatch.Elapsed.TotalSeconds.ToString("F1");


nit:
We're currently displaying hundredths of a second in the not-live logger, but it seems like we switched to tenths of a second for the live logger. Was that an intentional choice?

It is consistent in new LiveLogger, so I believe it is intentional. IMO, from practical point, tenths are just enough.

Forgind · 2023-04-20T15:38:13Z

src/MSBuild/LiveLogger/LiveLogger.cs

            return string.IsNullOrEmpty(TargetFramework)
-                ? $"{Indentation}{Project} {Target} ({Stopwatch.Elapsed.TotalSeconds:F1}s)"
-                : $"{Indentation}{Project} [{TargetFramework}] {Target} ({Stopwatch.Elapsed.TotalSeconds:F1}s)";
+                ? ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ProjectBuilding_NoTF",


I was thinking about ways we could combine these into one ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword call...I'm tempted to propose rearranging the arguments to put TF at the end, so we can always pass it even if it's null or empty but not necessarily display it or even having a custom part in the middle of the string, but I'm not convinced either change is actually better.

I though about that. I even have it coded as subpart but the code, IMO, was harder to read and translation was more confusing. It can be changed easily though, if we decide to do so.

src/MSBuild/Resources/Strings.resx

Forgind · 2023-04-20T15:59:26Z

src/MSBuild/Resources/Strings.resx

+    </comment>
+  </data>
+  <data name="ProjectFinished_NoTF" xml:space="preserve">
+    <value>{0}{1} {2} ({3}s)</value>


Suggested change

<value>{0}{1} {2} ({3}s)</value>

<value>{0}Building {1} {2} ({3}s)</value>

? (If so, then also below)

Intent of this PR was to make Live Logger localizable while keeping it exactly like it was.
If changes in strings are needed I'd rather do it in separate PR.

ladipro

Looks great! A couple of questions inline.

ladipro · 2023-04-24T09:41:57Z

src/MSBuild/Resources/Strings.resx

+  <data name="ProjectFinished_NoTF" xml:space="preserve">
+    <value>{0}{1} {2} ({3}s)</value>
+    <comment>
+      Project finished summary.
+      {0}: indentation - few spaces to visually indent row
+      {1}: project name
+      {2}: BuildResult_{X}
+      {3}: duration in seconds with 1 decimal point
+    </comment>
+  </data>
+  <data name="ProjectFinished_WithTF" xml:space="preserve">
+    <value>{0}{1} [2] {3} ({4}s)</value>
+    <comment>
+      Project finished summary including target framework information.
+      {0}: indentation - few spaces to visually indent row
+      {1}: project name
+      {2}: target framework
+      {3}: BuildResult_{X}
+      {4}: duration in seconds with 1 decimal point
+    </comment>
+  </data>
+  <data name="ProjectFinished_OutputPath" xml:space="preserve">
+    <value> → {0}</value>
+    <comment>
+      Info about project output - when known. Printed after ProjectFinished_NoTF or ProjectFinished_WithTF.
+      {0}: VT100 coded hyperlink to project output directory
+    </comment>
+  </data>
+  <data name="ProjectBuilding_NoTF" xml:space="preserve">
+    <value>{0}{1} {2} ({3}s)</value>
+    <comment>
+      Project finished summary.
+      {0}: indentation - few spaces to visually indent row
+      {1}: project name
+      {2}: target
+      {3}: duration in seconds with 1 decimal point
+    </comment>
+  </data>
+  <data name="ProjectBuilding_WithTF" xml:space="preserve">
+    <value>{0}{1} [2] {3} ({4}s)</value>
+    <comment>
+      Project finished summary including target framework information.
+      {0}: indentation - few spaces to visually indent row
+      {1}: project name
+      {2}: target framework
+      {3}: target
+      {4}: duration in seconds with 1 decimal point
+    </comment>
+  </data>


Do you think there's a chance non-English versions of these strings will look differently other than the seconds unit abbreviation? I wonder if we can have only one localized string {0}s and use it for all these.

Do you think there's a chance non-English versions of these strings will look differently

I see it not likely.

However, I am not expert in localization. It could be that theoretically some languages might not use spaces, other languages might despise brackets, some languages prefer different order of data presenting so they can prefer target before of project like target of project.

I'd rather play safe here...

The changes you're describing don't look like something that would be done as part of translation. Maybe there are RTL considerations, not sure. Not blocking but I'd like to ask our loc champion. @richaverma1 can you please provide guidance?

I have checked in past and none supported languages in MSBuild is, AFAIK, RTL.

ladipro · 2023-04-24T09:45:37Z

src/MSBuild/LiveLogger/LiveLogger.cs

    {
        public override string ToString()
        {
+            string duration = Stopwatch.Elapsed.TotalSeconds.ToString("F1");


Is the double -> string formatting locale-aware if done in code like this? My suggestion is to omit the ToString call here and change the placeholder in Strings.resx to something like {0:F1}.

Yes double.ToString does respect Thread.CurrentThread.CurrentCulture - I have also tested it.
Have though about if formatting rules belongs to resx. In this case I proffered it in application logic as I viewed presented decimal point precision as UX design responsibility which do not, IMO, belongs to translation team.
If we choose to change it in future, such change would not trigger need for translation.

Thank you. I guess I could argue either way - for the app logic to own the formatting so it's easier to change in the future and for the resource files to contain it so it can be localized. FWIW, we already have a precedence where the format is specified in .resx and it's translated to the same format in all languages. So it probably doesn't matter.

msbuild/src/Build/Resources/Strings.resx

Line 1481 in c6e6cd4

<value>{0}{1,-3}{2}: {3:0.000}s {4:0.000}s {5} ({6}) </value>

rokonec · 2023-04-24T20:52:53Z

I recommend to merge it as is - in order to limit future merge conflicts in area of code which will likely have some changes.
We can always improve it based on localization team recommendation later.

The double-bracing around this was missed in dotnet#8682.

The double-bracing around this was missed in #8682.

Localize LiveLogger strings.

9cb6176

Rework some areas to enable localization.

rokonec mentioned this pull request Apr 20, 2023

Make sure that the live logger is global ready #8391

Closed

3 tasks

Forgind approved these changes Apr 20, 2023

View reviewed changes

Minor syntax changes - by review comments.

58727f5

ladipro reviewed Apr 24, 2023

View reviewed changes

ladipro approved these changes Apr 24, 2023

View reviewed changes

rokonec added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Apr 24, 2023

JaynieBai merged commit 6300d22 into dotnet:main Apr 25, 2023

rainersigwald mentioned this pull request Apr 25, 2023

Include placeholder for TF in loc strings #8705

Merged

rainersigwald added a commit to rainersigwald/msbuild that referenced this pull request Apr 25, 2023

Include placeholder for TF in loc strings

5a3565a

The double-bracing around this was missed in dotnet#8682.

rainersigwald added a commit that referenced this pull request Apr 25, 2023

Include placeholder for TF in loc strings (#8705)

5738af0

The double-bracing around this was missed in #8682.

dependabot bot mentioned this pull request Aug 6, 2025

Bump Azure.Storage.Blobs and 38 others rajkumar-rangaraj/opentelemetry-dotnet-instrumentation#1006

Closed

dependabot bot mentioned this pull request Aug 15, 2025

Bump Microsoft.Build from 15.9.20 to 17.11.31 rajkumar-rangaraj/opentelemetry-dotnet-instrumentation#1036

Closed

dependabot bot mentioned this pull request Oct 15, 2025

Bump Microsoft.Build from 15.9.20 to 17.11.48 rajkumar-rangaraj/opentelemetry-dotnet-instrumentation#1085

Open

	<value>{0}{1} {2} ({3}s)</value>
	<value>{0}Building {1} {2} ({3}s)</value>

Uh oh!

[LiveLogger ] Localize strings #8682

[LiveLogger ] Localize strings #8682

Uh oh!

Conversation

rokonec commented Apr 20, 2023

Context

Changes Made

Testing

Notes

Uh oh!

Forgind left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ladipro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rokonec commented Apr 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rokonec commented Apr 24, 2023 •

edited

Loading