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

Drop arbitrary limit on splat arguments #54

Open
n0la opened this issue Nov 30, 2016 · 5 comments
Open

Drop arbitrary limit on splat arguments #54

n0la opened this issue Nov 30, 2016 · 5 comments

Comments

@n0la
Copy link

n0la commented Nov 30, 2016

What is the deal with a numeric limit on splat() arguments? Why not just specify unlimited by default? If anyone wants to limit it to 1 he could always optionally pass that integer. But a limit between 1 and 999 seems quite arbitrary to me.

@amireh
Copy link
Collaborator

amireh commented Dec 9, 2016

That's a good question. I don't recall the details, so I have to look at the sources again. Is the limit causing an issue for you?

Maybe @Tieske could shed some light if he still remembers as I believe he introduced it in the first place.

@n0la
Copy link
Author

n0la commented Dec 9, 2016

Not directly an issue but it's weird and counter intuitive API behaviour.

If I do a command like:

# takes a list of items and calculates the price of each
myproject calc "item 1" "item 2" "item3"

I expect that:

cli:splat("ITEMS", "list of items for which to calculate the price")
-- and not:
-- cli:splat("ITEMS", "list of items for which to calculate the price", nil, 999)

is enough to allow a list of multiple items. If there were just one item I would not be using splat() in the first place, but rather use option.

@Tieske
Copy link
Member

Tieske commented Dec 19, 2016

I don't recall why, probably has to do with implementation details. First thing that comes to mind is formatting error messages.

But from a functional perspective I don't see why this could not be changed.

@n0la
Copy link
Author

n0la commented Dec 19, 2016

I studied the code, and the maximum amount is also used to determine if enough parameters have been given on the command line; with 0 options being a valid amount.

amireh added a commit that referenced this issue Jan 14, 2017
Unfortunately, this is a breaking change and requires a major release.
The patch makes it so that we no longer reject more than 999 repetitions
for a splat argument, and that by default we allow for unlimited
arguments (maxcount=0 or nil) instead of 1.
@amireh
Copy link
Collaborator

amireh commented Jan 14, 2017

Hey @n0la, I've spent some time going over the splat argument logic and dropped the arbitrary limit of 999 and made the default behaviour to accept an unlimited number of repetitions. Since that's a breaking API change, we must do it in a major release.

Please check it out if you can so that can proceed.

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

No branches or pull requests

3 participants