Merge pull request #2284 from reitermarkus/cask-refactor-cli

Refactor CLI options.

Library/Homebrew/cask/lib/hbc.rb

@@ -21,7 +21,6 @@ require "hbc/fetcher"
 require "hbc/installer"
 require "hbc/locations"
 require "hbc/macos"
-require "hbc/options"
 require "hbc/pkg"
 require "hbc/qualified_token"
 require "hbc/scopes"
@@ -40,7 +39,6 @@ require "utils"
 module Hbc
   include Locations
   include Scopes
-  include Options
   include Utils
 
   def self.init

Library/Homebrew/cask/lib/hbc/artifact/binary.rb

@@ -4,7 +4,7 @@ module Hbc
   module Artifact
     class Binary < Symlinked
       def install_phase
-        super unless Hbc.no_binaries
+        super if CLI.binaries?
       end
     end
   end

Library/Homebrew/cask/lib/hbc/artifact/pkg.rb

@@ -48,7 +48,7 @@ module Hbc
           "-pkg",    source,
           "-target", "/"
         ]
-        args << "-verboseR" if Hbc.verbose
+        args << "-verboseR" if CLI.verbose?
         args << "-allowUntrusted" if pkg_install_opts :allow_untrusted
         with_choices_file do |choices_path|
           args << "-applyChoiceChangesXML" << choices_path if choices_path

Library/Homebrew/cask/lib/hbc/cask.rb

@@ -90,8 +90,7 @@ module Hbc
     end
 
     def dumpcask
-      return unless Hbc.respond_to?(:debug)
+      return unless CLI.debug?
-      return unless Hbc.debug
 
       odebug "Cask instance dumps in YAML:"
       odebug "Cask instance toplevel:", to_yaml

Library/Homebrew/cask/lib/hbc/cli.rb

@@ -68,13 +68,25 @@ module Hbc
     }.freeze
 
     FLAGS = {
-      "--no-binaries" => :no_binaries=,
+      ["--[no-]binaries", :binaries] => true,
-      "--debug"       => :debug=,
+      ["--debug",         :debug]    => false,
-      "--verbose"     => :verbose=,
+      ["--verbose",       :verbose]  => false,
-      "--outdated"    => :cleanup_outdated=,
+      ["--outdated",      :outdated] => false,
-      "--help"        => :help=,
+      ["--help",          :help]     => false,
     }.freeze
 
+    FLAGS.each do |(_, method), default_value|
+      instance_variable_set(:"@#{method}", default_value)
+
+      define_singleton_method(:"#{method}=") do |arg|
+        instance_variable_set(:"@#{method}", arg)
+      end
+
+      define_singleton_method(:"#{method}?") do
+        instance_variable_get(:"@#{method}")
+      end
+    end
+
     def self.command_classes
       @command_classes ||= constants.map(&method(:const_get))
                                     .select { |sym| sym.respond_to?(:run) }
@@ -138,13 +150,13 @@ module Hbc
 
       command_string, *rest = *arguments
       rest = process_options(rest)
-      command = Hbc.help ? "help" : lookup_command(command_string)
+      command = help? ? "help" : lookup_command(command_string)
       Hbc.default_tap.install unless Hbc.default_tap.installed?
       Hbc.init if should_init?(command)
       run_command(command, *rest)
     rescue CaskError, CaskSha256MismatchError, ArgumentError => e
       msg = e.message
-      msg << e.backtrace.join("\n") if Hbc.debug
+      msg << e.backtrace.join("\n") if debug?
       onoe msg
       exit 1
     rescue StandardError, ScriptError, NoMemoryError => e
@@ -194,9 +206,9 @@ module Hbc
           EOS
         end
 
-        FLAGS.each do |flag, method|
+        FLAGS.keys.each do |flag, method|
-          opts.on(flag) do
+          opts.on(flag) do |bool|
-            Hbc.public_send(method, true)
+            send(:"#{method}=", bool)
           end
         end
 
@@ -224,7 +236,8 @@ module Hbc
       end
 
       # for compat with Homebrew, not certain if this is desirable
-      Hbc.verbose = true if !ENV["VERBOSE"].nil? || !ENV["HOMEBREW_VERBOSE"].nil?
+      self.verbose = true if ARGV.verbose?
+      self.debug = true if ARGV.debug?
 
       remaining
     end

Library/Homebrew/cask/lib/hbc/cli/cleanup.rb

@@ -21,7 +21,7 @@ module Hbc
       end
 
       def self.default
-        @default ||= new(Hbc.cache, Hbc.cleanup_outdated)
+        @default ||= new(Hbc.cache, CLI.outdated?)
       end
 
       attr_reader :cache_location, :outdated_only

Library/Homebrew/cask/lib/hbc/cli/internal_dump.rb

@@ -12,7 +12,7 @@ module Hbc
       end
 
       def self.dump_casks(*cask_tokens)
-        Hbc.debug = true # Yuck. At the moment this is the only way to make dumps visible
+        CLI.debug = true # Yuck. At the moment this is the only way to make dumps visible
         count = 0
         cask_tokens.each do |cask_token|
           begin

Library/Homebrew/cask/lib/hbc/options.rb

@@ -1,39 +0,0 @@
-module Hbc
-  module Options
-    def self.included(base)
-      base.extend(ClassMethods)
-    end
-
-    module ClassMethods
-      attr_writer :no_binaries
-
-      def no_binaries
-        @no_binaries ||= false
-      end
-
-      attr_writer :debug
-
-      def debug
-        @debug ||= false
-      end
-
-      attr_writer :verbose
-
-      def verbose
-        @verbose ||= false
-      end
-
-      attr_writer :cleanup_outdated
-
-      def cleanup_outdated
-        @cleanup_outdated ||= false
-      end
-
-      attr_writer :help
-
-      def help
-        @help ||= false
-      end
-    end
-  end
-end

Library/Homebrew/cask/lib/hbc/system_command.rb

@@ -154,7 +154,7 @@ module Hbc
       def self._parse_plist(command, output)
         raise CaskError, "Empty plist input" unless output =~ /\S/
         output.sub!(/\A(.*?)(<\?\s*xml)/m, '\2')
-        _warn_plist_garbage(command, Regexp.last_match[1]) if Hbc.debug
+        _warn_plist_garbage(command, Regexp.last_match[1]) if CLI.debug?
         output.sub!(%r{(<\s*/\s*plist\s*>)(.*?)\Z}m, '\1')
         _warn_plist_garbage(command, Regexp.last_match[2])
         xml = Plist.parse_xml(output)

Library/Homebrew/cask/lib/hbc/utils.rb

@@ -30,8 +30,7 @@ end
 # global methods
 
 def odebug(title, *sput)
-  return unless Hbc.respond_to?(:debug)
+  return unless Hbc::CLI.debug?
-  return unless Hbc.debug
   puts Formatter.headline(title, color: :magenta)
   puts sput unless sput.empty?
 end

Library/Homebrew/test/cask/artifact/binary_spec.rb

@@ -47,15 +47,19 @@ describe Hbc::Artifact::Binary, :cask do
   end
 
   it "respects --no-binaries flag" do
-    Hbc.no_binaries = true
+    begin
+      Hbc::CLI.binaries = false
 
-    shutup do
+      expect(Hbc::CLI).not_to be_binaries
-      Hbc::Artifact::Binary.new(cask).install_phase
+
+      shutup do
+        Hbc::Artifact::Binary.new(cask).install_phase
+      end
+
+      expect(expected_path.exist?).to be false
+    ensure
+      Hbc::CLI.binaries = true
     end
-
-    expect(expected_path.exist?).to be false
-
-    Hbc.no_binaries = false
   end
 
   it "creates parent directory if it doesn't exist" do

Library/Homebrew/test/cask/cli/options_spec.rb

@@ -122,17 +122,23 @@ describe Hbc::CLI, :cask do
 
   describe "--debug" do
     it "sets the Cask debug method to true" do
-      Hbc::CLI.process_options %w[help --debug]
+      begin
-      expect(Hbc.debug).to be true
+        Hbc::CLI.process_options %w[help --debug]
-      Hbc.debug = false
+        expect(Hbc::CLI.debug?).to be true
+      ensure
+        Hbc::CLI.debug = false
+      end
     end
   end
 
   describe "--help" do
     it "sets the Cask help method to true" do
-      Hbc::CLI.process_options %w[foo --help]
+      begin
-      expect(Hbc.help).to be true
+        Hbc::CLI.process_options %w[foo --help]
-      Hbc.help = false
+        expect(Hbc::CLI.help?).to be true
+      ensure
+        Hbc::CLI.help = false
+      end
     end
   end
 end

Library/Homebrew/test/cask/cli_spec.rb

@@ -32,10 +32,13 @@ describe Hbc::CLI, :cask do
     end
 
     it "prints help output when subcommand receives `--help` flag" do
-      expect(described_class).to receive(:run_command).with("help")
+      begin
-      described_class.process(%w[noop --help])
+        expect(described_class).to receive(:run_command).with("help")
-      expect(Hbc.help).to eq(true)
+        described_class.process(%w[noop --help])
-      Hbc.help = false
+        expect(Hbc::CLI.help?).to eq(true)
+      ensure
+        Hbc::CLI.help = false
+      end
     end
 
     it "respects the env variable when choosing what appdir to create" do