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

AddDelegateAsync or pass original arguments #766

Closed
WojciechNagorski opened this issue Mar 9, 2022 · 3 comments · Fixed by #1215
Closed

AddDelegateAsync or pass original arguments #766

WojciechNagorski opened this issue Mar 9, 2022 · 3 comments · Fixed by #1215
Labels

Comments

@WojciechNagorski
Copy link

Is your feature request related to a problem? Please describe.
I would like to run ASP Core application in one of the command:

public static async Task Main(string[] args)
{
    var app = new CommandApp<RunCommand>();
    
    app.Configure(config =>
        {
            config.AddCommand<RunCommand>("db-migrate")
                .WithDescription("Only starts database migrations and exits the application.");
            config.AddCommand<RunCommand>("run")
                .WithDescription("Runs ASP core application");
        });
    await app.RunAsync(args);
}

But in this way, I'm not able to pass original args to my host:

public class RunCommand : AsyncCommand
{
    public override async Task<int> ExecuteAsync(CommandContext context)
    {
        var host = Host.CreateDefaultBuilder(args) // <-- here I need the program arguments, but context contains only parsed list
                .ConfigureWebHostDefaults(webBuilder =>
                    {
                        webBuilder.UseStartup<Startup>();
                    }).Build();

        await host.RunAsync();

        return 0;
    }
}

So I tried use the AddDelegate method instead of AddCommand, but this method can't be async.

public static async Task Main(string[] args)
{
    var app = new CommandApp<RunCommand>();

    app.Configure(config =>
        {
            config.AddCommand<RunCommand>("db-migrate")
                .WithDescription("Only starts database migrations and exits the application.");
            config.AddDelegate("run", context => RunApplication(args)) //<-- the RunApplication method can'd be async
                .WithDescription("Runs ASP core application");
        });

    await app.RunAsync(args);
}

public static async Task RunApplication(string[] args)
{
    var host = Host.CreateDefaultBuilder(args) // <-- here I need the program arguments, but context contains only parsed list
                .ConfigureWebHostDefaults(webBuilder =>
                    {
                        webBuilder.UseStartup<Startup>();
                    }).Build();

    await host.RunAsync();
}

Describe the solution you'd like
This problem can be solved in two ways:

  1. Add original parameters to the command context:
public class RunCommand : AsyncCommand
{
    public override async Task<int> ExecuteAsync(CommandContext context)
    {
        var args = context.OriginalArguments //<-- contains args from the main method

        return 0;
    }
}
  1. Support async deleget AddDelegateAsync:
public static async Task Main(string[] args)
{
    var app = new CommandApp<RunCommand>();

    app.Configure(config =>
        {
            config.AddDelegateAsync("run", async context => await RunApplication(args)) //<-- the new AddDelegateAsync method
        });

    await app.RunAsync(args);
}
@WojciechNagorski
Copy link
Author

I can prepare PR with the solution, but first I need to know which solution do you prefer.

@WojciechNagorski WojciechNagorski changed the title AddDelegateAsync AddDelegateAsync or pass original arguments Mar 9, 2022
@patriksvensson patriksvensson moved this to Todo 🕑 in Spectre Console Oct 4, 2022
@icalvo
Copy link
Contributor

icalvo commented Apr 14, 2023

I think that adding an async delegate is more generic. I came here because I need that as a way to constructing an async command by myself (fiddling with the typeregistrar to build it your way is much more complicated).

I would say, though, that ending the method name with Async can be confusing since the method itself is not async, I'd rather name it AddAsyncDelegate (we are not adding a delegate asynchronously, but adding an asynchronous delegate).

icalvo added a commit to icalvo/spectre.console that referenced this issue Apr 14, 2023
@FrankRay78
Copy link
Contributor

Hello @WojciechNagorski, I'm going to merge @icalvo's linked PR shortly which will provide you with solution 2.

However, I thought you might be interested to see the following example which passes data to an executing command through the CommandContext:

using Spectre.Console;
using Spectre.Console.Cli;

public class Program
{
    public static async Task Main(string[] args)
    {
        var app = new CommandApp();

        app.SetDefaultCommand<AsynchronousCommand>()
            .WithData(new string[] { "a", "b" });

        app.Configure(config =>
        {
            config.PropagateExceptions();
        });

        await app.RunAsync(args);
    }
}

public sealed class AsynchronousCommand : AsyncCommand
{
    private readonly IAnsiConsole _console;

    public AsynchronousCommand(IAnsiConsole console)
    {
        _console = console;
    }

    public async override Task<int> ExecuteAsync(CommandContext context)
    {
        _console.WriteLine($"AsynchronousCommand.ExecuteAsync");
        _console.WriteLine($"- context.Data: {string.Join(", ", ((string[])context.Data))}");

        return 0;
    }
}

Result:
image

FrankRay78 pushed a commit that referenced this issue May 25, 2023
@github-project-automation github-project-automation bot moved this from Todo 🕑 to Done 🚀 in Spectre Console May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done 🚀
Development

Successfully merging a pull request may close this issue.

4 participants