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

Convention based api #31

Closed
shovonnn opened this issue Dec 23, 2017 · 6 comments
Closed

Convention based api #31

shovonnn opened this issue Dec 23, 2017 · 6 comments
Assignees
Milestone

Comments

@shovonnn
Copy link

i have added an extension method to CommandLineApplication to support convention based command handling. see here https://github.com/shovonnn/artisancommandline

i tried to create a pull request for this repo. but it wont compile. few too many error. maybe cause of netcore1.x
i am on a tight schedule at the moment. so can't work much.

@natemcmaster
Copy link
Owner

I looked briefly at https://github.com/shovonnn/artisancommandline. From what I can see, it looks like there are a bunch of changes that would need to happen to support this. Would be good to nail down what exactly you're looking to support. Based on your sample, it seems like you would need at least the following

  1. small feature: Add app.Use<TCommand>() syntax via extensions #15 - support for mixing attribute binding and the builder API. Not currently supported.
var app = new CommandLineApplication();
app.Use<MyProgram>();
app.Execute(args);
  1. Support for changing the method that is invoked based on subcommand. Right now, the attribute API only invokes a method named "OnExecute" or "OnExecuteAsync".
class Program
{
   [Subcommand("delete")]
   public int OnDelete()
   { }

   [Subcommand("list")]
   public int OnList()
   { }
}
  1. Support for OptionAttribute on method parameters. Currently only properties are supported.
class Program
{
   public int OnExecute([Option("--name")] string name)
   { }
}

maybe cause of netcore1.x

Yup...System.Reflection is a pain in netstandard1.x. Usually you have to drop down use System.Reflection.TypeInfo instead of System.Type.

@natemcmaster
Copy link
Owner

@shovonnn can you provide more feedback on this issue? Did my response summarize well the things you would like added to this library?

@shovonnn
Copy link
Author

yeah it does. sorry i have been very busy last few days.
Basically what i wanted was minimal use of attribute or configurations.
That means By default:

  1. Every public property with some special types(int, float, string, list) is registered as option for the command.

  2. Any method named "Handle" contains execution logic for Main command.

  3. For every method with "Handle" prefix, it registers a subcommand( naming it with kebab case of the suffix)

  4. Parameters to Handle methods registered as arguments for respective command(main command and subcommands). if Type of the parameter is not simple like(int, float, string, list), it calls a special ResolveType method(if it exists), so that we can have service injection capability. look here https://github.com/shovonnn/artisancommandline/blob/master/ArtisanCommandLine/BaseConsoleCommand.cs

  5. if return type of the handle method is simply "int", then we treat it as synchronous command, or else if it is Task<int> treat it as async command.

                var res = methodInfo.Invoke(obj, handlerParams.ToArray());

                if (res.GetType() == typeof(Task<int>))
                {
                    var task = (Task<int>)res;
                    System.Console.CancelKeyPress += (sender, e) =>
                    {
                        cts?.Cancel();
                        Task.WaitAll(new[] { task }, 10000);
                    };
                    AppDomain.CurrentDomain.ProcessExit += (sender, e) =>
                    {
                        cts?.Cancel();
						Task.WaitAll(new[] { task }, 10000);
                    };
                    Task.WaitAll(task);
                    return task.Result;
                }
                else
                    return (int)res;

@natemcmaster
Copy link
Owner

@shovonnn so, some of these conventions are ones that I plan to make default. Plus, some of these conventions would break existing apps if they were made default. That said, it is my intention to make it possible for you to write your own contentions, and let users opt-in to them.

For 2.1.0, my goal was to write what I thought was a basic approach to mapping a CLR type to CommandLineApplication. I kept all this API internal, but in 2.2.0 I'm looking to provide ways for users to tap into this to light up scenarios such as the one you have proposed. I have an initial draft of this API is on the feature/conventions branch.

Here's an early example of what this might look like. For example, you could implement proposal 1 (every public property is an option) like this:

    public class PropertiesAsOptionsConvention : ICommandLineAppConvention
    {
        public void Apply(ConventionCreationContext context)
        {
            foreach (var prop in context.TargetType.GetProperties())
            {
                var option = context.Application.Option(
                    template: "--" + prop.Name,
                    description: prop.Name,
                    optionType: CommandOptionType.SingleValue);

                context.RegisterParsingCompleteAction((instance, parseContext) =>
                {
                    if (option.HasValue())
                    {
                        prop.SetMethod.Invoke(instance, new object[] { option.Value() });
                    }
                });
            }
        }
    }


    [ParsingConvention(typeof(PropertiesAsOptionsConvention))]
    public class Program
    {

    }

The API needs work, so expect some churn here, but let me know what you think.

@natemcmaster
Copy link
Owner

I spent some time refactoring the way attribute binding works. Although I don't plan to implement all of the conventions suggested here, I have made API public that should allow you to implement these kinds of features yourself. Using the new type CommandLineApplication<T>, you should be able to implement IConvention to add your own conventions such as adding subcommands with options and types for methods named "Handle" something. This should be available in 2.2.0

@natemcmaster
Copy link
Owner

By the way, just to prove to myself that this convention API is flexible enough to implement this, I made this a sample.

/// <summary>
/// This custom convention adds a subcommand for each method named "Handle-something"
/// </summary>
public class MethodsAsSubcommandsConventions : IConvention

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants