Skip to content

Fix dnx --help and tool commands to use valid .NET tool examples #51009

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Sep 29, 2025
Merged

Fix dnx --help and tool commands to use valid .NET tool examples #51009

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ad6b924
Initial plan
Copilot Sep 25, 2025
7c016c0
Add tool-specific package identity descriptions to replace generic ex…
Copilot Sep 25, 2025
fd497be
Update copilot-instructions with dogfood testing approach for CLI cha…
Copilot Sep 25, 2025
64be82e
Refactor to use parameterized package identity argument methods per r…
Copilot Sep 25, 2025
b03b828
Update CLI snapshot test baselines for tool-specific package examples
Copilot Sep 25, 2025
bc0f3a2
Use single format string with placeholders instead of separate resour…
Copilot Sep 25, 2025
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
3 changes: 3 additions & 0 deletions src/Cli/dotnet/CliStrings.resx
View file
Open in desktop
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

@baronfel baronfel Sep 25, 2025

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

Copilot AI Sep 25, 2025

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
2 changes: 1 addition & 1 deletion src/Cli/dotnet/Commands/Tool/Install/ToolInstallCommandParser.cs
View file
Open in desktop
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.RequiredToolPackageIdentityArgument();

public static readonly Option<string> VersionOption = new("--version")
{
Expand Down
8 changes: 8 additions & 0 deletions src/Cli/dotnet/CommonArguments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ public static DynamicArgument<PackageIdentityWithRange> RequiredPackageIdentityA
Arity = ArgumentArity.ExactlyOne,
};

public static DynamicArgument<PackageIdentityWithRange> RequiredToolPackageIdentityArgument() =>
new("packageId")
{
Description = CliStrings.ToolPackageIdentityArgumentDescription,
CustomParser = (ArgumentResult argumentResult) => ParsePackageIdentityWithVersionSeparator(argumentResult.Tokens[0]?.Value)!.Value,
Arity = ArgumentArity.ExactlyOne,
};

Copy link
Member

@baronfel baronfel Sep 25, 2025

Choose a reason for hiding this comment

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

@copilot instead of creating a new parallel packageId argument, use a single shared implementation that takes a description as input. Then update call sites to pass the description that is most appropriate. Don't forget that when you change argument/option names or descriptions, you need to update the cli's snapshot test baselines as well.

Copy link
Contributor Author

Copilot AI Sep 25, 2025

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 shared implementation with parameterized description as suggested. The changes in b03b828:

  1. Replaced duplicate methods with parameterized versions:

    • RequiredPackageIdentityArgument(string description)
    • OptionalPackageIdentityArgument(string description)
    • Backward-compatible overloads maintained

  2. Updated call sites to pass appropriate descriptions:

    • Tool commands (install/update/execute/dnx): use CliStrings.ToolPackageIdentityArgumentDescription with [email protected]
    • Package commands: continue using generic description with [email protected]

  3. Updated CLI snapshot baselines using the MSBuild targets - the snapshot tests now pass and show the tool commands correctly use the new examples.

The CLI completion system now properly distinguishes between tool and package contexts while maintaining a clean, shared implementation.

private static PackageIdentityWithRange? ParsePackageIdentityWithVersionSeparator(string? packageIdentity, char versionSeparator = '@')
{
if (string.IsNullOrEmpty(packageIdentity))
Expand Down
5 changes: 5 additions & 0 deletions src/Cli/dotnet/xlf/CliStrings.cs.xlf
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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

Loading