search

crystal-lang / shards

Dependency manager for the Crystal language
Other
758 stars 99 forks source link

Forward unmodified ARGV to subcommand #631

Open luislavena opened 1 month ago

luislavena commented 1 month ago

Hello, this PR aims to forward all arguments supplied to shards to the subcommand, if found.

It does this to allow correct parsing of all ARGV supplied, including SHARDS_OPTS that might be appended, avoids altering the original ARGV when combining it.

Introduces a naive integration test for subcommand to validate the change works correctly.

Last but not least, allows also forwarding --help by avoiding it to short-circuit and return immediately by setting a flag for it and evaluating at the end of the processing of unknown options.

This is only done for the CLI invocation and is not part of Shards module (as the help and usage options are only available in this context).

I'm not sure about the implementation as this was the first integration test for subcommands, so would appreciate some notes on additional changes.

Thank you.

Fixes #600

ysbaddaden commented 1 month ago

I like how Git, for example, distinguishes between general and command arguments: those before the command name are general git arguments, while those after the command name will all be forwarded to the actual command.

Applied to Shards:

luislavena commented 1 month ago

@ysbaddaden I like that too, even it mentions git help -a to list all the available subcommands since the default --help lists only some of the core (porcelain) ones (like init, merge, commit, branch, etc).

Off-topic: I found OptionParser quite limited on that, but from my personal point of view, most of the other shards out there are too overwhelming (aka: everything but the kitchen sink).

Unsure if I should back off from the changes in commit 5a6dc6c and leave it for Shards main program to respond to --help, that way might avoid confusion.

Open to suggestions!

Thank you for looking into it! ❤️ ❤️ ❤️

luislavena commented 1 month ago

@straight-shoota is there other feedback or changes necessary? Would be possible for you to highlight any other needed adjustments (outside of the class_property change indicated above) so I can consolidate all my changes at once?

Thank you.

straight-shoota commented 1 month ago

I don't have any further comments at this point. You could wait for feedback from others, if you want.