Refactor: Switch from CommandLineParser to System.CommandLine (#1016)#3014
Refactor: Switch from CommandLineParser to System.CommandLine (#1016)#3014abdulrahmanhossam wants to merge 4 commits intodotnet:masterfrom
Conversation
|
@dotnet-policy-service agree |
| [Option('a', "artifacts", Required = false, HelpText = "Valid path to accessible directory")] | ||
| public DirectoryInfo? ArtifactsDirectory { get; set; } | ||
|
|
||
| [Option("outliers", Required = false, Default = OutlierMode.RemoveUpper, HelpText = "DontRemove/RemoveUpper/RemoveLower/RemoveAll")] |
There was a problem hiding this comment.
Default = OutlierMode.RemoveUpper value also need to be migrated.
Currently it seems OutlierMode.DontRemove is used as default value.
|
When running Therefore, I though it need to fix these tests first. Other points of concern are listed below.
|
|
Thanks a lot for the detailed review and catching these points, @filzrev! I have pushed a new commit that addresses all your concerns:
The local build is completely green now. Let me know if there is anything else needed! |
| DisassemblerFilters = parseResult.GetValue(CommandLineOptions.DisassemblerFiltersOption) ?? Array.Empty<string>(), | ||
| DisassemblerDiff = parseResult.GetValue(CommandLineOptions.DisassemblerDiffOption), | ||
| Outliers = parseResult.GetValue(CommandLineOptions.OutliersOption), | ||
| WasmArgs = parseResult.GetValue(CommandLineOptions.WasmJavaScriptEngineArgumentsOption), |
There was a problem hiding this comment.
There are following two properties
- WasmArgs (Option)
- WasmJavaScriptEngineArguments (Plain Property)
I though WasmArgs option should be renamed to WasmJavaScriptEngineArguments.
And existing WasmJavaScriptEngineArguments plain property should be removed.
| { Description = "Sets the recursive depth for the disassembler. Default is 1.", DefaultValueFactory = _ => 1 }; | ||
|
|
||
| public static readonly Option<OutlierMode> OutliersOption = new("--outliers") | ||
| { Description = "Specifies outlier detection mode. Default is DontRemove.", DefaultValueFactory = _ => OutlierMode.DontRemove }; |
There was a problem hiding this comment.
It need to set default value OutlierMode.RemoveUpper here to keep existing behavior.
3 participants
Description
This PR addresses issue #1016 by migrating the command-line argument parsing logic from the legacy
CommandLineParserto the modern and stableSystem.CommandLine(v2.0).Key Changes
CommandLineParserandMcMaster.Extensions.CommandLineUtilsdependencies.System.CommandLinepackage reference.CommandLineOptionsto define options using the new API.ConfigParserto handle the new parsing result and map it back to the options object.Testing
Fixes #1016