Localize the text in the common target #10388
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JanKrivanek
approved these changes
Jul 22, 2024
Nirmal4G
suggested changes
Jul 26, 2024
Comment on lines
838
to
841
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationError)' == 'true' and '$(BuildingInsideVisualStudio)' == 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithBuildingInsideVisualStudio" Severity="Error" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | ||
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationError)' == 'true' and '$(BuildingInsideVisualStudio)' != 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithoutBuildingInsideVisualStudio" Severity="Error" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | ||
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationWarning)' == 'true' and '$(BuildingInsideVisualStudio)' == 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithBuildingInsideVisualStudio" Severity="Warning" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | ||
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationWarning)' == 'true' and '$(BuildingInsideVisualStudio)' != 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithoutBuildingInsideVisualStudio" Severity="Warning" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> |
There was a problem hiding this comment.
Suggested change
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationError)' == 'true' and '$(BuildingInsideVisualStudio)' == 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithBuildingInsideVisualStudio" Severity="Error" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | |
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationError)' == 'true' and '$(BuildingInsideVisualStudio)' != 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithoutBuildingInsideVisualStudio" Severity="Error" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | |
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationWarning)' == 'true' and '$(BuildingInsideVisualStudio)' == 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithBuildingInsideVisualStudio" Severity="Warning" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | |
| <MSBuildInternalMessage Condition=" '$(_InvalidConfigurationWarning)' == 'true' and '$(BuildingInsideVisualStudio)' != 'true'" ResourceName="CommonTarget.InvalidConfigurationTextWithoutBuildingInsideVisualStudio" Severity="Warning" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | |
| <PropertyGroup> | |
| <_InvalidConfigurationMessageResourceName Condition=" '$(BuildingInsideVisualStudio)' == 'true' ">CommonSdk.InvalidConfigurationTextWhenBuildingInsideVisualStudio</_InvalidConfigurationMessageResourceName> | |
| <_InvalidConfigurationMessageResourceName Condition=" '$(BuildingInsideVisualStudio)' != 'true' ">CommonSdk.InvalidConfigurationTextWhenBuildingOutsideVisualStudio</_InvalidConfigurationMessageResourceName> | |
| </PropertyGroup> | |
| <MSBuildInternalMessage Code="MSB3541" | |
| ResourceName="$(_InvalidConfigurationMessageResourceName)" | |
| Severity="$(_InvalidConfigurationMessageSeverity)" | |
| FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)" | |
| Condition="'$(_InvalidConfigurationMessageSeverity)' != ''" /> | |
| <PropertyGroup> | |
| <_InvalidConfigurationMessageSeverity /> | |
| <_InvalidConfigurationMessageResourceName /> | |
| </PropertyGroup> |
Linked Changes
If you are applying this patch, replace the _InvalidConfiguration[Warning|Error] prperties with _InvalidConfigurationMessageSeverity like so:
Replace these lines
msbuild/src/Tasks/Microsoft.Common.CurrentVersion.targets
Lines 205 to 206 in 47f364c
with these lines
<_InvalidConfigurationMessageSeverity Condition=" '$(SkipInvalidConfigurations)' == 'true' ">Warning</_InvalidConfigurationMessageSeverity>
<_InvalidConfigurationMessageSeverity Condition=" '$(SkipInvalidConfigurations)' != 'true' ">Error</_InvalidConfigurationMessageSeverity>Important
You could use the above pattern everywhere else. It makes the code easy to read. I don't know if cleaning up the properties is needed or not but kept there for clarity.
src/Tasks/Resources/Strings.resx
Outdated
| <comment>{StrBegin="MSB3540: "}</comment> | ||
| </data> | ||
|
|
||
| <data name="CommonTarget.InvalidConfigurationTextWithBuildingInsideVisualStudio"> |
There was a problem hiding this comment.
Suggested change
| <data name="CommonTarget.InvalidConfigurationTextWithBuildingInsideVisualStudio"> | |
| <data name="CommonSdk.InvalidConfigurationTextWhenBuildingInsideVisualStudio"> |
src/Tasks/Resources/Strings.resx
Outdated
| <comment>LOCALIZATION: Do not localize the words "BaseOutputPath/OutputPath", "Configuration" and "Platform"</comment> | ||
| </data> | ||
|
|
||
| <data name="CommonTarget.InvalidConfigurationTextWithoutBuildingInsideVisualStudio"> |
There was a problem hiding this comment.
Suggested change
| <data name="CommonTarget.InvalidConfigurationTextWithoutBuildingInsideVisualStudio"> | |
| <data name="CommonSdk.InvalidConfigurationTextWhenBuildingOutsideVisualStudio"> |
Nirmal4G
suggested changes
Aug 20, 2024
| @@ -834,23 +834,20 @@ Copyright (C) Microsoft Corporation. All rights reserved. | |||
| <Target | |||
| Name="_CheckForInvalidConfigurationAndPlatform" | |||
| BeforeTargets="$(BuildDependsOn);Build;$(RebuildDependsOn);Rebuild;$(CleanDependsOn);Clean"> | |||
|
|
|||
There was a problem hiding this comment.
nit: undo the line deletion!
| </PropertyGroup> | ||
|
|
||
| <Error Condition=" '$(_InvalidConfigurationError)' == 'true' " Text="$(_InvalidConfigurationMessageText)"/> | ||
| <Warning Condition=" '$(_InvalidConfigurationWarning)' == 'true' " Text="$(_InvalidConfigurationMessageText)"/> | ||
| <MSBuildInternalMessage ResourceName="$(_InvalidConfigurationMessageResourceName)" Severity="$(_InvalidConfigurationMessageSeverity)" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> |
There was a problem hiding this comment.
Sorry about my mistake.
Suggested change
| <MSBuildInternalMessage ResourceName="$(_InvalidConfigurationMessageResourceName)" Severity="$(_InvalidConfigurationMessageSeverity)" FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)"/> | |
| <MSBuildInternalMessage | |
| ResourceName="$(_InvalidConfigurationMessageResourceName)" | |
| Severity="$(_InvalidConfigurationMessageSeverity)" | |
| FormatArguments="$(MSBuildProjectFile);$(_OriginalConfiguration);$(_OriginalPlatform)" | |
| Condition="'$(_InvalidConfigurationMessageSeverity)' != ''" /> |
Important
For the condition, we could have used the same output path check here also
but we must not forget to update both when it changes in the future.
That's why I have used Severity instead.
There was a problem hiding this comment.
Thanks a lot @Nirmal4G
Nirmal4G
reviewed
Aug 21, 2024
| <MSBuildInternalMessage | ||
| Condition="'$(_NonExistentProjectReferenceSecuity)' != ''" | ||
| ResourceName="CommonSdk.NonExistentProjectReference" | ||
| Severity="$(_NonExistentProjectReferenceSecuity)" |
There was a problem hiding this comment.
Suggested change
| Severity="$(_NonExistentProjectReferenceSecuity)" | |
| Severity="$(_NonExistentProjectReferenceSeverity)" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #10171
Context
MsBuild produces unlocalized messages for some specific validation cases that happen in scope of target.
The example of the task usage: https://github.com/YuliiaKovalova/msbuild/blob/be21253d85f7766356880d376e26aaa69c34c4cd/src/Tasks/Microsoft.Common.CurrentVersion.targets#L862
Changes Made
Refactor the existing messages with the recently added task allows to produce localized messages:
https://github.com/dotnet/msbuild/blob/79dff86b18613cfe3510b719ac28e8a8c3e7f96c/src/Tasks/MSBuildInternalMessage.cs , produced by with hardcoded text.
Testing
Notes