Emit warnings for mistyped options #8937

Closed
charlax opened this Issue Dec 2, 2011 · 17 comments

Projects

None yet

10 participants

Contributor
charlax commented Dec 2, 2011

I mistyped an option and got no error:

$ brew install macvim --overide-system-vim # install macvim successfully without giving any warning
$ # instead of
$ brew install macvim --override-system-vim

I think this can be misleading, there should be at least a warning.

Contributor
adamv commented Dec 2, 2011

Perhaps brew install should warn and quit if an unknown option is given...

Contributor
Sharpie commented Dec 2, 2011

As far as I can tell, this would require a substantial re-write of the install machinery. All formulae and dependencies would have to be gathered up front and their options lists would have to be concatenated and reduced. Only then would we be able to check for mistyped options.

Perhaps there is a clever way around a re-write, but I cannot see one offhand.

Note if Homebrew's tab completion is enabled, it can be used to autofill install options which can save some typos.

Contributor
charlax commented Dec 2, 2011

I really don't know brew, so this is a naive attempt: why not requiring options to be before the package? Like brew install --override-system-vim macvim

It adds constraints for the user, too.

Contributor
Sharpie commented Dec 2, 2011

The root of the problem is that options are defined in the Formula files themselves and the installer processes these files one by one. It starts with the first formula listed on the command line, works its way through any dependencies that formula has, then moves on to the next one.

So, without re-writing most of the installer code, there is no way to look at all the possible options to decide if a user passed something invalid or undefined on the command line.

Contributor
adamv commented Dec 2, 2011

"Homebrew 2"

Contributor
ambs commented Mar 7, 2012

third time I build boost :-/

Contributor
Sharpie commented Mar 7, 2012

Again---suggested recourse is to use tab-completion. It can save you from typos and we can fix bugs in the tab completion script a lot easier than re-writing the install and upgrade routines to detect unused options.

Owner

We can trivially query the options for the entire dependency tree now so this is probably actionable.

Contributor

Nothing has really changed here, dependencies are still expanded immediately before installation begins (this remains the case after my deps branch is applied). It would be simple enough to add a post-expansion hook, but I think checking passed options against the entire dependency tree is wrong anyway. It should only happen against formulae that are explicitly passed to the install command.

Contributor

From my work-in-progress on a new zsh-completion for brew, I know it's quite tricky.
Completion and and checking for misspelled options are tightly related and in the ideal case, the completion function would just delegate the real work to brew options.

Right now, at any given point, I need to get the union of all options from all dependencies for all formulae given to brew install. But then it feels wrong to present this possibly huge list to the user, because I don't know yet, how to mark which option belongs to which formula.

It would be easier to tab-complete if we had a syntax like:
brew install [GLOBALOPTIONS] formula1 [OPTIONS_1] [formula2] [OPTIONS_2] [...]
where the OPTIONS_X belong to formulaX (or it's dependencies). For example:
brew install --verbose graphicsmagic --use-cms python --with-brewed-openssl --with-functions

However, then it's not possible to install a bunch of formula, all taking the same option, e.g.:
brew install vtk ldns ledger libming sigar --python
(This is in practice often not possible, because sometimes it's called --with-python, --use-python, --no-python, --enable-python...)

Another difficulty is that the option --with-functions from the first example belongs to the python-dependency sqlite and it is only valid, if sqlite is not already installed. :-/

Contributor

This is why that invalid option checking should only happen against formulae explicitly listed on the commmand line, and not dependencies. Anything more complex quickly disintegrates into chaos, and for what real gain? How often would such complex behavior be utilized?

IMO the simplest solution is the best here. If the user is installing python, but really wants sqlite to be installed with --with-functions, then they can install sqlite explicitly.

Contributor

I agree with you @jacknagel.

Contributor
LinusU commented Oct 21, 2013

I also agree with @jacknagel. Also, how often does user install more than one package at a time? I know I don't do it but maybe it's more common than I think.

Anyway, in those cases isn't it weird to have command line options apply to every package?! What if I want to install php with imagemagick support and the c++ bindings for imagemagick. brew install imagemagick++ php --with-imagemagick, since I want to build php with imagemagick support, however this will be interpreted by the imagemagick++ formula as "I want to use the imagemagick library from this path: ". This is just one example I could think up from a user reading "php requires the imagemagick c++ binding to be available and to be compiled with --with-imagemagick".

Maybe I'm totally in the wrong here but I would be okay with, even prefer, to not be able to specify any options if specify more than one package to be installed (except maybe for global options such as --use-gcc or --env=std).

If we could decide on this I'd be willing to provide a pull request, I actually found this request since I sorted all open issues by date looking for something to contribute with.

@mxcl is this your call? Who are the repo collabs? Who decides stuff like this? @mikemcquaid @adamv @Sharpie

Contributor

It's easiest to just submit a pull request with your proposed implementation; I doubt there is much to be gained by further discussion here (seeing as the last comment was over a year ago).

Contributor
LinusU commented Oct 21, 2013

Okay, it would be nice just to know if it would be merged (assuming that my code is perfect and follows best practices). I'll start to look around in the source to get my head around how I'd like to implement this 🍺

Contributor
apjanke commented Mar 12, 2014

Would it be reasonable to do warnings or errors on bad options for commands beside install that don't need to pass on arbitrary options? Could help with usability and avoid typo bugs with destructive commands. If nothing else, maybe a note in the brew man page that mistyped options are silently ignored (or passed on). I've been using brew for a while and just now figured out this is probably why some commands have been confusing to me, after seeing weird brew list behavior ( https://gist.github.com/apjanke/9501702 ).

@jacknagel jacknagel removed the help wanted label May 19, 2014
@mdhash mdhash referenced this issue in Homebrew/Outreachy-and-Google-Summer-of-Code Mar 11, 2016
Closed

Emit warnings for mistyped options. #3

Owner

Closing old usability issues in favour of pull requests.

@MikeMcQuaid MikeMcQuaid closed this Aug 9, 2016
@MikeMcQuaid MikeMcQuaid locked and limited conversation to collaborators Aug 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.