Unknown CLI options should be a failure instead of silently being ignored

crystal-lang / shards

Dependency manager for the Crystal language

Other

763 stars 100 forks source link

Unknown CLI options should be a failure instead of silently being ignored #480

Open jwoertink opened 3 years ago

jwoertink commented 3 years ago

https://github.com/crystal-lang/shards/blob/c93b628fb5b51b75495feedf7a0f19e3bf4e08c1/src/cli.cr#L66

It turns out, that the flag just has to be there somewhere to work 😂

Probably not a huge deal, but just in case I figured I'd open the issue.

asterite commented 3 years ago

https://github.com/crystal-lang/crystal/pull/9465 :-)

So a bug in Crystal that affects shrads.

jwoertink commented 3 years ago

oh, just noticed as well... If you do write something funky, it borks the formatting:

shards list --tree
Shards installed:
  * lucky (0.26.0)
    * lucky_cli (0.26.0)
      * teeplate (0.8.2)
        * future (0.1.0)

shards list --treefiddy
Shards installed:
* lucky (0.26.0)
* lucky_cli (0.26.0)
* teeplate (0.8.2)
* future (0.1.0)

straight-shoota commented 3 years ago

@asterite No, the Crystal bug is already fixed in 0.36.0.

There is no actual bug here except that shards list doesn't complain about additional, unknown flags. The result of shards list --treeweeeee is exactly the same as shards list (the flag is just silently ignored).

straight-shoota commented 3 years ago

To fix this properly CLI.run needs a major refactoring because currently a number of common options is handled there and thus allowed for all commands. But shards list --skip-postinstall doesn't make any more sense than shards list --treeweeeee 😆

jwoertink commented 3 years ago

but it's so fun to say!! 😆

Yeah, not a big deal. This actually came up because my finger slipped and typed --treed, and I was shocked that it still output something. So I started playing with it to see how far that went.

straight-shoota commented 3 years ago

Yeah, it should definitely fail on invalid options.