Skip to content

Conversation

@AngelMunoz
Copy link
Contributor

This adds support for adding aliases for commands

Let me know if anything looks weird!

Copy link
Owner

@JordanMarr JordanMarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks! It does bring to my attention, however, that there are multiple conventions happening for the add overloads:

  • addCommand and addCommands
  • add and add (to add extra inputs)
  • addAlias and 'addAlias`

I'm not sure why I called it add instead of addInput.
I'm also not sure if it's better to use singular form with overloads or separate singular and plural.

What do you think?

@AngelMunoz
Copy link
Contributor Author

I personally use addCommands [] for my root command but for me that's because I have around 16 subcommands I'm sure I would be using the single addCommand overload in apps with less sub commands.

I think addInput makes more sense than add as it is very generic and can be ambiguous.

For plural vs singular operation names, I guess we could just settle with the singular version and just having the single and seq overloads just to be consistent, although the plural reads nice and I guess it feels natural when reading it in the case of adding multiple things, I favor slightly the singluar names with the required overloads

@JordanMarr JordanMarr merged commit 89b8cb5 into JordanMarr:main Apr 8, 2023
@JordanMarr
Copy link
Owner

JordanMarr commented Apr 8, 2023

Released: https://github.com/JordanMarr/FSharp.SystemCommandLine/releases/tag/v0.17.0-beta4

After some experimenting, I opted to go with the addInput / addInputs and addAlias / addAliases convention because addInput was having issues with the overloads, specifically when I wanted to have a less specific type in the seq overload. Splitting them up into distinctly named operations fixed the issue. Also, Visual Studio IntelliSense doesn't seem to provide a way to toggle through overloads using arrow up/down the way you can with method overloads which makes discoverability a bit more difficult.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants