Skip to content
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

[BLOCKED] update System.CommandLine #7559

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Feb 3, 2025

Edit: this PR is blocked until NuGet/NuGet.Client#6236 gets merged (ETA is April).

This PR consists of:

@adamsitnik adamsitnik requested a review from a team as a code owner February 3, 2025 09:49

// Hook so we can cancel and exit when ctrl+c is pressed.
var cancellationTokenSource = new CancellationTokenSource();
Console.CancelKeyPress += (sender, e) =>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now being performed out of the box by System.CommandLine. All you need it to pass the cancellation token in SetAction to given async method

var rootCommand = DiffCommand.CreateCommandLineOptions();
rootCommand.Handler = CommandHandler.Create(new DiffCommand.Handler(RunAsync));

// Parse the incoming args so we can give warnings when deprecated options are used.
Copy link
Member Author

Choose a reason for hiding this comment

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

I was not able to find any other usage of Program.RunAsync so I've assumed that it's most likely some leftover and removed it.

FWIW if there will be any parsing errors detected, the Parse(args).InvokeAsync() call is going to get them printed to output and also return error code.

Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.54%. Comparing base (8fe7aeb) to head (232f8ce).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7559   +/-   ##
=======================================
  Coverage   96.54%   96.54%           
=======================================
  Files        1454     1454           
  Lines      352505   352505           
  Branches    11478    11478           
=======================================
+ Hits       340329   340331    +2     
+ Misses       9262     9260    -2     
  Partials     2914     2914           

@adamsitnik adamsitnik marked this pull request as draft February 6, 2025 11:22
@adamsitnik adamsitnik changed the title update System.CommandLine [BLOCKED] update System.CommandLine Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant