Skip to content

stop using System.CommandLine types that won't be public anymore #116065

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

Merged
merged 8 commits into from
May 31, 2025

Conversation

adamsitnik
Copy link
Member

Copy link
Contributor

@Copilot Copilot AI left a 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 replaces obsolete System.CommandLine types by switching help extensions to use ParseResult and Action<ParseResult> instead of HelpContext and IEnumerable<Func<HelpContext, bool>>. It also centralizes the post-build help customization into a new CustomizedHelpAction.

  • Change GetExtendedHelp in three root commands to void GetExtendedHelp(ParseResult) and remove legacy HelpContext-based enumerator.
  • Update UseExtendedHelp extension to accept an Action<ParseResult> and wire up CustomizedHelpAction.
  • Add CustomizedHelpAction implementation and an unused using import in CommandLineHelpers.cs.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/coreclr/tools/dotnet-pgo/PgoRootCommand.cs Updated GetExtendedHelp signature and logic to print examples for trace
src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Updated GetExtendedHelp signature and removed legacy layout iteration
src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs Updated GetExtendedHelp signature and removed legacy layout iteration
src/coreclr/tools/Common/CommandLineHelpers.cs Changed UseExtendedHelp to use CustomizedHelpAction and added helper
Comments suppressed due to low confidence (2)

src/coreclr/tools/Common/CommandLineHelpers.cs:133

  • [nitpick] The parameter name customizer is clear, but the unused discard _ in the root commands may be confusing. Consider using a descriptive name like parseResult for consistency.
public static RootCommand UseExtendedHelp(this RootCommand command, Action<ParseResult> customizer)

src/coreclr/tools/dotnet-pgo/PgoRootCommand.cs:263

  • The new GetExtendedHelp(ParseResult) behavior (especially for create-mibc and create-jittrace) lacks automated tests to verify the extended help output.
public static void GetExtendedHelp(ParseResult parseResult)

@@ -329,64 +329,57 @@ public ILCompilerRootCommand(string[] args) : base(".NET Native IL Compiler")
});
}

public static IEnumerable<Func<HelpContext, bool>> GetExtendedHelp(HelpContext _)
public static void GetExtendedHelp(ParseResult _)
Copy link
Member

Choose a reason for hiding this comment

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

This method is not returning anything after this change, so Get... name does not fit what it does anymore.

I would rename it to PrintHelp, DisplayHelp or something similar.

Copy link
Member

Choose a reason for hiding this comment

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

(I have not commented on all places with this name. All of them should be fixed.)

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM modulo a few nits

@jkotas jkotas merged commit f329648 into dotnet:main May 31, 2025
156 of 158 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants