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

commands/subcommands API is awkward #116

Closed
SidShetye opened this issue Jul 3, 2018 · 10 comments
Closed

commands/subcommands API is awkward #116

SidShetye opened this issue Jul 3, 2018 · 10 comments
Labels
closed-stale This issue is closed because it went stale and there was no action planned. It can be reopened. documentation help wanted We would be willing to take a well-written PR to help fix this.

Comments

@SidShetye
Copy link

SidShetye commented Jul 3, 2018

Is your feature request related to a problem? Please describe.

Building a CLI tool that takes has multiple commands (verbs) when processing a given data store.

manage-store add <add params>
manage-store list <list param>
manage-store delete <delete params>

Describe the solution you'd like

I'd like to just define the add, list and delete classes decorated with something like the [Command] (?) attribute. And perhaps a single entrypoint into a library-standardized dispatcher e.g.

public static void Main(string[] args) 
{
    // making up this name but you get the point
    McMaster.Extensions.CommandLineUtils.RouteToCommand(args);
}

Describe alternatives you've considered

The current documentation just feels very awkward. It seems to offload CLI parsing and routing/dispatching back to user written code. Given that the concern is CLI parsing and routing, this should be handled from within the library itself.

On a related note, the documentation is unnecessarily complicated by cramming too many things into a single command/sub-command example.

IMHO, your root level readme.md should cover one quick example for regular CLIs (e.g. wget) and another example for verbs/commands based CLIs (like dotnet <verbs>).

Additional context

Don't really care about the builder API if it matters. Great tool by the way - thanks!

@SidShetye
Copy link
Author

Slightly off-topic but still related: I noticed that you've got help-wanted on several issues and that this is your side project. There is another project at https://github.com/commandlineparser/commandline which does the same job (slightly different API surface) but that's stuck on an older .net standard (=lots of dependencies). Have you guys spoken to each other to consider pooling talent and time?

@natemcmaster
Copy link
Owner

natemcmaster commented Jul 3, 2018

The samples/Subcommands/Program.cs file you pointed to is not a representative sample. I recommend taking a look at these instead and tell me then if you think the API is still awkward.

https://github.com/natemcmaster/CommandLineUtils/blob/master/samples/Subcommands/Inheritance.cs
https://github.com/natemcmaster/CommandLineUtils/blob/master/samples/Subcommands/BuilderApi.cs
https://github.com/natemcmaster/CommandLineUtils/blob/master/samples/Subcommands/NestedTypes.cs

Agree, we should cleanup the samples and docs folder. They've grown organically and separately. Would be good to unify.

@natemcmaster
Copy link
Owner

Re your side note: I've seen https://github.com/commandlineparser/commandline, but there are no plans to unify with them. I'm hoping at some point to unify this library with System.CommandLine, when/if that lands.

@SidShetye
Copy link
Author

SidShetye commented Jul 3, 2018

@natemcmaster seems like you already have this functionality and it's actually quite simple and well designed. Documentation is definitely misleading/confusing about the minimal 'magic' / assumptions needed to get this working.

Program.cs

using McMaster.Extensions.CommandLineUtils;

namespace DataStore
{
    [Command(ThrowOnUnexpectedArgument = false)]
    [Subcommand("add", typeof(AddCmd))]
    [Subcommand("delete", typeof(DeleteCmd))]
    [HelpOption("--help")]
    class Program
    {
        public static void Main(string[] args)
        { 
            CommandLineApplication.Execute<Program>(args);
        }
    }
}

AddCmd.cs (DeleteCmd is similar)

using McMaster.Extensions.CommandLineUtils;
using System;
using System.ComponentModel.DataAnnotations;

namespace DataStore
{
    [HelpOption("--help")]
    public class AddCmd
    {
        [Required]
        [Option("--addParam1", Description = "1st parameter for add")]
        public string AddParam1 { get; set; }

        [Required]
        [Option("--addParam2", Description = "2nd parameter for add")]
        public MyEnum AddParam2 { get; set; }

        public void OnExecute()
        {
            // do work, AddParam1 and AddParam2 (enum auto-parsed) available here
        }
    }
}

Regarding System.Commandline that would be even better. IMHO as long as 'console application' is a supported flavor, this sort of CLI parsing should be right in the .net standard itself. Imagine having ASP.NET as a supported programming model but leaving Controllers or the router entirely as community effort libraries. So your work is very much appreciated!

@natemcmaster
Copy link
Owner

Ok, glad you found what you needed. I'll work on cleaning up docs to make it more discoverable. Of course, I'm happy to take contributions, too.

this sort of CLI parsing should be right in the .net standard itself.

There be dragons, my friend. There is a surprisingly contentious history on this matter.

@jerriep
Copy link
Contributor

jerriep commented Sep 18, 2018

@natemcmaster

I'm hoping at some point to unify this library with System.CommandLine, when/if that lands.

What is System.CommandLine Nate? Anywhere one can find more info?

@ooo
Copy link

ooo bot commented Sep 18, 2018

👋 Hey @jerriep...

Letting you know, @natemcmaster is currently OOO until Monday, September 24th 2018. ❤️

@natemcmaster
Copy link
Owner

@jerriep it's project that's been brewing on the .NET team for a while. I helped earlier this year on some of the early versions. I don't think there is a target date yet for making it a real thing. See dotnet/corefxlab#2245 (comment)

@natemcmaster natemcmaster added this to the 3.0.0 milestone Sep 25, 2018
@jerriep
Copy link
Contributor

jerriep commented Sep 26, 2018

Thanks Nate! I have subscribed to that issue to keep track of the progress.

@natemcmaster natemcmaster added the help wanted We would be willing to take a well-written PR to help fix this. label Jan 6, 2019
@natemcmaster natemcmaster removed this from the 3.0.0 milestone Jan 6, 2019
@stale
Copy link

stale bot commented Apr 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please comment if you believe this should remain open, otherwise it will be closed in 7 days.

@stale stale bot added the closed-wontfix This issue is closed because there are no plans to fix this. label Apr 6, 2019
@stale stale bot closed this as completed Apr 13, 2019
@natemcmaster natemcmaster added closed-stale This issue is closed because it went stale and there was no action planned. It can be reopened. and removed closed-wontfix This issue is closed because there are no plans to fix this. labels May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-stale This issue is closed because it went stale and there was no action planned. It can be reopened. documentation help wanted We would be willing to take a well-written PR to help fix this.
Projects
None yet
Development

No branches or pull requests

3 participants