Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

[RFC] Improve handling of --verbose/-v argument #46387

Closed
UniqMartin opened this issue Nov 26, 2015 · 21 comments
Closed

[RFC] Improve handling of --verbose/-v argument #46387

UniqMartin opened this issue Nov 26, 2015 · 21 comments

Comments

@UniqMartin
Copy link
Contributor

Currently, the handling of the --verbose argument and its short version -v is pretty inconsistent and gives a horrible user experience. Compare these outputs (the list command is just an example):

Short Form -v

$ brew list -v colordiff
/opt/homebrew/Cellar/colordiff/1.0.16/bin/cdiff
/opt/homebrew/Cellar/colordiff/1.0.16/bin/colordiff
/opt/homebrew/Cellar/colordiff/1.0.16/CHANGES
/opt/homebrew/Cellar/colordiff/1.0.16/COPYING
/opt/homebrew/Cellar/colordiff/1.0.16/INSTALL_RECEIPT.json
/opt/homebrew/Cellar/colordiff/1.0.16/README
/opt/homebrew/Cellar/colordiff/1.0.16/share/man/man1/cdiff.1
/opt/homebrew/Cellar/colordiff/1.0.16/share/man/man1/colordiff.1
$ brew -v list colordiff
Homebrew 0.9.5 (git revision 5979e; last commit 2015-11-24)
/opt/homebrew/Cellar/colordiff/1.0.16/bin/cdiff
/opt/homebrew/Cellar/colordiff/1.0.16/bin/colordiff
/opt/homebrew/Cellar/colordiff/1.0.16/CHANGES
/opt/homebrew/Cellar/colordiff/1.0.16/COPYING
/opt/homebrew/Cellar/colordiff/1.0.16/INSTALL_RECEIPT.json
/opt/homebrew/Cellar/colordiff/1.0.16/README
/opt/homebrew/Cellar/colordiff/1.0.16/share/man/man1/cdiff.1
/opt/homebrew/Cellar/colordiff/1.0.16/share/man/man1/colordiff.1

Notice how the header Homebrew 0.9.5 (git revision 5979e; last commit 2015-11-24) is either printed or not depending on the position of the -v option. In both cases, it is interpreted to mean “verbose”.

Long Form --verbose

$ brew list --verbose colordiff
/opt/homebrew/Cellar/colordiff/1.0.16/bin/cdiff
/opt/homebrew/Cellar/colordiff/1.0.16/bin/colordiff
/opt/homebrew/Cellar/colordiff/1.0.16/CHANGES
/opt/homebrew/Cellar/colordiff/1.0.16/COPYING
/opt/homebrew/Cellar/colordiff/1.0.16/INSTALL_RECEIPT.json
/opt/homebrew/Cellar/colordiff/1.0.16/README
/opt/homebrew/Cellar/colordiff/1.0.16/share/man/man1/cdiff.1
/opt/homebrew/Cellar/colordiff/1.0.16/share/man/man1/colordiff.1
$ brew --verbose list colordiff
Error: Unknown command: --verbose

Suddenly, --verbose is recognized as a command and thus not accepted in the same place where its short counterpart is (so it isn't exactly equivalent).

Other Related Cases

$ brew -v
Homebrew 0.9.5 (git revision 5979e; last commit 2015-11-24)
$ brew --version
0.9.5 (git revision 5979e; last commit 2015-11-24)

Without any other arguments -v is just an odd variation of --version with Homebrew prefixed.

Suggestion

Passing -v as the first argument is undocumented, as is its special handling and the additional header it prints. Passing -v as the sole argument doesn't seem very useful given that --version also exists (and is documented). If I'm not the only one who feels that way, I volunteer to clean up brew.rb accordingly.

@MikeMcQuaid
Copy link
Member

Agreed this makes sense to make more consistent 👍

@bfontaine
Copy link
Contributor

Passing -v as the sole argument doesn't seem very useful given that --version also exists (and is documented).

I think it’s fine to keep it for the convention. A lot of programs print their version with both -v and --version.

@UniqMartin
Copy link
Contributor Author

A lot of programs print their version with both -v and --version.

My subjective impression was that there are more programs that interpret -v (lowercase) as verbose and print their version with --version and sometimes also with -V (uppercase). Or am I missing your point? Alternatively, we could keep -v and make it identical to --version. In this case we shouldn't accept any other arguments and exit immediately. Do you agree?

But in this case, I still firmly believe that clarity (-v shouldn't mean two different things in two different positions of the argument list) trumps convenience. I wouldn't want to document those two meanings of -v and not documenting it seems a bit like admitting that there's something “wrong” about it.

@MikeMcQuaid
Copy link
Member

(-v shouldn't mean two different things in two different positions of the argument list) trumps convenience.

Unfortunately both are trumped by "there will be a lot of scripts in the wild that expect this behaviour". The rest of the proposed changes I agree with though 👍

@bfontaine
Copy link
Contributor

My subjective impression was that there are more programs that interpret -v (lowercase) as verbose and print their version with --version and sometimes also with -V (uppercase).

I was under the impression than <command> -v usually was the equivalent of --version but I tried with a few commands (vim, emacs, git, curl, wget) and it didn’t work (it did work with gcc, ruby, perl).

(-v shouldn't mean two different things in two different positions of the argument list)

One could argue that in brew -v foo, -v is an option for brew (= a global option); while in brew foo -v, v is an option for foo (= a subcommand option).

In this case we shouldn't accept any other arguments and exit immediately. Do you agree?

Yep.

@UniqMartin
Copy link
Contributor Author

Scripts written against undocumented flags deserve to be broken. We shouldn't deliberately break things, but we also shouldn't shy away from it because it could inconvenience a few people, if it improves the situation. There's no progress without change and with change comes occasional breakage.

Ultimately, the decision is of course with the people who will most likely have to deal with the consequences, i.e. long-term contributors/maintainers. Just wanted to disclose my thought process.

@UniqMartin
Copy link
Contributor Author

One could argue that in brew -v foo, -v is an option for brew (= a global option); while in brew foo -v, -v is an option for foo (= a subcommand option).

That would be a reasonable thing to expect, but that's not how things stand currently. By that measure, at least -d, --debug, and --verbose also deserve to be recognized before the subcommand, but that's not the case. Most of the time, subcommands don't care at all about the order of their arguments, but the dispatcher in brew.rb assumes the first argument to be the subcommand, with -v being the only exception (and thus treated specially) that I'm aware of.

@MikeMcQuaid
Copy link
Member

Sure. We've told people for years the command-line is our API so we try to be fairly careful about breaking it. I believe another original motivation for this was that Ruby treats the -v flag somewhat similarly.

@bfontaine
Copy link
Contributor

I think we should at least fix the brew -v list colordiff issue; either by not printing the version (thus assuming -v = verbose) or printing only the version.

@UniqMartin
Copy link
Contributor Author

I believe another original motivation for this was that Ruby treats the -v flag somewhat similarly.

Thanks for pointing that out! I don't necessarily think this is good, but it pretty much explains the current behavior with all its quirks. In that case I retreat and accept this design decision, unless other maintainers have strong opinions one way or the other.

I think we should at least fix the brew -v list colordiff issue; either by not printing the version (thus assuming -v = verbose) or printing only the version.

Even this is perfectly consistent with Ruby's interpretation of that switch, if I understand it correctly. It didn't feel right to me, but I also wasn't aware of where that came from.


To quote from Ruby's manual:

-v: Enables verbose mode. Ruby will print its version at the beginning and set the variable $VERBOSE to true. Some methods print extra messages if this variable is true. If this switch is given, and no other switches are present, Ruby quits after printing its version.

@MikeMcQuaid
Copy link
Member

I think we should at least fix the brew -v list colordiff issue; either by not printing the version (thus assuming -v = verbose) or printing only the version.

Agreed. It should be assumed to be verbose unless the actual argument.

@MikeMcQuaid
Copy link
Member

Ruby will print its version at the beginning and set the variable $VERBOSE to true. Some methods print extra messages if this variable is true. If this switch is given, and no other switches are present, Ruby quits after printing its version.

So, that's the behaviour we're emulating. I do think the "will print its version at the beginning" can be killed but the "no other switches" should remain the same.

@UniqMartin
Copy link
Contributor Author

Addendum: The only thing inconsistent about the current implementation is that something like brew list colordiff -v should also print the version number, and not only when -v appears as the first argument. (But that's mostly nitpicking.)

@UniqMartin
Copy link
Contributor Author

To summarize, the current proposal (supported at least by Mike) would be:

  • brew --version continues to print the version line and then exits:
    • Output: 0.9.5 (git revision 5979e; last commit 2015-11-24)
  • brew -v continues to print the version line and then exits:
    • Output: Homebrew 0.9.5 (git revision 5979e; last commit 2015-11-24)
  • brew -v <arguments> continues to be interpreted as brew <arguments> -v, but no longer prints the version line at the very top of the output. No other switch gets this special treatment of being accepted as the first argument (no change here).

Did I get that right? And do we continue to generate slightly different output for -v and --version for the sake of script compatibility?

@MikeMcQuaid
Copy link
Member

And do we continue to generate slightly different output for -v and --version for the sake of script compatibility?

I think we can unify their output. That seems like a mistake rather than intentional. Otherwise 👍

@UniqMartin
Copy link
Contributor Author

I think we can unify their output.

To which one? I'm leaning towards the Ruby precedent of always prefixing the project name.

@MikeMcQuaid
Copy link
Member

Yeh, prefixing project name works for me 👍

@UniqMartin
Copy link
Contributor Author

Cool. 😎 Let's give other maintainers (or even interested users) some time to voice their opinion on this matter and later on I'll prepare a PR.

@bfontaine
Copy link
Contributor

👍 on this.

@apjanke
Copy link
Contributor

apjanke commented Dec 1, 2015

👍 from me too.

@UniqMartin
Copy link
Contributor Author

Please see PR #46790 for the code resulting from this discussion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants