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

Make it easier to use cancellation tokens #2271

Closed
KeithHenry opened this issue Sep 5, 2023 · 9 comments
Closed

Make it easier to use cancellation tokens #2271

KeithHenry opened this issue Sep 5, 2023 · 9 comments

Comments

@KeithHenry
Copy link

The various SetHandler overloads are great for hooking up simple actions.

However, if you need a cancellation token none of these are available and you have to use:

command.SetHandler(
    async context => 
        await MyMethod(
            context.ParseResult.GetValueForArgument(anArgument)), 
            context.ParseResult.GetValueForOption(anOption),
            context.GetCancellationToken()));

Which feels very clunky compared to the methods without the need for cancellation.

Please could we add:

command.SetCancellableHandler(
    async (arg, opt, token) => 
        await MyMethod(
            arg, 
            opt,
            token), 
    anArgument, 
    anOption);

// or even
command.SetCancellableHandler(
    MyMethod, 
    anArgument, 
    anOption);

I realise this could be done with extension methods, but they wouldn't have access to the internal GetValueForHandlerParameter that the overloads of SetHandler do.

@KalleOlaviNiemitalo
Copy link

The API is quite different on the main branch. There, CliCommand.SetAction gives the CancellationToken as a separate parameter to the asynchronous action delegate.

/// <summary>
/// Sets an asynchronous action to be run when the command is invoked.
/// </summary>
public void SetAction(Func<ParseResult, CancellationToken, Task> action)
{
if (action is null)
{
throw new ArgumentNullException(nameof(action));
}
Action = new AnonymousAsynchronousCliAction(async (context, cancellationToken) =>
{
await action(context, cancellationToken);
return 0;
});
}
/// <summary>
/// Sets an asynchronous action when the command is invoked.
/// </summary>
/// <remarks>The value returned from the <paramref name="action"/> delegate can be used to set the process exit code.</remarks>
public void SetAction(Func<ParseResult, CancellationToken, Task<int>> action)
{
if (action is null)
{
throw new ArgumentNullException(nameof(action));
}
Action = new AnonymousAsynchronousCliAction(action);
}

That feature is not currently available for synchronous actions but IIRC there is another issue tracking that.

@KalleOlaviNiemitalo
Copy link

CancellationToken as a separate parameter was added in #2044.

@KeithHenry
Copy link
Author

KeithHenry commented Sep 5, 2023

@KalleOlaviNiemitalo thanks, so this is already done - #2044 back in Feb 2023 it seems.

Why is the NuGet 2.0.0-beta4.22272.1 package from May 2022 so far behind?

@KalleOlaviNiemitalo
Copy link

According to #1882 (comment), they want to collect many breaking changes to the same preview release. But the progress seems stalled; only one pull request was merged in August 2023 and I don't think it addressed any of the issues remaining in #1891. I hope the team will have more time for this after the .NET 8 release.

@KeithHenry
Copy link
Author

@KalleOlaviNiemitalo thanks for the information - it does look rather stalled and dying on the vine :-(

Still, this issue isn't needed given there is already a fix in a future candidate branch. Thanks for your help.

@KalleOlaviNiemitalo
Copy link

Well, .NET SDK 8.0 preview 7 heavily uses System.CommandLine (I found using System.CommandLine in 280 *.cs files of https://github.com/dotnet/sdk/), so I don't think Microsoft intends to cease maintenance.

@baronfel
Copy link
Member

baronfel commented Sep 5, 2023

Don't worry, this library is supported. We're trying to tackle all of the issues raised by the .NET Runtime team in API review before releasing a new beta - it's unfair to ask the community to continually absorb API surface area changes so we've been holding off until the review process is complete.

If individual users would like to use the daily builds, the README has instructions on how to use the daily builds, it's a matter of adding a NuGet source to a NuGet.config file - note that this will open you up to API changes when you update packages, however. We've been doing this in the dotnet CLI with auto-updates, so we're accepting that churn to get the latest features and fixes ahead of a stable release.

@KalleOlaviNiemitalo
Copy link

What's the support policy on the daily builds feed — will it still be there ten years from now?

@KeithHenry
Copy link
Author

@baronfel That seems a good long term goal to get to a 2.0 release candidate (or 3.0 if breaking?)

It's been more than a year, and may be more than a year to finish it. In the meantime there's nothing between the custom NuGet host nightly (which is honestly a bit too beta for us) and the stale NuGet.org package from 2022.

Any chance of a new (beta 5) package with the many enhancements/fixes since last year, in the full knowledge that it isn't final and the project is still working towards that #1891 aspirational goal in the long term?

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

No branches or pull requests

3 participants