feat: custom arity for Command.String and Command.Array

[paul-soporan]

Edit: The original description isn't accurate anymore, see https://github.com/arcanis/clipanion/pull/40#issuecomment-694146387 for an updated description.

Just a fun experiment I've played with during the past few hours: Command.Tuple - options accepting a fixed number of arguments.

This PR introduces a new Command.Tuple(optNames: string, {length: number}) decorator, which allows for fixed arity options (other than 0 or 1). This enables use cases such as filter-players Player1 Player2 Player3 --by-position 1 2 3.

It works by modifying commandBuilder.registerOptions to make it work correctly with arities higher than 1 (the same code is used even when the arity is 1).

I've also added tests and updated the documentation.

My only concern about this that I haven't been able to find a solution for: A way to make it also work with the Command.Array decorator (bin --position 1 2 3 --position 4 5 6). I was initially thinking about overloading both the Command.String and Command.Array decorator to allow specifying a custom arity, but Command.String is already overloaded enough and it also wouldn't make sense for a decorator called Command.String to return a tuple. That's why I've decided on Command.Tuple for single occurrences, but I don't know what's the best way to compose both Command.Array and Command.Tuple. I'm really curious about what your ideas are on this.

[arcanis]

Seems cool! Regarding the tuple+array thing, perhaps we could make the syntax something like this?

@Command.Array()
@Command.Tuple(`--position`, {length: 3})

The main issues would be to find a proper way to type Array (although it probably can be done by magically tagging Tuple and adding an Array override), and enforce the right decorator order (ideally in a way that can be understood).

[paul-soporan]

If we are going to compose option decorators like this, wouldn't it make more sense to do it for both Command.String and Command.Tuple (and possibly even for Command.Boolean for consistency)?

We could deprecate Command.Array as it currently is and make it only be a modifier to other option decorators (where it makes sense, so not Command.Counter). This would mean:

class MyCommand extends Command {
  @Command.Array()
  @Command.Tuple(`--position`, {length: 3})
  position: Array<[string, string, string]> = [];

  @Command.Array()
  @Command.String(`--foo`)
  foo: string[] = [];
  // ...
}

[arcanis]

I think I like that, although it would be a semver-major change if we don't support the old syntax. In this case, I think we should make the old signature a wrapper around the "proper one", and mark it as @deprecated so that TS would still report it in the editors. This way we wouldn't have to bump the major.

[paul-soporan]

I've successfully managed to get Command.Array to work as a modifier with some getter and setter magic. Note: Command.Array has to be below the option decorators, because that's how decorator evaluation works (it needs to be called before the option decorators).

This works just as expected now:

class MyCommand extends Command {
  @Command.Tuple(`--position`, {length: 3})
  @Command.Array()
  position: Array<[string, string, string]> = [];

  @Command.String(`--foo`)
  @Command.Array()
  foo: string[] = [];
  // ...
}

I've also added tests and updated the documentation. (I've deprecated the old Command.Array and completely removed it from the readme)

[arcanis]

After giving it another look I think it's better to avoid turning the @Array annotation into a modifier:

• It departs from the current "one decorator per option" model
• It complexifies the implementation
• It adds mental overhead (the order in which it needs to be set)
• It doesn't make sense with all decorators (only String and Tuple)
• The alternative (turning Tuple into an arity option on both String and Array) is pretty simple

I made the changes, and also removed the support for arrays of booleans, since I'm not quite sure I see the use case. If those changes are fine with you, I can merge and release that in the next version bump 😃

[paul-soporan]

Looks good to me! This was my original idea that I've scrapped because I was worried about the complexity of adding an arity option to Command.String. I now realize that it's still much better than Command.Array as a modifier.

Also, thanks for tweaking the syntax! In case you're wondering, I've started working on implementing your other idea (https://github.com/arcanis/clipanion/pull/40#discussion_r485412676) a few days ago, but I didn't get far because this week I've been a bit out of bandwidth 😅.

[arcanis]

Perfect! 🚀