Added support for async programming by joseangelmt · Pull Request #390

Added support for async programming by joseangelmt · Pull Request #390 · commandlineparser/commandline

With C# 7.1 you can create a pure async console application with an async Task Main() or async Task<int> Main()entry point.
The library is not easy to use in this scenario, so I added extension methods that complements all the the existing overloads of CommandLine.ParserResultExtensions.WithParsed and CommandLine.ParserResultExtensions.MapResult extension methods.

With this new async extention methods you can create async versions of sampleapps like this..

using CommandLine;
using System;
using System.Threading.Tasks;

namespace ConsoleApp1
{
    [Verb("add", HelpText = "Add file contents to the index.")]
    class AddOptions
    {
        //normal options here
    }

    [Verb("commit", HelpText = "Record changes to the repository.")]
    class CommitOptions
    {
        //normal options here
    }

    [Verb("clone", HelpText = "Clone a repository into a new directory.")]
    class CloneOptions
    {
        //normal options here
    }

    class Program
    {
        static async Task<int> Main(string[] args)
        {
            return await Parser.Default.ParseArguments<AddOptions, CommitOptions, CloneOptions>(args)
              .MapResultAsync(
                (AddOptions opts) => RunAddAndReturnExitCodeAsync(opts),
                (CommitOptions opts) => RunCommitAndReturnExitCodeAsync(opts),
                (CloneOptions opts) => RunCloneAndReturnExitCodeAsync(opts),
                errs => Task.FromResult(1));
        }

        static Task<int> RunAddAndReturnExitCodeAsync(AddOptions opts)
        {
            throw new NotImplementedException();
        }

        private static Task<int> RunCommitAndReturnExitCodeAsync(CommitOptions opts)
        {
            throw new NotImplementedException();
        }

        private static Task<int> RunCloneAndReturnExitCodeAsync(CloneOptions opts)
        {
            throw new NotImplementedException();
        }
    }
}

or like this

using CommandLine;
using System;
using System.Collections.Generic;
using System.Threading.Tasks;

namespace ConsoleApp1
{
    class Options
    {
        [Option('r', "read", Required = true, HelpText = "Input files to be processed.")]
        public IEnumerable<string> InputFiles { get; set; }

        // Omitting long name, defaults to name of property, ie "--verbose"
        [Option(Default = false, HelpText = "Prints all messages to standard output.")]
        public bool Verbose { get; set; }

        [Option("stdin", Default = false, HelpText = "Read from stdin")]
        public bool stdin { get; set; }

        [Value(0, MetaName = "offset", HelpText = "File offset.")]
        public long? Offset { get; set; }
    }

    class Program
    {
        static async Task Main(string[] args)
        {
            var parsedArguments = Parser.Default.ParseArguments<Options>(args);

            await parsedArguments.WithParsedAsync(opts => RunOptionsAndReturnExitCodeAsync(opts));
            await parsedArguments.WithNotParsedAsync((errs) => HandleParseErrorAsync(errs));
        }

        static Task<int> RunOptionsAndReturnExitCodeAsync(Options options)
        {
            return Task.FromResult(0);
        }
        static Task HandleParseErrorAsync(IEnumerable<Error> errs)
        {
            return Task.CompletedTask;
        }
    }
}

TBH I feel like async is a virus that keeps infecting every method it touches. I don't know if I would've actually been happy with adding async support when we're not even doing any I/O.

The issue is, at a certain point, you have a sequence of operations that need to happen in order that can't be effectively further destructed into parts.

This opens up the possibility for the underlying library to start doing things async, however, here's my biggest opposition to this...

The commandlineparser library does not use external I/O. We don't even hit the filesystem for testing if an argument represents an existing file.

One could possibly argue that the parsing routines could be done in parallel, asynchronously, and map the results back into a single result, which would be the Options class populated with the parse results from the command line.

That all being said, it looks like a good thing to add, although right now the usage of it is kinda dubious... ;-)

It is indeed a virus that spreads until the moment when you no longer have to call any asynchronous method.

I'm already using it with the modification I've made in a process that runs on Linux micro PC that basically creates a communication server and communicates by serial port with different devices. I think there will be more contexts in which it is really necessary, and more and more.

Does this process work with the multiple verbs and MapResults extension method? (Funny thing, I actually awoke myself thinking about the MapResults method, of how to do this better) So be aware, I'm trying to figure out a better way to invoke the parser and get results or parsing errors in order to report back to the user.

I'm kinda apprehensive on this, but probably unfounded.

manne mentioned this pull request

Jan 29, 2019

I hit the same issue as OP.

Have a look to this discussion in how to use async/await in MapResult without the need to change the Codebase of the library.

       var result = parser.ParseArguments<Options>(args);
       await result.MapResult(async 
               x =>
                     {                       
                         await new Command(x).Execute();
                     },
                 errors => Task.FromResult(0)
          );

In C# 7.1 , you can use it in Task Main().

TBH I feel like async is a virus that keeps infecting every method it touches. I don't know if I would've actually been happy with adding async support when we're not even doing any I/O.

I understand this reply is over two years old at this point, but Implementing async/await into the mappings is practically a necessity for this library to stay relevant with modern C#.

At the time, it may have been somewhat foreign to .NET, reserved for only niche circumstances, or just considered some syntactic sugar, but is now extremely widely used, and one of the languages most powerful features. For any application that deals with web-based technologies, it is pretty much a requirement, not an option. Having an application that pulls data from the web or database, such as a server, web scraper, etc, etc, having the the program being unresponsive during communication is simply not a possibility.

There are indeed some workarounds, but they are a bit ugly, and add extra complexity. I would really love to see this implemented as a core feature in a future release, and thank all the contributors for the fine work they have done.

I stumbled across this issue today. It might be that the library itself does not do I/O. However, the user code processing the options might do I/O and require async. I just had a case like that.

@joseangelmt Thank for the PR.

I will try to work around the problem in my code and continue to use async code at the same time. I hope that the PR will be accepted at some point.

I'm brand new to using this lib, but just wondering if there are any drawbacks to using @moh-hassan 's solution? I just implemented it here and it seems to be working well.

@joseangelmt
What about the use case when only one verb is async and the others are not.
For example, this code can't run and sure generate compilation errors:

    static async Task<int> Main(string[] args)
   {
    return await Parser.Default.ParseArguments<AddOptions, CommitOptions, CloneOptions>(args)
      .MapResultAsync(
        (AddOptions opts) => RunAddAndReturnExitCodeAsync(opts),  //async             
 (CommitOptions opts) => RunCommitAndReturnExitCode(opts), //not async
        (CloneOptions opts) => RunCloneAndReturnExitCode(opts), //not async
        errs => Task.FromResult(1));
}

What is the official way for mix and match async/non async parameters?
What is the return value for MapResultXXX in this case?

@moh-hassan since the passed functions all must return TResult it's implicit that they must all return tasks in an async overload. If the consumer wants to mix and match it's trivial to wrap synchronous methods in a task with Task.FromResult, which is no different to what they'd have to do now when passing a mixture of synchronous and asynchronous methods to the existing MapResult.

That said I'm not sure that MapResultAsync really adds anything - the implementation is trivial (it can be implemented as a single line: return MapResult(result, parsedFunc, notParsedFunc);) and invoking it is no different to invoking the existing generic method, which can already be used with async functions.

@antplant

it can be implemented as a single line: return MapResult(result, parsedFunc, notParsedFunc);

Yes, it can.
So, why not to implement async operations as described in this discussion without the change of the Codebase of the library.

@moh-hassan I agree - in the case of MapResult the existing signature already gives you what you need for async as you can just use a return type of Task. There's no real need for a separate MapResultAsync overload.

The same can't be said for e.g. WithParsed, however, which forces you to use MapResult as a workaround.

WithParsed definitely benefits from an async version.

@joseangelmt ,
I want to merge the PR keeping WithParsedAsync /WithNotParsedAsync and dropping MapResultAsync because the MapResult already support async/await.
Kindly, can you modify the PR and drop MapResultAsync.
It's better if you move all async methods to a separate partial file ParserResultExtensionsAsync.cs
I can do this if you agree.

@joseangelmt ,
I want to merge the PR keeping WithParsedAsync /WithNotParsedAsync and dropping MapResultAsync because the MapResult already support async/await.
Kindly, can you modify the PR and drop MapResultAsync.
It's better if you move all async methods to a separate partial file ParserResultExtensionsAsync.cs
I can do this if you agree.

Today you'll have it.

moh-hassan added a commit that referenced this pull request

Feb 7, 2020

Thanks @joseangelmt for the modified version of PR.
It's merged.
One note, I excluded async in Net40 with #if because it's not supported.

Quick question: how can I try this very recently merged WithParsedAsync/WithNotParsedAsync stuff? Can is simply install a newer version using dotnet add (I'm a bit now to dotnet :-) but having much fun in the process).

You can use the nightly develop nuget package CommandLineParser.2.8.0-beta-125.nupkg on CI Appveyor server.

How To use:

 var result = await Parser.Default.ParseArguments<Options>(args)
                    .WithParsedAsync(RunAsync);
  result.WithNotParsed(HandleErrors); //Or WithNotParsedAsync


 async Task RunAsync(Options options)
            {
			// await ....
	   }

  Task HandleErrorsAsync(IEnumerable<Error> arg)
            {
                //handle errors
            }

@ericnewton76
Can you add MyGet in Appveyor to enable reviewing knightly build packages.
Thanks.

Can the betas/previews be published to nuget?

@jafin

Can the betas/previews be published to nuget?

I'm waiting for the Nuget key update by the site Admin to publish the package.

cc / @ericnewton76
cc / @nemec

@moh-hassan can you send me an email to the address on my profile?