-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Adjust message for bad #:package / #:sdk to indicate what the parts mean #49483
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR refines the example text in the InvalidDirectiveName
message to use the placeholders “Name” and “Version” instead of “Abc” and “Xyz,” making it clearer what each part represents.
- Updated the example in all XLF localization files to
'#:{0} Name{1}Version'
- Updated the primary
.resx
resource with the same example change
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hant.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hans.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.tr.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.ru.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.pt-BR.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.pl.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.ko.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.ja.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.it.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.fr.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.es.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.de.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/xlf/CliCommandStrings.cs.xlf | Example text updated for InvalidDirectiveName |
src/Cli/dotnet/Commands/CliCommandStrings.resx | Example text updated for InvalidDirectiveName |
Comments suppressed due to low confidence (1)
src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hant.xlf:1439
- Avoid manually editing
.xlf
localization files. Instead, update the source.resx
entry and run the/t:UpdateXlf
MSBuild target to regenerate these translations consistently.
<trans-unit id="InvalidDirectiveName">
@@ -1520,7 +1520,7 @@ Tool '{1}' (version '{2}') was successfully installed. Entry is added to the man | |||
<comment>{0} is the file path and line number.</comment> | |||
</data> | |||
<data name="InvalidDirectiveName" xml:space="preserve"> | |||
<value>The directive at '{2}' should contain a name without special characters and an optional version separated by '{1}' like '#:{0} Abc{1}Xyz'.</value> | |||
<value>The directive at '{2}' should contain a name without special characters and an optional version separated by '{1}' like '#:{0} Name{1}Version'.</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.
This error can be reported for #:property
as well though, where the error text #:property Name=Version
might be confusing?
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.
It looks like this message is already oriented around "a name and optional version" scenarios, and, properties have their own diagnostic messages above this one.
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 will go back and adjust the <data name="..."
value to indicate the scenarios this message is used for.
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.
Actually, I see that DotnetProjectConvertTests.Directives_InvalidName
is expecting some property directives to report this diagnostic? In which case, the suggestion to use an optional version probably doesn't make sense, this message could probably suggest using "an optional value" and "Name=Value" instead, to generalize.
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.
Right, the message shouldn't mention version in the text, I've missed that. It's true properties have their own diagnostic messages but also can use this one as well. Using "optional value" and "Name{1}Value" sounds good to me. Thanks.
Just happened to see this message on the CLI while working on
#:project
in the IDE, and felt that more specific names for the parts a laPropertyDirectiveMissingParts
message would be helpful here. In both the project and sdk cases these parts are a name and version.