Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #25307 - Add ability to disable defaults #293

Merged
merged 4 commits into from Nov 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions bin/hammer
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?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it a duplication of https://github.com/theforeman/hammer-cli/pull/293/files#diff-aca6a835822ab9b99782be0a5f01f8ddR87 ? If it's a shorcut for Settings.get(:_params, :use_defaults), why keep it under _params then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ofedoren Exactly. I had a reason to have it there but it was bad idea indeed. Removed.


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
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
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
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
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
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
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
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
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