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

Commit

Permalink
Ensure that equals options get parsed properly
Browse files Browse the repository at this point in the history
Options such as --userimg=<path> should be parsed as an option with an
equals in its name ("userimg=") and without the path argument in
Option.name

Closes #34219.

Signed-off-by: Jack Nagel <jacknagel@gmail.com>
  • Loading branch information
staticfloat authored and jacknagel committed Nov 15, 2014
1 parent 4b150ca commit d21ccee
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion Library/Homebrew/options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class Options
include Enumerable

def self.create(array)
new array.map { |e| Option.new(e[/^--(.+)$/, 1] || e) }
new array.map { |e| Option.new(e[/^--([^=]+=?)(.+)?$/, 1] || e) }
end

def initialize(*args)
Expand Down

5 comments on commit d21ccee

@sjackman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staticfloat @jacknagel This changes breaks homebrew/science/abyss. How does one fetch the argument of an option?

@staticfloat
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @sjackman. You seem to be declaring multiple static options with set parameters. That is, you're declaring an --enable-maxk=32 option, and an --enable-maxk=64 option, etc... All of these options are functionally independent and aren't really how options with an = at the end of the name typically work, where the user can provide freeform input after the equals sign.

To give an example, in my Julia formula, I first declare an option that ends in =, then retrieve the value and use it in my formula.

@sjackman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the Julia example, Elliot. The abyss formula was written before options supported arguments. I'll fix it up.

@sjackman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including the argument explicitly in the option to make the brew options text more helpful does not work (see N in the example below). @staticfloat Would you be interested in adding this feature?

  option "enable-maxk=N", "set the maximum k-mer length to N"

@staticfloat
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjackman, yes I think it would be worthwhile to ignore anything after the =. I've opened a PR to do just that, and clean up a bit of the option name parsing in the process.

Please sign in to comment.