-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
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 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
GetExtendedHelpin three root commands tovoid GetExtendedHelp(ParseResult)and remove legacyHelpContext-based enumerator. - Update
UseExtendedHelpextension to accept anAction<ParseResult>and wire upCustomizedHelpAction. - Add
CustomizedHelpActionimplementation and an unusedusingimport inCommandLineHelpers.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
customizeris clear, but the unused discard_in the root commands may be confusing. Consider using a descriptive name likeparseResultfor 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 forcreate-mibcandcreate-jittrace) lacks automated tests to verify the extended help output.
public static void GetExtendedHelp(ParseResult parseResult)
Co-authored-by: Copilot <[email protected]>
| } | ||
|
|
||
| public static IEnumerable<Func<HelpContext, bool>> GetExtendedHelp(HelpContext _) | ||
| public static void GetExtendedHelp(ParseResult _) |
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 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.
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 have not commented on all places with this name. All of them should be fixed.)
jkotas
left a comment
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.
LGTM modulo a few nits
Context: dotnet/command-line-api#2563