Skip to content

Commit

Permalink
Fixes #28440 - Help contains squeezed options
Browse files Browse the repository at this point in the history
  • Loading branch information
ofedoren committed Apr 21, 2020
1 parent b384850 commit 6ee19a9
Show file tree
Hide file tree
Showing 14 changed files with 347 additions and 26 deletions.
12 changes: 12 additions & 0 deletions doc/commands_extension.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ Each command can be easily extended with one ore more `HammerCLI::CommandExtensi
inheritable true
# Simply add a new option to a command is being extended
option(option_params)
# Add option family to a command
option_family(common_options = {}) do
parent option_params
child option_params
end
# Extend hash with data returned from server before it is printed
before_print do |data|
# data modifications
Expand Down Expand Up @@ -65,6 +70,13 @@ class MyCommandExtensions < HammerCLI::CommandExtensions
option ['--new-option'], 'TYPE', _('Option description')
option_family(
description: _('Common description')
) do
parent ['--new-option'], 'TYPE', _('Option description')
child ['--new-option-ver2'], 'TYPE', _('Option description')
end
before_print do |data|
data['results'].each do |result|
result['status'] = process_errors(result['errors'])
Expand Down
48 changes: 48 additions & 0 deletions doc/creating_commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,54 @@ Here is the list of predefined options:
* `:fields` Expects a list with fields to show in output, see [example](creating_commands.md#printing-hash-records).


### Option family
Option family is the way to unify options which have the same meaning or purpose,
but contain some differences in their definitions (e.g. the name/switch of an option).
Mainly serves as a container for options, which purpose is to show less repetitive
output in commands' help. Option builders use it by default.

To define an option family, use the following DSL:
```ruby
# options is a Hash with options for family/each defined option within it
option_family(options = {}) do
# parent is the main option. Must be single, option family can have only one parent.
parent switches, type, description, options
# child is an additional option. Could be none or more than one. Aren't shown in the help output.
child switches, type, description, options
end
```

##### Example

```ruby
option_family(
aliased_resource: 'environment',
description: _('Puppet environment'),
deprecation: _("Use %s instead") % '--puppet-environment[-id]'
deprecated: { '--environment' => _("Use %s instead") % '--puppet-environment[-id]',
'--environment-id' => _("Use %s instead") % '--puppet-environment[-id]'}
) do
parent '--environment-id', 'ENVIRONMENT_ID', _(''),
format: HammerCLI::Options::Normalizers::Number.new,
attribute_name: :option_environment_id
child '--environment', 'ENVIRONMENT_NAME', _('Environment name'),
attribute_name: :option_environment_name
end

# $ hammer command --help:
# ...
# Options:
# --environment[-id] Puppet environment (Deprecated: Use --puppet-environment[-id] instead)
# ...

# $ hammer full-help:
# ...
# Options:
# --environment ENVIRONMENT_NAME Environment name (--environment is deprecated: Use --puppet-environment[-id] instead)
# --environment-id ENVIRONMENT_ID (--environment-id is deprecated: Use --puppet-environment[-id] instead)
# ...
```

### Option builders
Hammer commands offer option builders that can be used for automatic option generation.
See [documentation page](option_builders.md#option-builders) dedicated to this topic for more details.
Expand Down
18 changes: 18 additions & 0 deletions lib/hammer_cli/abstract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
require 'hammer_cli/help/builder'
require 'hammer_cli/help/text_builder'
require 'hammer_cli/command_extensions'
require 'hammer_cli/options/option_family'
require 'logging'

module HammerCLI
Expand Down Expand Up @@ -191,6 +192,16 @@ def self.build_options(builder_params={})
# skip switches that are already defined
next if option.nil? or option.switches.any? {|s| find_option(s) }

if option.respond_to?(:referenced_resource)
# Collect options that don't have family, but related to this parent.
children = find_options(
referenced_resource: option.referenced_resource.to_s,
aliased_resource: option.aliased_resource.to_s
).select { |o| o.family.nil? || o.family.head.nil? }
children.each do |child|
option.family.adopt(child) if option.family
end
end
declared_options << option
block ||= option.default_conversion_block
define_accessors_for(option, &block)
Expand All @@ -207,6 +218,7 @@ def self.extend_with(*extensions)
extension.delegatee(self)
extension.extend_predefined_options(self)
extension.extend_options(self)
extension.extend_option_family(self)
extension.extend_output(self)
extension.extend_help(self)
logger('Extensions').info "Applied #{extension.details} on #{self}."
Expand All @@ -222,6 +234,12 @@ def self.use_option(*names)

protected

def self.option_family(options = {}, &block)
options[:creator] ||= self
family = HammerCLI::Options::OptionFamily.new(options)
family.instance_eval(&block)
end

def self.find_options(switch_filter, other_filters={})
filters = other_filters
if switch_filter.is_a? Hash
Expand Down
28 changes: 15 additions & 13 deletions lib/hammer_cli/apipie/option_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ def options_for_params(params, filter, resource_name_map)
end

def create_option(param, resource_name_map)
option(
option_switch(param, resource_name_map),
option_type(param, resource_name_map),
option_desc(param),
option_opts(param)
)
family = HammerCLI::Options::OptionFamily.new
family.parent(option_switch(param, resource_name_map),
option_type(param, resource_name_map),
option_desc(param),
option_opts(param, resource_name_map))
end

def option_switch(param, resource_name_map)
Expand All @@ -61,7 +60,7 @@ def option_desc(param)
param.description || " "
end

def option_opts(param)
def option_opts(param, resource_name_map)
opts = {}
opts[:required] = true if (param.required? and require_options?)
if param.expected_type.to_s == 'array'
Expand All @@ -80,19 +79,22 @@ def option_opts(param)
end
opts[:attribute_name] = HammerCLI.option_accessor_name(param.name)
opts[:referenced_resource] = resource_name(param)
opts[:aliased_resource] = aliased_name(resource_name(param), resource_name_map)

return opts
end

def aliased_name(name, resource_name_map)
return if name.nil?

resource_name_map[name.to_s] || resource_name_map[name.to_sym] || name
end

def aliased(param, resource_name_map)
resource_name = resource_name(param)
return param.name if resource_name.nil?

if resource_name.nil?
return param.name
else
aliased_name = resource_name_map[resource_name.to_s] || resource_name_map[resource_name.to_sym] || resource_name
return param.name.gsub(resource_name, aliased_name.to_s)
end
param.name.gsub(resource_name, aliased_name(resource_name, resource_name_map).to_s)
end

def resource_name(param)
Expand Down
12 changes: 8 additions & 4 deletions lib/hammer_cli/apipie/option_definition.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
require File.join(File.dirname(__FILE__), 'options')

module HammerCLI::Apipie

class OptionDefinition < HammerCLI::Options::OptionDefinition

attr_accessor :referenced_resource
attr_accessor :referenced_resource, :aliased_resource, :family

def initialize(switches, type, description, options = {})
@referenced_resource = options[:referenced_resource].to_s if options[:referenced_resource]
@aliased_resource = options[:aliased_resource].to_s if options[:aliased_resource]
@family = options[:family]
super
# Apipie currently sends descriptions as escaped HTML once this is changed this should be removed.
# See #15198 on Redmine.
@description = CGI::unescapeHTML(description)
end

end
def child?
return unless @family

@family.children.include?(self)
end
end
end
22 changes: 21 additions & 1 deletion lib/hammer_cli/command_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def inheritable?
ALLOWED_EXTENSIONS = %i[
option command_options before_print data output help request
request_headers headers request_options options request_params params
option_sources predefined_options use_option
option_sources predefined_options use_option option_family
].freeze

def initialize(options = {})
Expand Down Expand Up @@ -86,6 +86,11 @@ def self.option_sources(&block)
@option_sources_block = block
end

def self.option_family(options = {}, &block)
@option_family_opts = options
@option_family_block = block
end

# Object

def extend_options(command_class)
Expand Down Expand Up @@ -151,6 +156,13 @@ def extend_option_sources(sources, command = nil)
self.class.extend_option_sources(sources, command)
end

def extend_option_family(command_class)
allowed = @only & %i[option_family]
return if allowed.empty? || (allowed & @except).any?

self.class.extend_option_family(command_class)
end

def delegatee(command_class)
self.class.delegatee = command_class
end
Expand Down Expand Up @@ -234,5 +246,13 @@ def self.extend_option_sources(sources, command = nil)
@option_sources_block.call(sources, command)
logger.debug("Called block for #{@delegatee} option sources:\n\t#{@option_sources_block}")
end

def self.extend_option_family(command_class)
return if @option_family_block.nil?

@option_family_opts[:creator] = command_class
command_class.send(:option_family, @option_family_opts, &@option_family_block)
logger.debug("Called option family block for #{command_class}:\n\t#{@option_family_block}")
end
end
end
2 changes: 2 additions & 0 deletions lib/hammer_cli/full_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ class FullHelpCommand < HammerCLI::AbstractCommand

def execute
@adapter = option_md? ? MDAdapter.new : TxtAdapter.new
HammerCLI.context[:full_help] = true
print_heading
print_help
HammerCLI.context[:full_help] = false
HammerCLI::EX_OK
end

Expand Down
11 changes: 9 additions & 2 deletions lib/hammer_cli/help/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,19 @@ def add_list(heading, items)

label_width = DEFAULT_LABEL_INDENT
items.each do |item|
label, description = item.help
label = item.help.first
label_width = label.size if label.size > label_width
end

items.each do |item|
label, description = item.help
if item.respond_to?(:child?) && item.child?
next unless HammerCLI.context[:full_help]
end
label, description = if !HammerCLI.context[:full_help] && item.respond_to?(:family) && item.family
[item.family.switch, item.family.description || item.help[1]]
else
item.help
end
description.gsub(/^(.)/) { Unicode::capitalize($1) }.each_line do |line|
puts " %-#{label_width}s %s" % [label, line]
label = ''
Expand Down
10 changes: 4 additions & 6 deletions lib/hammer_cli/options/option_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ module Options

class OptionDefinition < Clamp::Option::Definition

attr_accessor :value_formatter
attr_accessor :context_target
attr_accessor :deprecated_switches
attr_accessor :value_formatter, :context_target, :deprecated_switches

def initialize(switches, type, description, options = {})
self.value_formatter = options[:format] || HammerCLI::Options::Normalizers::Default.new
self.context_target = options[:context_target]
self.deprecated_switches = options[:deprecated]
@value_formatter = options[:format] || HammerCLI::Options::Normalizers::Default.new
@context_target = options[:context_target]
@deprecated_switches = options[:deprecated]
super
end

Expand Down
Loading

0 comments on commit 6ee19a9

Please sign in to comment.