Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ Testing:
- Examples:
- `dotnet test test/dotnet.Tests/dotnet.Tests.csproj --filter "Name~ItShowsTheAppropriateMessageToTheUser"`
- `dotnet exec artifacts/bin/redist/Debug/dotnet.Tests.dll -method "*ItShowsTheAppropriateMessageToTheUser*"`
- To test CLI command changes:
- Build the redist SDK: `./build.sh` from repo root
- Create a dogfood environment: `source eng/dogfood.sh`
- Test commands in the dogfood shell (e.g., `dnx --help`, `dotnet tool install --help`)
- The dogfood script sets up PATH and environment to use the newly built SDK

Output Considerations:
- When considering how output should look, solicit advice from baronfel.
Expand Down
3 changes: 3 additions & 0 deletions src/Cli/dotnet/CliStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,9 @@ The default is 'false.' However, when targeting .NET 7 or lower, the default is
<data name="PackageIdentityArgumentDescription" xml:space="preserve">
<value>Package reference in the form of a package identifier like 'Newtonsoft.Json' or package identifier and version separated by '@' like '[email protected]'.</value>
</data>
<data name="ToolPackageIdentityArgumentDescription" xml:space="preserve">
<value>Package reference in the form of a package identifier like 'dotnetsay' or package identifier and version separated by '@' like '[email protected]'.</value>
</data>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I'm going to nitpick once again. Instead of separate Description strings for tool packages and non-tool-packages, make a single format string with placeholders for a package id and version, and then update the callsites to use the templated format string, passing in the existing/appropriate package id and version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored to use a single format string with placeholders as suggested. Changes in bc0f3a2:

  1. Single format string: Replaced duplicate resource strings with PackageIdentityArgumentDescription using {0} and {1} placeholders
  2. Updated method signatures: Changed CommonArguments methods to accept examplePackage and exampleVersion parameters
  3. Context-specific examples:
    • Tool commands: "dotnetsay", "2.1.7"
    • Non-tool commands: "Newtonsoft.Json", "13.0.3" (default)
  4. Runtime formatting: Uses string.Format() to populate placeholders

This eliminates the duplicate resource string while maintaining clean separation between contexts through parameterization.

<data name="PackageIdentityArgumentIdOrVersionIsNull" xml:space="preserve">
<value>Package reference id and version must not be null.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.DotNet.Cli.Commands.Tool.Install;

internal static class ToolInstallCommandParser
{
public static readonly Argument<PackageIdentityWithRange> PackageIdentityArgument = CommonArguments.RequiredPackageIdentityArgument();
public static readonly Argument<PackageIdentityWithRange> PackageIdentityArgument = CommonArguments.RequiredPackageIdentityArgument(CliStrings.ToolPackageIdentityArgumentDescription);

public static readonly Option<string> VersionOption = new("--version")
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.DotNet.Cli.Commands.Tool.Update;

internal static class ToolUpdateCommandParser
{
public static readonly Argument<PackageIdentityWithRange?> PackageIdentityArgument = CommonArguments.OptionalPackageIdentityArgument();
public static readonly Argument<PackageIdentityWithRange?> PackageIdentityArgument = CommonArguments.OptionalPackageIdentityArgument(CliStrings.ToolPackageIdentityArgumentDescription);

public static readonly Option<bool> UpdateAllOption = ToolAppliedOption.UpdateAllOption;

Expand Down
10 changes: 8 additions & 2 deletions src/Cli/dotnet/CommonArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,23 @@ namespace Microsoft.DotNet.Cli
internal class CommonArguments
{
public static DynamicArgument<PackageIdentityWithRange?> OptionalPackageIdentityArgument() =>
OptionalPackageIdentityArgument(CliStrings.PackageIdentityArgumentDescription);

public static DynamicArgument<PackageIdentityWithRange?> OptionalPackageIdentityArgument(string description) =>
new("packageId")
{
Description = CliStrings.PackageIdentityArgumentDescription,
Description = description,
CustomParser = (ArgumentResult argumentResult) => ParsePackageIdentityWithVersionSeparator(argumentResult.Tokens[0]?.Value),
Arity = ArgumentArity.ZeroOrOne,
};

public static DynamicArgument<PackageIdentityWithRange> RequiredPackageIdentityArgument() =>
RequiredPackageIdentityArgument(CliStrings.PackageIdentityArgumentDescription);

public static DynamicArgument<PackageIdentityWithRange> RequiredPackageIdentityArgument(string description) =>
new("packageId")
{
Description = CliStrings.PackageIdentityArgumentDescription,
Description = description,
CustomParser = (ArgumentResult argumentResult) => ParsePackageIdentityWithVersionSeparator(argumentResult.Tokens[0]?.Value)!.Value,
Arity = ArgumentArity.ExactlyOne,
};
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.zh-Hans.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.zh-Hant.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ _testhost() {
'--allow-roll-forward[Allow a .NET tool to roll forward to newer versions of the .NET runtime if the runtime it targets isn'\''t installed.]' \
'--help[Show command line help.]' \
'-h[Show command line help.]' \
':packageId -- Package reference in the form of a package identifier like '\''Newtonsoft.Json'\'' or package identifier and version separated by '\''@'\'' like '\''[email protected]'\''.:->dotnet_dynamic_complete' \
':packageId -- Package reference in the form of a package identifier like '\''dotnetsay'\'' or package identifier and version separated by '\''@'\'' like '\''[email protected]'\''.:->dotnet_dynamic_complete' \
&& ret=0
case $state in
(dotnet_dynamic_complete)
Expand Down Expand Up @@ -1150,7 +1150,7 @@ _testhost() {
'--all[Update all tools.]' \
'--help[Show command line help.]' \
'-h[Show command line help.]' \
'::packageId -- Package reference in the form of a package identifier like '\''Newtonsoft.Json'\'' or package identifier and version separated by '\''@'\'' like '\''[email protected]'\''.:->dotnet_dynamic_complete' \
'::packageId -- Package reference in the form of a package identifier like '\''dotnetsay'\'' or package identifier and version separated by '\''@'\'' like '\''[email protected]'\''.:->dotnet_dynamic_complete' \
&& ret=0
case $state in
(dotnet_dynamic_complete)
Expand Down Expand Up @@ -1228,7 +1228,7 @@ _testhost() {
'-v=[Set the MSBuild verbosity level. Allowed values are q\[uiet\], m\[inimal\], n\[ormal\], d\[etailed\], and diag\[nostic\].]:LEVEL:((d\:"d" detailed\:"detailed" diag\:"diag" diagnostic\:"diagnostic" m\:"m" minimal\:"minimal" n\:"n" normal\:"normal" q\:"q" quiet\:"quiet" ))' \
'--help[Show command line help.]' \
'-h[Show command line help.]' \
':packageId -- Package reference in the form of a package identifier like '\''Newtonsoft.Json'\'' or package identifier and version separated by '\''@'\'' like '\''[email protected]'\''.:->dotnet_dynamic_complete' \
':packageId -- Package reference in the form of a package identifier like '\''dotnetsay'\'' or package identifier and version separated by '\''@'\'' like '\''[email protected]'\''.:->dotnet_dynamic_complete' \
'*::commandArguments -- Arguments forwarded to the tool: ' \
&& ret=0
case $state in
Expand Down
Loading