ponylang / pony-stable

:horse: A simple dependency manager for the Pony language.
BSD 2-Clause "Simplified" License
134 stars 18 forks source link

replace usage of options with cli #95

Open mfelsche opened 5 years ago

mfelsche commented 5 years ago

and do the necessary refactoring.

Fixes #92

mfelsche commented 5 years ago

there is some flakyness in how errors are printed to stderr, gonna investigate how to fix this.

SeanTAllen commented 5 years ago

@mfelsche should this be "do not merge for now"?

mfelsche commented 5 years ago

This is good to go from my side. Fixed the test issue. Depends on if you want to have the renaming get merged first.

srenatus commented 5 years ago

I've built this locally and took it for a spin.

One change in behaviour I've noticed is when using stable env CMD with a CMD that has parameters. Concretely: with stable from master, I could do

$ stable env ponyc --debug jylis/

with this branch, it fails,

$ stable env ponyc --debug jylis/
Error: unknown long option at: 'debug'

usage: stable [<options>] <command> [<args> ...]
[...]

but passes when using --:

$ stable env -- ponyc --debug jylis/

Now, I'm not saying that this is incorrect -- my expectations aren't violated, putting -- after stable env is the proper thing to do in this case -- it's just a small change in behaviour we might want to make users aware of when releasing this? 😃

mfelsche commented 5 years ago

This change was actually not intended, but it makes total sense to me. Previously it was interpreting a simple call of stable env as calling the env executable, which might not be intended after all. So this explicit separation with -- is an improvement imho.

I just think, I need to update the documentation in that respect. And check if cli catches all edge-cases. Gonna update the PR shortly.

mfelsche commented 5 years ago

@jemc and @SeanTAllen do you have an opinion on the change introduced with this PR? Namely, that now instead of:

stable env ponyc --debug

one has to (and should, even without any flags) write:

stable env -- ponyc --debug

Otherwise the cli machinery is not able to differentiate between flags for stable or stable env and flags for the subcommand. and would interpret the --debug as part of the stable cli definition, which it isnt, and it would thus fail.

While this seems like an accident, i think it is changing the stable cli for good. What do you think?

jemc commented 5 years ago

I'm okay with that change, although it does break all of my Makefiles, and likely will for others as well, so we need to make sure the change is well-advertised.

mfelsche commented 5 years ago

I think it would make sense, to include that change in one go, shortly before we transition the name to tack, so we have a clean cut.

SeanTAllen commented 5 years ago

Discussed at Sync. We will merge this once we renamed to tack

mfelsche commented 4 years ago

Feels weird to approve my own PR. But this is good to go from my side, if it is good for you @SeanTAllen