Split cask config into three tiers.

Library/Homebrew/cask/cmd.rb

@@ -186,16 +186,14 @@ module Cask
     end
 
     def process_options(*args)
-      all_args = Shellwords.shellsplit(ENV["HOMEBREW_CASK_OPTS"] || "") + args
-
       non_options = []
 
-      if idx = all_args.index("--")
+      if idx = args.index("--")
-        non_options += all_args.drop(idx)
+        non_options += args.drop(idx)
-        all_args = all_args.first(idx)
+        args = args.first(idx)
       end
 
-      remaining = all_args.select do |arg|
+      remaining = args.select do |arg|
         begin
           !process_arguments([arg]).empty?
         rescue OptionParser::InvalidOption, OptionParser::MissingArgument, OptionParser::AmbiguousOption

Library/Homebrew/cask/config.rb

@@ -1,27 +1,34 @@
 require "json"
 
+require "extend/hash_validator"
+using HashValidator
+
 module Cask
-  class Config < DelegateClass(Hash)
+  class Config
     DEFAULT_DIRS = {
-      appdir:               "/Applications",
+      appdir:               Pathname("/Applications").expand_path,
-      prefpanedir:          "~/Library/PreferencePanes",
+      prefpanedir:          Pathname("~/Library/PreferencePanes").expand_path,
-      qlplugindir:          "~/Library/QuickLook",
+      qlplugindir:          Pathname("~/Library/QuickLook").expand_path,
-      dictionarydir:        "~/Library/Dictionaries",
+      dictionarydir:        Pathname("~/Library/Dictionaries").expand_path,
-      fontdir:              "~/Library/Fonts",
+      fontdir:              Pathname("~/Library/Fonts").expand_path,
-      colorpickerdir:       "~/Library/ColorPickers",
+      colorpickerdir:       Pathname("~/Library/ColorPickers").expand_path,
-      servicedir:           "~/Library/Services",
+      servicedir:           Pathname("~/Library/Services").expand_path,
-      input_methoddir:      "~/Library/Input Methods",
+      input_methoddir:      Pathname("~/Library/Input Methods").expand_path,
-      internet_plugindir:   "~/Library/Internet Plug-Ins",
+      internet_plugindir:   Pathname("~/Library/Internet Plug-Ins").expand_path,
-      audio_unit_plugindir: "~/Library/Audio/Plug-Ins/Components",
+      audio_unit_plugindir: Pathname("~/Library/Audio/Plug-Ins/Components").expand_path,
-      vst_plugindir:        "~/Library/Audio/Plug-Ins/VST",
+      vst_plugindir:        Pathname("~/Library/Audio/Plug-Ins/VST").expand_path,
-      vst3_plugindir:       "~/Library/Audio/Plug-Ins/VST3",
+      vst3_plugindir:       Pathname("~/Library/Audio/Plug-Ins/VST3").expand_path,
-      screen_saverdir:      "~/Library/Screen Savers",
+      screen_saverdir:      Pathname("~/Library/Screen Savers").expand_path,
     }.freeze
 
     def self.global
       @global ||= new
     end
 
+    def self.clear
+      @global = nil
+    end
+
     def self.for_cask(cask)
       if cask.config_path.exist?
         from_file(cask.config_path)
@@ -37,11 +44,33 @@ module Cask
         raise e, "Cannot parse #{path}: #{e}", e.backtrace
       end
 
-      new(Hash[config.map { |k, v| [k.to_sym, v] }])
+      new(
+        default:  config.fetch("default",  {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h,
+        env:      config.fetch("env",      {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h,
+        explicit: config.fetch("explicit", {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h,
+      )
     end
 
-    def initialize(**dirs)
+    attr_accessor :explicit
-      super(Hash[DEFAULT_DIRS.map { |(k, v)| [k, Pathname(dirs.fetch(k, v)).expand_path] }])
+
+    def initialize(default: nil, env: nil, explicit: {})
+      env&.assert_valid_keys!(*DEFAULT_DIRS.keys)
+      explicit.assert_valid_keys!(*DEFAULT_DIRS.keys)
+
+      @default = default
+      @env = env
+      @explicit = explicit.map { |(k, v)| [k.to_sym, Pathname(v).expand_path] }.to_h
+    end
+
+    def default
+      @default ||= DEFAULT_DIRS
+    end
+
+    def env
+      @env ||= Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", ""))
+                         .map { |arg| arg.split("=", 2) }
+                         .map { |(flag, value)| [flag.sub(/^\-\-/, "").to_sym, Pathname(value).expand_path] }
+                         .to_h
     end
 
     def binarydir
@@ -50,14 +79,26 @@ module Cask
 
     DEFAULT_DIRS.keys.each do |dir|
       define_method(dir) do
-        self[dir]
+        explicit.fetch(dir, env.fetch(dir, default.fetch(dir)))
       end
 
       define_method(:"#{dir}=") do |path|
-        self[dir] = Pathname(path).expand_path
+        explicit[dir] = Pathname(path).expand_path
       end
     end
 
+    def merge(other)
+      self.class.new(**other.explicit.merge(explicit))
+    end
+
+    def to_json(*args)
+      {
+        default:  default,
+        env:      env,
+        explicit: explicit,
+      }.to_json(*args)
+    end
+
     def write(path)
       path.atomic_write(to_json)
     end

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

@@ -1,15 +1,11 @@
 describe Cask::Cmd, :cask do
   it "supports setting the appdir" do
-    allow(Cask::Config.global).to receive(:appdir).and_call_original
-
     described_class.new.process_options("help", "--appdir=/some/path/foo")
 
     expect(Cask::Config.global.appdir).to eq(Pathname.new("/some/path/foo"))
   end
 
   it "supports setting the appdir from ENV" do
-    allow(Cask::Config.global).to receive(:appdir).and_call_original
-
     ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/some/path/bar"
 
     described_class.new.process_options("help")
@@ -18,16 +14,12 @@ describe Cask::Cmd, :cask do
   end
 
   it "supports setting the prefpanedir" do
-    allow(Cask::Config.global).to receive(:prefpanedir).and_call_original
-
     described_class.new.process_options("help", "--prefpanedir=/some/path/foo")
 
     expect(Cask::Config.global.prefpanedir).to eq(Pathname.new("/some/path/foo"))
   end
 
   it "supports setting the prefpanedir from ENV" do
-    allow(Cask::Config.global).to receive(:prefpanedir).and_call_original
-
     ENV["HOMEBREW_CASK_OPTS"] = "--prefpanedir=/some/path/bar"
 
     described_class.new.process_options("help")
@@ -36,16 +28,12 @@ describe Cask::Cmd, :cask do
   end
 
   it "supports setting the qlplugindir" do
-    allow(Cask::Config.global).to receive(:qlplugindir).and_call_original
-
     described_class.new.process_options("help", "--qlplugindir=/some/path/foo")
 
     expect(Cask::Config.global.qlplugindir).to eq(Pathname.new("/some/path/foo"))
   end
 
   it "supports setting the qlplugindir from ENV" do
-    allow(Cask::Config.global).to receive(:qlplugindir).and_call_original
-
     ENV["HOMEBREW_CASK_OPTS"] = "--qlplugindir=/some/path/bar"
 
     described_class.new.process_options("help")
@@ -54,16 +42,12 @@ describe Cask::Cmd, :cask do
   end
 
   it "supports setting the colorpickerdir" do
-    allow(Cask::Config.global).to receive(:colorpickerdir).and_call_original
-
     described_class.new.process_options("help", "--colorpickerdir=/some/path/foo")
 
     expect(Cask::Config.global.colorpickerdir).to eq(Pathname.new("/some/path/foo"))
   end
 
   it "supports setting the colorpickerdir from ENV" do
-    allow(Cask::Config.global).to receive(:colorpickerdir).and_call_original
-
     ENV["HOMEBREW_CASK_OPTS"] = "--colorpickerdir=/some/path/bar"
 
     described_class.new.process_options("help")
@@ -72,16 +56,12 @@ describe Cask::Cmd, :cask do
   end
 
   it "supports setting the dictionarydir" do
-    allow(Cask::Config.global).to receive(:dictionarydir).and_call_original
-
     described_class.new.process_options("help", "--dictionarydir=/some/path/foo")
 
     expect(Cask::Config.global.dictionarydir).to eq(Pathname.new("/some/path/foo"))
   end
 
   it "supports setting the dictionarydir from ENV" do
-    allow(Cask::Config.global).to receive(:dictionarydir).and_call_original
-
     ENV["HOMEBREW_CASK_OPTS"] = "--dictionarydir=/some/path/bar"
 
     described_class.new.process_options("help")
@@ -90,16 +70,12 @@ describe Cask::Cmd, :cask do
   end
 
   it "supports setting the fontdir" do
-    allow(Cask::Config.global).to receive(:fontdir).and_call_original
-
     described_class.new.process_options("help", "--fontdir=/some/path/foo")
 
     expect(Cask::Config.global.fontdir).to eq(Pathname.new("/some/path/foo"))
   end
 
   it "supports setting the fontdir from ENV" do
-    allow(Cask::Config.global).to receive(:fontdir).and_call_original
-
     ENV["HOMEBREW_CASK_OPTS"] = "--fontdir=/some/path/bar"
 
     described_class.new.process_options("help")
@@ -108,16 +84,12 @@ describe Cask::Cmd, :cask do
   end
 
   it "supports setting the servicedir" do
-    allow(Cask::Config.global).to receive(:servicedir).and_call_original
-
     described_class.new.process_options("help", "--servicedir=/some/path/foo")
 
     expect(Cask::Config.global.servicedir).to eq(Pathname.new("/some/path/foo"))
   end
 
   it "supports setting the servicedir from ENV" do
-    allow(Cask::Config.global).to receive(:servicedir).and_call_original
-
     ENV["HOMEBREW_CASK_OPTS"] = "--servicedir=/some/path/bar"
 
     described_class.new.process_options("help")
@@ -126,8 +98,6 @@ describe Cask::Cmd, :cask do
   end
 
   it "allows additional options to be passed through" do
-    allow(Cask::Config.global).to receive(:appdir).and_call_original
-
     rest = described_class.new.process_options("edit", "foo", "--create", "--appdir=/some/path/qux")
 
     expect(Cask::Config.global.appdir).to eq(Pathname.new("/some/path/qux"))

Library/Homebrew/test/cask/cmd_spec.rb

@@ -56,9 +56,7 @@ describe Cask::Cmd, :cask do
     end
 
     it "respects the env variable when choosing what appdir to create" do
-      allow(ENV).to receive(:[]).and_call_original
+      ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/custom/appdir"
-      allow(ENV).to receive(:[]).with("HOMEBREW_CASK_OPTS").and_return("--appdir=/custom/appdir")
-      allow(Cask::Config.global).to receive(:appdir).and_call_original
 
       described_class.run("noop")
 

Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb

@@ -4,25 +4,34 @@ require "test/support/helper/cask/fake_system_command"
 require "test/support/helper/cask/install_helper"
 require "test/support/helper/cask/never_sudo_system_command"
 
-HOMEBREW_CASK_DIRS = {
+module Cask
+  class Config
+    remove_const :DEFAULT_DIRS
+
+    DEFAULT_DIRS = {
       appdir:               Pathname.new(TEST_TMPDIR).join("cask-appdir"),
       prefpanedir:          Pathname.new(TEST_TMPDIR).join("cask-prefpanedir"),
       qlplugindir:          Pathname.new(TEST_TMPDIR).join("cask-qlplugindir"),
+      dictionarydir:        Pathname.new(TEST_TMPDIR).join("cask-dictionarydir"),
+      fontdir:              Pathname.new(TEST_TMPDIR).join("cask-fontdir"),
+      colorpickerdir:       Pathname.new(TEST_TMPDIR).join("cask-colorpickerdir"),
       servicedir:           Pathname.new(TEST_TMPDIR).join("cask-servicedir"),
+      input_methoddir:      Pathname.new(TEST_TMPDIR).join("cask-input_methoddir"),
+      internet_plugindir:   Pathname.new(TEST_TMPDIR).join("cask-internet_plugindir"),
+      audio_unit_plugindir: Pathname.new(TEST_TMPDIR).join("cask-audio_unit_plugindir"),
+      vst_plugindir:        Pathname.new(TEST_TMPDIR).join("cask-vst_plugindir"),
+      vst3_plugindir:       Pathname.new(TEST_TMPDIR).join("cask-vst3_plugindir"),
+      screen_saverdir:      Pathname.new(TEST_TMPDIR).join("cask-screen_saverdir"),
     }.freeze
+  end
+end
 
 RSpec.shared_context "Homebrew Cask", :needs_macos do
-  before do
-    HOMEBREW_CASK_DIRS.each do |method, path|
-      Cask::Config.global.send("#{method}=", path)
-    end
-  end
-
   around do |example|
     third_party_tap = Tap.fetch("third-party", "tap")
-    begin
-      HOMEBREW_CASK_DIRS.values.each(&:mkpath)
 
+    begin
+      Cask::Config::DEFAULT_DIRS.values.each(&:mkpath)
       Cask::Config.global.binarydir.mkpath
 
       Tap.default_cask_tap.tap do |tap|
@@ -37,11 +46,12 @@ RSpec.shared_context "Homebrew Cask", :needs_macos do
 
       example.run
     ensure
-      FileUtils.rm_rf HOMEBREW_CASK_DIRS.values
+      FileUtils.rm_rf Cask::Config::DEFAULT_DIRS.values
       FileUtils.rm_rf [Cask::Config.global.binarydir, Cask::Caskroom.path, Cask::Cache.path]
       Tap.default_cask_tap.path.unlink
       third_party_tap.path.unlink
       FileUtils.rm_rf third_party_tap.path.parent
+      Cask::Config.clear
     end
   end
 end