Auto merge of #5981 - bundler:seg-cleanup-config, r=indirect

[2.0] Implement config subcommands

### What was the end-user problem that led to this PR?

The problem was the current `bundle config` mega-command is hacky and confusing.

### What was your diagnosis of the problem?

My diagnosis was we should add subcommands for `list`, `get`, `set`, and `unset` to make the CLI clearer.

Closes #4600.

### What is your fix for the problem, implemented in this PR?

My fix implements those subcommands while preserving the current bare `bundle config` command.

### Why did you choose this fix out of the possible options?

I chose this fix because, as opposed to https://github.com/bundler/bundler/pull/5507, we can keep the existing command in for a little while to help ease the transition.

@denniss how do you feel about this compared to your PR?

3 files changed, 237 insertions, 91 deletions

diff --git a/lib/bundler/cli.rb b/lib/bundler/cli.rb
index 05e1851c18..b8e570e380 100644
--- a/lib/bundler/cli.rb
+++ b/lib/bundler/cli.rb
@@ -436,11 +436,8 @@ module Bundler
       will show the current value, as well as any superceded values and
       where they were specified.
     D
-    method_option "parseable", :type => :boolean, :banner => "Use minimal formatting for more parseable output"
-    def config(*args)
-      require "bundler/cli/config"
-      Config.new(options, args, self).run
-    end
+    require "bundler/cli/config"
+    subcommand "config", Config
 
     desc "open GEM", "Opens the source directory of the given bundled gem"
     def open(name)
diff --git a/lib/bundler/cli/config.rb b/lib/bundler/cli/config.rb
index 12f71ea8fe..cc89465adc 100644
--- a/lib/bundler/cli/config.rb
+++ b/lib/bundler/cli/config.rb
@@ -1,119 +1,182 @@
 # frozen_string_literal: true
 
 module Bundler
-  class CLI::Config
-    attr_reader :name, :options, :scope, :thor
-    attr_accessor :args
-
-    def initialize(options, args, thor)
-      @options = options
-      @args = args
-      @thor = thor
-      @name = peek = args.shift
-      @scope = "global"
-      return unless peek && peek.start_with?("--")
-      @name = args.shift
-      @scope = peek[2..-1]
+  class CLI::Config < Thor
+    class_option :parseable, :type => :boolean, :banner => "Use minimal formatting for more parseable output"
+
+    def self.scope_options
+      method_option :global, :type => :boolean, :banner => "Only change the global config"
+      method_option :local, :type => :boolean, :banner => "Only change the local config"
     end
+    private_class_method :scope_options
 
-    def run
-      unless name
-        confirm_all
-        return
-      end
+    desc "base NAME [VALUE]", "The Bundler 1 config interface", :hide => true
+    scope_options
+    method_option :delete, :type => :boolean, :banner => "delete"
+    def base(name = nil, *value)
+      SharedHelpers.major_deprecation 3,
+        "Using the `config` command without a subcommand [list, get, set, unset]"
+      Base.new(options, name, value, self).run
+    end
 
-      unless valid_scope?(scope)
-        Bundler.ui.error "Invalid scope --#{scope} given. Please use --local or --global."
-        exit 1
-      end
+    desc "list", "List out all configured settings"
+    def list
+      Base.new(options, nil, nil, self).run
+    end
+
+    desc "get NAME", "Returns the value for the given key"
+    def get(name)
+      Base.new(options, name, nil, self).run
+    end
+
+    desc "set NAME VALUE", "Sets the given value for the given key"
+    scope_options
+    def set(name, value, *value_)
+      Base.new(options, name, value_.unshift(value), self).run
+    end
+
+    desc "unset NAME", "Unsets the value for the given key"
+    scope_options
+    def unset(name)
+      options[:delete] = true
+      Base.new(options, name, nil, self).run
+    end
 
-      if scope == "delete"
-        Bundler.settings.set_local(name, nil)
-        Bundler.settings.set_global(name, nil)
-        return
+    default_task :base
+
+    class Base
+      attr_reader :name, :value, :options, :scope, :thor
+
+      def initialize(options, name, value, thor)
+        @options = options
+        @name = name
+        value = Array(value)
+        @value = value.empty? ? nil : value.join(" ")
+        @thor = thor
+        validate_scope!
       end
 
-      if args.empty?
-        if options[:parseable]
-          if value = Bundler.settings[name]
-            Bundler.ui.info("#{name}=#{value}")
+      def run
+        unless name
+          warn_unused_scope "Ignoring --#{scope}"
+          confirm_all
+          return
+        end
+
+        if options[:delete]
+          if !explicit_scope? || scope != "global"
+            Bundler.settings.set_local(name, nil)
+          end
+          if !explicit_scope? || scope != "local"
+            Bundler.settings.set_global(name, nil)
           end
           return
         end
 
-        confirm(name)
-        return
-      end
+        if value.nil?
+          warn_unused_scope "Ignoring --#{scope} since no value to set was given"
 
-      Bundler.ui.info(message) if message
-      Bundler.settings.send("set_#{scope}", name, new_value)
-    end
+          if options[:parseable]
+            if value = Bundler.settings[name]
+              Bundler.ui.info("#{name}=#{value}")
+            end
+            return
+          end
+
+          confirm(name)
+          return
+        end
 
-  private
+        Bundler.ui.info(message) if message
+        Bundler.settings.send("set_#{scope}", name, new_value)
+      end
 
-    def confirm_all
-      if @options[:parseable]
-        thor.with_padding do
+      def confirm_all
+        if @options[:parseable]
+          thor.with_padding do
+            Bundler.settings.all.each do |setting|
+              val = Bundler.settings[setting]
+              Bundler.ui.info "#{setting}=#{val}"
+            end
+          end
+        else
+          Bundler.ui.confirm "Settings are listed in order of priority. The top value will be used.\n"
           Bundler.settings.all.each do |setting|
-            val = Bundler.settings[setting]
-            Bundler.ui.info "#{setting}=#{val}"
+            Bundler.ui.confirm "#{setting}"
+            show_pretty_values_for(setting)
+            Bundler.ui.confirm ""
           end
         end
-      else
-        Bundler.ui.confirm "Settings are listed in order of priority. The top value will be used.\n"
-        Bundler.settings.all.each do |setting|
-          Bundler.ui.confirm "#{setting}"
-          show_pretty_values_for(setting)
-          Bundler.ui.confirm ""
-        end
       end
-    end
 
-    def confirm(name)
-      Bundler.ui.confirm "Settings for `#{name}` in order of priority. The top value will be used"
-      show_pretty_values_for(name)
-    end
+      def confirm(name)
+        Bundler.ui.confirm "Settings for `#{name}` in order of priority. The top value will be used"
+        show_pretty_values_for(name)
+      end
 
-    def new_value
-      pathname = Pathname.new(args.join(" "))
-      if name.start_with?("local.") && pathname.directory?
-        pathname.expand_path.to_s
-      else
-        args.join(" ")
+      def new_value
+        pathname = Pathname.new(value)
+        if name.start_with?("local.") && pathname.directory?
+          pathname.expand_path.to_s
+        else
+          value
+        end
       end
-    end
 
-    def message
-      locations = Bundler.settings.locations(name)
-      if @options[:parseable]
-        "#{name}=#{new_value}" if new_value
-      elsif scope == "global"
-        if locations[:local]
-          "Your application has set #{name} to #{locations[:local].inspect}. " \
-            "This will override the global value you are currently setting"
-        elsif locations[:env]
-          "You have a bundler environment variable for #{name} set to " \
-            "#{locations[:env].inspect}. This will take precedence over the global value you are setting"
-        elsif locations[:global] && locations[:global] != args.join(" ")
-          "You are replacing the current global value of #{name}, which is currently " \
-            "#{locations[:global].inspect}"
+      def message
+        locations = Bundler.settings.locations(name)
+        if @options[:parseable]
+          "#{name}=#{new_value}" if new_value
+        elsif scope == "global"
+          if !locations[:local].nil?
+            "Your application has set #{name} to #{locations[:local].inspect}. " \
+              "This will override the global value you are currently setting"
+          elsif locations[:env]
+            "You have a bundler environment variable for #{name} set to " \
+              "#{locations[:env].inspect}. This will take precedence over the global value you are setting"
+          elsif !locations[:global].nil? && locations[:global] != value
+            "You are replacing the current global value of #{name}, which is currently " \
+              "#{locations[:global].inspect}"
+          end
+        elsif scope == "local" && !locations[:local].nil? && locations[:local] != value
+          "You are replacing the current local value of #{name}, which is currently " \
+            "#{locations[:local].inspect}"
         end
-      elsif scope == "local" && locations[:local] != args.join(" ")
-        "You are replacing the current local value of #{name}, which is currently " \
-          "#{locations[:local].inspect}"
       end
-    end
 
-    def show_pretty_values_for(setting)
-      thor.with_padding do
-        Bundler.settings.pretty_values_for(setting).each do |line|
-          Bundler.ui.info line
+      def show_pretty_values_for(setting)
+        thor.with_padding do
+          Bundler.settings.pretty_values_for(setting).each do |line|
+            Bundler.ui.info line
+          end
         end
       end
-    end
 
-    def valid_scope?(scope)
-      %w[delete local global].include?(scope)
+      def explicit_scope?
+        @explicit_scope
+      end
+
+      def warn_unused_scope(msg)
+        return unless explicit_scope?
+        return if options[:parseable]
+
+        Bundler.ui.warn(msg)
+      end
+
+      def validate_scope!
+        @explicit_scope = true
+        scopes = %w[global local].select {|s| options[s] }
+        case scopes.size
+        when 0
+          @scope = "global"
+          @explicit_scope = false
+        when 1
+          @scope = scopes.first
+        else
+          raise InvalidOption,
+            "The options #{scopes.join " and "} were specified. Please only use one of the switches at a time."
+        end
+      end
     end
   end
 end
diff --git a/spec/commands/config_spec.rb b/spec/commands/config_spec.rb
index 9e49357465..c76135d72c 100644
--- a/spec/commands/config_spec.rb
+++ b/spec/commands/config_spec.rb
@@ -362,6 +362,92 @@ E
       expect(out).to match(long_string_without_special_characters)
     end
   end
+
+  describe "subcommands" do
+    it "list" do
+      bundle! "config list"
+      expect(last_command.stdout).to eq "Settings are listed in order of priority. The top value will be used.\nspec_run\nSet via BUNDLE_SPEC_RUN: \"true\""
+
+      bundle! "config list", :parseable => true
+      expect(last_command.stdout).to eq "spec_run=true"
+    end
+
+    it "get" do
+      ENV["BUNDLE_BAR"] = "bar_val"
+
+      bundle! "config get foo"
+      expect(last_command.stdout).to eq "Settings for `foo` in order of priority. The top value will be used\nYou have not configured a value for `foo`"
+
+      ENV["BUNDLE_FOO"] = "foo_val"
+
+      bundle! "config get foo --parseable"
+      expect(last_command.stdout).to eq "foo=foo_val"
+
+      bundle! "config get foo"
+      expect(last_command.stdout).to eq "Settings for `foo` in order of priority. The top value will be used\nSet via BUNDLE_FOO: \"foo_val\""
+    end
+
+    it "set" do
+      bundle! "config set foo 1"
+      expect(last_command.stdout).to eq ""
+
+      bundle! "config set --local foo 2"
+      expect(last_command.stdout).to eq ""
+
+      bundle! "config set --global foo 3"
+      expect(last_command.stdout).to eq "Your application has set foo to \"2\". This will override the global value you are currently setting"
+
+      bundle! "config set --parseable --local foo 4"
+      expect(last_command.stdout).to eq "foo=4"
+
+      bundle! "config set --local foo 4.1"
+      expect(last_command.stdout).to eq "You are replacing the current local value of foo, which is currently \"4\""
+
+      bundle "config set --global --local foo 5"
+      expect(last_command).to be_failure
+      expect(last_command.bundler_err).to eq "The options global and local were specified. Please only use one of the switches at a time."
+    end
+
+    it "unset" do
+      bundle! "config unset foo"
+      expect(last_command.stdout).to eq ""
+
+      bundle! "config set foo 1"
+      bundle! "config unset foo --parseable"
+      expect(last_command.stdout).to eq ""
+
+      bundle! "config set --local foo 1"
+      bundle! "config set --global foo 2"
+
+      bundle! "config unset foo"
+      expect(last_command.stdout).to eq ""
+      expect(bundle!("config get foo")).to eq "Settings for `foo` in order of priority. The top value will be used\nYou have not configured a value for `foo`"
+
+      bundle! "config set --local foo 1"
+      bundle! "config set --global foo 2"
+
+      bundle! "config unset foo --local"
+      expect(last_command.stdout).to eq ""
+      expect(bundle!("config get foo")).to eq "Settings for `foo` in order of priority. The top value will be used\nSet for the current user (#{home(".bundle/config")}): \"2\""
+      bundle! "config unset foo --global"
+      expect(last_command.stdout).to eq ""
+      expect(bundle!("config get foo")).to eq "Settings for `foo` in order of priority. The top value will be used\nYou have not configured a value for `foo`"
+
+      bundle! "config set --local foo 1"
+      bundle! "config set --global foo 2"
+
+      bundle! "config unset foo --global"
+      expect(last_command.stdout).to eq ""
+      expect(bundle!("config get foo")).to eq "Settings for `foo` in order of priority. The top value will be used\nSet for your local app (#{bundled_app(".bundle/config")}): \"1\""
+      bundle! "config unset foo --local"
+      expect(last_command.stdout).to eq ""
+      expect(bundle!("config get foo")).to eq "Settings for `foo` in order of priority. The top value will be used\nYou have not configured a value for `foo`"
+
+      bundle "config unset foo --local --global"
+      expect(last_command).to be_failure
+      expect(last_command.bundler_err).to eq "The options global and local were specified. Please only use one of the switches at a time."
+    end
+  end
 end
 
 RSpec.describe "setting gemfile via config" do