jakeheis / SwiftCLI

A powerful framework for developing CLIs in Swift
MIT License
861 stars 72 forks source link

The reason why I actually wanted to be able to customize Command Signature. #30

Closed Lord-Kamina closed 7 years ago

Lord-Kamina commented 7 years ago

I really like SwiftCLI but there's a couple non-trivial features missing in order to make truly complex CLIs.

1) The ability to group parameters (and options) in groups, and subsequently specify how many members of a group we are expecting (i.e. Exactly one, At least one, At most one, all of them) or alternatively a way to specify that certain parameters or options conflict with each other.

2) The ability to make parameters that also require a value, like keyed options do.

For example, in my use case, before I began looking into the whole changing the Command Signature issue, I ended up using commands with signature "" and all of their functionality implemented through options and flags., since I found that was the only way I could avoid issues caused by mismatching signature parameter counts. Although what I did to aliases would probably greatly offset this issue, it's still not really an elegant solution to use extensively.

jakeheis commented 7 years ago

Could you give an example of 1 and 2? I'm not sure I fully understand what you're describing. Thanks!

Lord-Kamina commented 7 years ago

I wasn't sure myself that was what was missing until I began reading some documentation on generic argument parsing systems as well as for frameworks written in other languages.

In a nutshell, the second is (I realized while working on it today) a convoluted way to describe options that are not actually optional, or rather parameters that are not positional (can always be placed anywhere in the command but always tied to a value)

As for the first, I mean being able to group flags and keyed options in arbitrary groups. We can then define properties on these groups to make them (and their respective options) behave differently. (I got an initial implementation done during the morning but haven't committed it yet)

At the moment, I'm working on implementing:

and

Lord-Kamina commented 7 years ago

I've finished an implementation of the things I was talking about here, as best as I could figure to do it. It's been included in my PR, so you can take a look there.

jakeheis commented 7 years ago

I appreciate your work on this! Just to understand the problem better, are you essentially trying to make something like this built into SwiftCLI?

public class MyCommand: OptionCommand {

    var silent = false
    var loud = false

    public func setupOptions(options: OptionRegistry) {
        options.add(flags: ["-s", "--silently"]) {
            self.silent = true
        }
        options.add(flags: ["-l", "--loudly"]) {
            self.loud = true
        }
    }

    public func execute(arguments: CommandArguments) throws  {
        if silent && loud {
            throw CLIError.error("Can't be silent and loud")
        }
    }

}

If so, I'm not sure this feature is used widely enough to be included in SwiftCLI. Most terminal programs don't disallow certain options together, but rather just have them override each other. For example, here is a section of the manual entry of ln:

     -f    If the target file already exists, then unlink it so that the link may occur.  (The -f option overrides
           any previous -i options.)
     -i    Cause ln to write a prompt to standard error if the target file exists.  If the response from the stan-
           dard input begins with the character `y' or `Y', then unlink the target file so that the link may occur.
           Otherwise, do not attempt the link.  (The -i option overrides any previous -f options.)

As such, I think it would be keeping with more common practices to write this code like:

    enum Volume {
        case normal
        case silent
        case loud
    }

    var volume = Volume.normal

    public func setupOptions(options: OptionRegistry) {
        options.add(flags: ["-s", "--silently"]) {
            self.volume = .silent
        }
        options.add(flags: ["-l", "--loudly"]) {
            self.volume = .loud
        }
    }

If I'm completely misunderstanding your intention, please correct me.

Lord-Kamina commented 7 years ago

Yes and no.

My primary intentions were:

  1. Being able to use position-agnostic parameters.
  2. Being able to treat related keys or flags (or combinations of both) as distinct groups.

Indeed, the example you posted would be terrible design, and while what I did would support something like that, it was definitely my reason for doing it.

Although, mind, that example might look something like this in my fork (which BTW I consider done for now, excepting bugs I might find and correct eventually):

public class MyCommand: OptionCommand {

    var verbosity: Int = 2

    public func setupOptions(options: OptionRegistry) {
        options.addGroup(name:"verbosity",required:true,conflicting:true)
        options.add(flags: ["-s", "--silently"], group:"verbosity") {
            self.verbosity = 1
        }
        options.add(flags: ["-informative", "--informative"], group:"verbosity") {
            self.verbosity = 5
        }
        options.add(keys: ["-v", "--verbosity"], usage: "Sets the verbosity level to <value>", valueSignature: "value", group:"verbosity") { [unowned self] (value) in
    self.verbosity = value
}
    }

    public func execute(arguments: CommandArguments) throws  {

        // Do Things. We don't need to worry about incompatible options here, the default parser will handle it and print the appropriate error message.
}

What would happen now would be that SwiftCLI would treat those options (two flags and one key) as a single required entity and thus would not even get to the execute part unless all conditions are satisfied:

  1. One (and only one) of the three is present.
  2. In case of "verbosity" we must supply an appropriate value.

The "conflicting" feature is mostly there to provide a safeguard against unpredictability caused by the order of the factors altering the product. The real beef of the issue is the ability to group keys and flags together and treat them as required parameters without having to worry about their position in the arguments string.

jakeheis commented 7 years ago

As a whole, options are almost always used as optional things, and options which change the same behavior almost always override earlier options rather than throwing an error. As such, I'm not sure it makes sense to include this specific functionality in SwiftCLI itself. It can be implemented easily enough:

enum Verbosity: Int {
        case none = -1
        case silent = 1
        case errors = 2
        case some = 3
        case most = 4
        case informative = 5
    }

    var verbosity: Verbosity = .none

    public func setupOptions(options: OptionRegistry) {
        options.add(flags: ["-s", "--silently"]) {
            self.verbosity = .silent
        }
        options.add(flags: ["-informative", "--informative"]) {
            self.verbosity = .informative
        }
        options.add(keys: ["-v", "--verbosity"], usage: "Sets the verbosity level to <value>") { (value) in
            self.verbosity = Verbosity(rawValue: Int(value) ?? -1) ?? .none
        }
    }

    public func execute(arguments: CommandArguments) throws  {
        if verbosity == .none {
            throw CLIError.error("Can't be silent and loud")
        }
    }

or if you didn't want overriding options:

    enum Verbosity: Int {
        case error = -2
        case none = -1
        case silent = 1
        case errors = 2
        case some = 3
        case most = 4
        case informative = 5
    }

    var verbosity: Verbosity = .none

    public func setupOptions(options: OptionRegistry) {
        options.add(flags: ["-s", "--silently"]) {
            self.updateVerbosity(.silent)
        }
        options.add(flags: ["-informative", "--informative"]) {
            self.updateVerbosity(.informative)
        }
        options.add(keys: ["-v", "--verbosity"], usage: "Sets the verbosity level to <value>") { (value) in
            self.updateVerbosity(Verbosity(rawValue: Int(value) ?? -1) ?? .none)
        }
    }

    func updateVerbosity(_ val: Verbosity) {
        verbosity = (verbosity == .none ? val : .error)
    }

    public func execute(arguments: CommandArguments) throws  {
        if verbosity == .none || verbosity == .error {
            throw CLIError.error("Error")
        }
    }

Since this functionality goes against general convention yet SwiftCLI still allows for this sort of customization if it's desired, I don't think it makes sense to change substantial sections of SwiftCLI in order to accommodate this feature.

Lord-Kamina commented 7 years ago

BTW, I made a typo in my comment before,

Indeed, the example you posted would be terrible design, and while what I did would support something like that, it was definitely NOT my reason for doing it.

Basically, what I meant to say is your example is just scratching the surface of what I intended to be able to do.

Stop thinking of options strictly as options for a second. What I did was add a completely new type of parameter to SwiftCLI; and I implemented it in such a way that options as they are now are a subset of this kind of parameter.

Now, you can make a CLI exactly as you would have done before and the current behavior will not be modified. (i.e. by default OptionCommands will have an OptionGroup called "options" that is neither required nor conflicting, and all options will be assigned to that group unless specified otherwise)

But, users are now also provided with a simple and straight-forward way to implement more complex logic in their OptionCommands.

I was sure I'd included a link to where I got the idea but perhaps I forgot. I was actually looking for alternatives to SwiftCLI, hoping to find something that might a) allow me to do what I wanted out-of-the-box, or b) Give me an insight on how exactly I might make a command signature to satisfy what I needed. I couldn't really find anything suitable to my needs, but I did find this Commandline parser written in C#, and that's where I got the idea (although the logic they implement is MUCH more complex)

https://www.codeproject.com/Articles/19869/Powerful-and-simple-command-line-parsing-in-C#x1-180004.1.2.3

jakeheis commented 7 years ago

Ok I understand better what you're getting at with this. Could you provide a non-toy example of when this would be needed? I'm having a hard time thinking of times when an option should be required. By convention, required parameters are positional and optional parameters are named, and by convention conflicting options override each other. While I understand that the functionality you have implemented allows for new possibilities, I think those possibilities go against convention and I'm not sure they have much real utility. A non-toy, non-niche example might persuade me otherwise though, as I'm not set in this line of thinking.

Lord-Kamina commented 7 years ago

When you have a program where commands might have sub-commands for example; or, when you don't want to implement several commands that end up being virtually the same.

Now, what I don't get is this... the so-called "convention" is flaky at best; the fact of the matter is command-line conventions are about as standard as CSS used to be back in the IE7 days. Being in swift, the primary targets for SwiftCLI would be macOS and probably Linux. Yes? The closest thing to a convention available (to the best of my knowledge) are the POSIX guidelines and the UNIX conventions to which SwiftCLI doesn't really adhere anyway. Honestly, I'd be hard pressed to think of more than a few examples of utilities that use positional parameters as they're implemented in SwiftCLI. In most cases, they are just there to specify input files.

Furthermore, it's very common to see shell scripts developed in python; ruby, or other languages, most of which have one or more argument parsers, several of which implement features such as I did.

IMO, building a powerful and customizable CLI module, and then purposely leaving out features that are implemented in several other similar modules across many different languages is kind-of self-defeating.

jakeheis commented 7 years ago

I'll ask again for a non-toy example of when this would be needed. You have not provided any examples of when this kind of feature would be helpful or intuitive to a user.

SwiftCLI certainly was designed with the POSIX guidelines in mind, but you're right that it does not adhere to those guidelines rigorously. A few popular utilities that use positional parameters as they are implemented in SwfitCLI: git, rails, brew. If you could point me to some examples of CLIs which use arguments as you're proposing here, I think that would help me to see the value in them.

I'd argue that simply adopting a feature because other modules do is a silly rationale and a great way to welcome feature-creep. I'm open to this idea, but you have as-yet been unable to give an example of when this functionality would be useful to the user and not counter-intuitive given how other CLIs function.

Lord-Kamina commented 7 years ago

Okay, let's try this again...

1) Most commands do not actually use positional arguments except to specify lists of files. The exception being commands (such as in git).

2) While it's true that many applications can be implemented in just the way you describe, not all of them can and certainly not all of them are. I insist on the fact that the (or at least my idea, for what it's worth) of making a framework is to not arbitrarily push design styles on the user (in this case the user is actually the programmer of the CLI utility, mind you). What this boils down to, for me, is that (you might disagree) when you are making a tool other people will use to build things with, less is not really more.

3) My implementation could certainly be improved, I think, by adding additional logic to the way what I already implemented is handled (like that link I shared; i.e. not only being able to XOR option groups)

4) You must remember that in the end, CLIs are exposed to user-input, and that can often be almost literally anything, not even taking into account simple typos or errors. The way your commands are implemented, built around required/optional positional parameters as the primary component, flexibility of design is very reduced (even if someone wants to work with just options, as many CLIs do, you often will have to trade this for a decent usage-message.), and even then you will often end-up running into the "Too many/too few arguments" errors.

If you really want an example of a popular command that implements similar logic, look no further than tar:

[Koji@Anasas:~/test] $ tar -xf ./foo.tar -cf baz.tar
tar: Can't specify both -c and -x
jakeheis commented 7 years ago

Ok thanks for the example, I'm seeing the value in it now. I'm improving how options are dealt with in SwiftCLI significantly in the upcoming update (3.0), so while I can't accept your pull request, I will be implementing this feature. Check out https://github.com/jakeheis/SwiftCLI/blob/opt_groups/Sources/OptionRegistry.swift#L74 to see the WIP version, and feel free to open more issues if it fails to do something you're wishing it did.

Thanks for taking the time to talk this feature through with me. I firmly believe every feature should be carefully considered before implementing it, and while I know you have a different opinion on that, I appreciate you taking the time to convince me of the utility of this feature.

Lord-Kamina commented 7 years ago

I haven't really looked thoroughly at it or tested it yet, I'll try upgrading to 3.0 and see but it looks like it's definitely a more complete implementation than what I did.

One thing I do encourage you to look at again and implement of my PR, though, is what I did to the usage statement and error message generators. It's mostly a cosmetic issue but there were also cases where your code to calculate padding could lead to crashes because of trying to add a negative number of spaces.

jakeheis commented 7 years ago

Closing this issue, can create a new issue for the message generators if need be