Skip to content

Commit

Permalink
Fixes #25307 - Add ability to disable defaults (#293)
Browse files Browse the repository at this point in the history
Fixes #25307 - Add ability to disable defaults

Processing of defaults can be altered in cli_config.yml with
:use_defaults: true/false or on CLI with --use-defaults
--no-use-defaults respectively. both ways can be combined.
The value is propagated in the context as :use_defaults.
The defaults are enabled by default.

Hammer settings class was changed so that it can be the single
source of default values which has three benefits
 - we don't have to fallback to default with every use of the value
 - we can keep the values commented out in config which makes it easier
to change later
 - there is single place to look for the default setting
  • Loading branch information
mbacovsky committed Nov 6, 2018
1 parent 3f5b2ab commit 5baac2d
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 10 deletions.
3 changes: 3 additions & 0 deletions bin/hammer
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ require 'hammer_cli/options/normalizers'
# Option descriptions are never displayed and thus do not require translation.
class PreParser < Clamp::Command
option ['-v', '--[no-]verbose'], :flag, _('Be verbose (or not). True by default')
option ['--[no-]use-defaults'], :flag, _('Enable/disable stored defaults. Enabled by default')
option ['-q', '--quiet'], :flag, _('Completely silent')
option ["-d", "--debug"], :flag, "show debugging output"
option ["-c", "--config"], "CFG_FILE", "path to custom config file" do |path|
Expand Down Expand Up @@ -92,6 +93,8 @@ HammerCLI::Settings.load({
:ssl_with_basic_auth => preparser.ssl_with_basic_auth?
}})

HammerCLI::Settings.load({:use_defaults => preparser.use_defaults?}) unless preparser.use_defaults?.nil?

if HammerCLI::Settings.get(:ui, :mark_translated)
include HammerCLI::I18n::Debug
end
Expand Down
3 changes: 3 additions & 0 deletions config/cli_config.template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
# 2 - data, messages, interactive I/O, progress bars. Equals to --verbose.
:verbosity: 2

# Enable/disable defaults. Preset default values for the options won't be used.
#:use_defaults: true

#:log_owner: 'foreman'
#:log_group: 'foreman'

Expand Down
7 changes: 3 additions & 4 deletions lib/hammer_cli/abstract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,9 @@ def option_collector


def option_sources
[
HammerCLI::Options::Sources::CommandLine.new(self),
HammerCLI::Options::Sources::SavedDefaults.new(context[:defaults], logger)
]
sources = [HammerCLI::Options::Sources::CommandLine.new(self)]
sources << HammerCLI::Options::Sources::SavedDefaults.new(context[:defaults], logger) if context[:use_defaults]
sources
end

private
Expand Down
1 change: 1 addition & 0 deletions lib/hammer_cli/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ def self.context
:is_tty? => HammerCLI.tty?,
:api_connection => HammerCLI::Connection.new(Logging.logger['Connection']),
:no_headers => HammerCLI::Settings.get(:ui, :no_headers),
:use_defaults => HammerCLI::Settings.get(:use_defaults),
:capitalization => HammerCLI.capitalization,
:verbosity => (HammerCLI::Settings.get(:verbosity) || HammerCLI::V_VERBOSE).to_i
}
Expand Down
4 changes: 1 addition & 3 deletions lib/hammer_cli/defaults.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ class DefaultsPathError < DefaultsError; end
attr_reader :defaults_settings

def initialize(settings, file_path = nil)

@defaults_settings = settings || {}
@path = file_path || DEFAULT_FILE
end
Expand Down Expand Up @@ -100,8 +99,7 @@ def switch_to_name(opt)
end

def self.defaults
@defaults ||= Defaults.new(HammerCLI::Settings.settings[:defaults])

@defaults ||= Defaults.new(HammerCLI::Settings.get(:defaults))
end

HammerCLI::MainCommand.subcommand "defaults", _("Defaults management"), HammerCLI::DefaultsCommand
Expand Down
1 change: 1 addition & 0 deletions lib/hammer_cli/main.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class MainCommand < AbstractCommand
option ['-q', '--quiet'], :flag, _('Completely silent') do
context[:verbosity] = HammerCLI::V_QUIET
end
option ['--[no-]use-defaults'], :flag, _('Enable/disable stored defaults. Enabled by default')
option ["-d", "--debug"], :flag, _("Show debugging output"), :context_target => :debug
option ["-r", "--reload-cache"], :flag, _("Force reload of Apipie cache")

Expand Down
14 changes: 13 additions & 1 deletion lib/hammer_cli/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ def self.load(settings_hash)
end

def self.clear
empty
load(default_settings)
end

def self.empty
settings.clear
path_history.clear
end
Expand All @@ -59,9 +64,16 @@ def self.path_history
@path_history
end

def self.default_settings
{
:use_defaults => true
}
end

private

def self.settings
@settings_hash ||= {}
@settings_hash ||= default_settings
@settings_hash
end

Expand Down
8 changes: 7 additions & 1 deletion test/unit/abstract_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,20 @@ def options
before do
@defaults = mock()
@defaults.stubs(:get_defaults).returns(nil)
@cmd = TestDefaultsCmd.new("", { :defaults => @defaults })
@cmd = TestDefaultsCmd.new("", { :defaults => @defaults, :use_defaults => true })
@cmd_no_defaults = TestDefaultsCmd.new("", { :defaults => @defaults, :use_defaults => false })
end

it 'provides default value for an option flag' do
@defaults.expects(:get_defaults).with('--test').returns(2)
assert_equal({'different_attr_name' => 2}, @cmd.options)
end

it 'does not set default value if the defaults are disabled' do
@defaults.expects(:get_defaults).never()
assert_equal({}, @cmd_no_defaults.options)
end

it 'prefers values from command line' do
@defaults.stubs(:get_defaults).with('--test').returns(2)
@cmd.run(['--test=1'])
Expand Down
15 changes: 14 additions & 1 deletion test/unit/settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
end

it "dumps all settings" do
settings.empty
data = {:a => 1, :b => 2}
settings.load(data)
settings.dump.must_equal data
Expand All @@ -69,11 +70,23 @@
settings.get(:x).size.must_equal 4
end

it "clear wipes all settings" do
it "clear wipes all settings but default values" do
settings.load({:a => 1, :b => 2})
settings.clear
settings.get(:a).must_be_nil
settings.get(:b).must_be_nil
settings.default_settings.each { |key, val| settings.get(key).must_equal val }
end

it "empty wipes all settings including default values" do
settings.load({:a => 1, :b => 2})
settings.empty
settings.dump.must_equal({})
end

it "initializes settings with default settings" do
settings.instance_variable_set(:@settings_hash, nil)
settings.dump.must_equal settings.default_settings
end

context "load from paths" do
Expand Down

0 comments on commit 5baac2d

Please sign in to comment.