[timriley]

While working on merging #122 (focusing on options with required: true), it became clear that we needed stricter handling of arguments with required: true — we were allowing required: true arguments to also have a default:, but didn't do anything with that default value.

Arguments are more fragile than options, because they're given positionally, versus being able to be supplied by name and in any order. So we handle conflicting options on arguments more strictly. Now we raise a Dry::CLI::CommandDefinitionError in a few cases:

• If an argument with required: true is defined after one that is not required
• If an argument is defined with both required: true and a default:.

We also fix a bug from when an argument is redefined. If someone e.g. changes an existing argument's description, it was previously moved to the end of this list, and would result in values arriving in an unexpected order. Now we allow the argument to be modified while keeping its place in the sequence.

Finally, now that we've introduced the broader CommandDefinitionError, we swap IncompatibleOptionError to use this one, just to keep things simple :)

[timriley]

Thanks for this quick feedback, @parndt! I think I might actually change tack a bit here. I'm going to address your bug about redefining args (this is important!) but instead of automagically turning an arg with required: and a default: into an optional arg, I'm going to raise an exception instead. For things as fragile as positional arguments, I think it's better we ask the user to get things right.

[parndt]

Declaring an argument with required: true and a default used to load fine, and the default was just ignored. Now the command fails as soon as it's defined, so people with existing commands like that will see it after upgrading. Should we call this out as a breaking change?

[timriley]

Fair point, I'll add this before merging :)