Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Refactor option handling internals

Currently we handle options in several ways, and it is hard to remember
what code needs an option string ("--foo"), what needs only the name
("foo") and what needs an Option object.

Now that Option objects can act as strings and be converted to JSON, we
can start using them instead of passing around strings between Formula
objects, Tab objects, and ARGV-style arrays.

The Options class is a special collection that can be queried for the
inclusion of options in any form: '--foo', 'foo', or Option.new("foo").
  • Loading branch information...
commit 8f5ea8eea6dfb3da758417e2c9bdda7d2a169408 1 parent 91f15da
@jacknagel authored
View
2  Library/Homebrew/formula_installer.rb
@@ -208,7 +208,7 @@ def build
ENV['HOMEBREW_ERROR_PIPE'] = write.to_i.to_s
args = ARGV.clone
- args.concat tab.used_options unless tab.nil? or args.include? '--fresh'
+ args.concat(tab.used_options.as_flags) unless tab.nil? or args.include? '--fresh'
# FIXME: enforce the download of the non-bottled package
# in the spawned Ruby process.
args << '--build-from-source'
View
48 Library/Homebrew/formula_support.rb
@@ -1,6 +1,7 @@
require 'download_strategy'
require 'checksums'
require 'version'
+require 'options'
class SoftwareSpec
attr_reader :checksum, :mirrors, :specs
@@ -161,36 +162,6 @@ def to_s
end
end
-
-# Represents a build-time option for a formula
-class Option
- attr_reader :name, :description, :flag
-
- def initialize name, description=nil
- @name = name.to_s
- @description = description.to_s
- @flag = '--'+name.to_s
- end
-
- def to_s
- flag
- end
- alias_method :to_str, :to_s
-
- def to_json
- flag.inspect
- end
-
- def eql?(other)
- @name == other.name
- end
-
- def hash
- @name.hash
- end
-end
-
-
# This class holds the build-time options defined for a Formula,
# and provides named access to those options during install.
class BuildOptions
@@ -198,11 +169,8 @@ class BuildOptions
include Enumerable
def initialize args
- # Take a copy of the args (any string array, actually)
- @args = Array.new(args)
- # Extend it into an ARGV extension
- @args.extend(HomebrewArgvExtension)
- @options = Set.new
+ @args = Array.new(args).extend(HomebrewArgvExtension)
+ @options = Options.new
end
def add name, description=nil
@@ -222,12 +190,12 @@ def empty?
@options.empty?
end
- def each(&blk)
- @options.each(&blk)
+ def each(*args, &block)
+ @options.each(*args, &block)
end
def as_flags
- map { |opt| opt.flag }
+ @options.as_flags
end
def include? name
@@ -259,10 +227,10 @@ def build_32_bit?
end
def used_options
- as_flags & @args.options_only
+ Options.new((as_flags & @args.options_only).map { |o| Option.new(o) })
end
def unused_options
- as_flags - @args.options_only
+ Options.new((as_flags - @args.options_only).map { |o| Option.new(o) })
end
end
View
78 Library/Homebrew/options.rb
@@ -0,0 +1,78 @@
+class Option
+ include Comparable
+
+ attr_reader :name, :description, :flag
+
+ def initialize(name, description=nil)
+ @name = name.to_s[/^(?:--)?(.+)$/, 1]
+ @description = description.to_s
+ @flag = "--#{@name}"
+ end
+
+ def to_s
+ flag
+ end
+ alias_method :to_str, :to_s
+
+ def to_json
+ flag.inspect
+ end
+
+ def <=>(other)
+ name <=> other.name
+ end
+
+ def eql?(other)
+ other.is_a?(self.class) && hash == other.hash
+ end
+
+ def hash
+ name.hash
+ end
+end
+
+class Options
+ include Enumerable
+
+ def initialize(*args)
+ @options = Set.new(*args)
+ end
+
+ def each(*args, &block)
+ @options.each(*args, &block)
+ end
+
+ def <<(o)
+ @options << o
+ self
+ end
+
+ def +(o)
+ Options.new(@options + o)
+ end
+
+ def -(o)
+ Options.new(@options - o)
+ end
+
+ def *(arg)
+ @options.to_a * arg
+ end
+
+ def empty?
+ @options.empty?
+ end
+
+ def as_flags
+ map(&:flag)
+ end
+
+ def include?(o)
+ any? { |opt| opt == o || opt.name == o || opt.flag == o }
+ end
+
+ def to_a
+ @options.to_a
+ end
+ alias_method :to_ary, :to_a
+end
View
13 Library/Homebrew/tab.rb
@@ -1,5 +1,6 @@
require 'ostruct'
require 'formula'
+require 'options'
require 'vendor/multi_json'
# Inherit from OpenStruct to gain a generic initialization method that takes a
@@ -61,14 +62,22 @@ def installed_with? opt
used_options.include? opt
end
+ def used_options
+ Options.new(super.map { |o| Option.new(o) })
+ end
+
+ def unused_options
+ Options.new(super.map { |o| Option.new(o) })
+ end
+
def options
used_options + unused_options
end
def to_json
MultiJson.encode({
- :used_options => used_options,
- :unused_options => unused_options,
+ :used_options => used_options.to_a,
+ :unused_options => unused_options.to_a,
:built_as_bottle => built_as_bottle,
:tapped_from => tapped_from,
:time => time,
View
90 Library/Homebrew/test/test_options.rb
@@ -0,0 +1,90 @@
+require 'testing_env'
+require 'options'
+
+class OptionTests < Test::Unit::TestCase
+ def setup
+ @option = Option.new("foo")
+ end
+
+ def test_to_s
+ assert_equal "--foo", @option.to_s
+ end
+
+ def test_to_str
+ assert_equal "--foo", @option.to_str
+ end
+
+ def test_to_json
+ assert_equal %q{"--foo"}, @option.to_json
+ end
+
+ def test_equality
+ foo = Option.new("foo")
+ bar = Option.new("bar")
+ assert_equal foo, @option
+ assert_not_equal bar, @option
+ assert @option.eql?(foo)
+ assert !@option.eql?(bar)
+ assert bar < foo
+ end
+
+ def test_strips_leading_dashes
+ option = Option.new("--foo")
+ assert_equal "foo", option.name
+ assert_equal "--foo", option.flag
+ end
+
+ def test_description
+ assert_empty @option.description
+ assert_equal "foo", Option.new("foo", "foo").description
+ end
+end
+
+class OptionsTests < Test::Unit::TestCase
+ def setup
+ @options = Options.new
+ end
+
+ def test_no_duplicate_options
+ @options << Option.new("foo")
+ @options << Option.new("foo")
+ assert @options.include? "--foo"
+ assert_equal 1, @options.count
+ end
+
+ def test_include
+ @options << Option.new("foo")
+ assert @options.include? "--foo"
+ assert @options.include? "foo"
+ assert @options.include? Option.new("foo")
+ end
+
+ def test_union_returns_options
+ assert_instance_of Options, (@options + Options.new)
+ end
+
+ def test_difference_returns_options
+ assert_instance_of Options, (@options - Options.new)
+ end
+
+ def test_shovel_returns_self
+ assert_same @options, (@options << Option.new("foo"))
+ end
+
+ def test_as_flags
+ @options << Option.new("foo")
+ assert_equal %w{--foo}, @options.as_flags
+ end
+
+ def test_to_a
+ option = Option.new("foo")
+ @options << option
+ assert_equal [option], @options.to_a
+ end
+
+ def test_to_ary
+ option = Option.new("foo")
+ @options << option
+ assert_equal [option], @options.to_ary
+ end
+end
Please sign in to comment.
Something went wrong with that request. Please try again.