Fixes #30996 - Better family assignment for options

(cherry picked from commit ee8ce26)

lib/hammer_cli/abstract.rb

@@ -190,18 +190,9 @@ def self.build_options(builder_params={})
 
       option_builder.build(builder_params).each do |option|
         # 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
+        next if option.nil? || option.switches.any? { |s| find_option(s) }
+
+        adjust_family(option) if option.respond_to?(:family)
         declared_options << option
         block ||= option.default_conversion_block
         define_accessors_for(option, &block)
@@ -411,6 +402,32 @@ def self.completion_type_for(option, opts = {})
 
     private
 
+    def self.adjust_family(option)
+      # Collect options that should share the same family
+      # If those options have family, adopt the current one
+      # Else adopt those options to the family of the current option
+      # NOTE: this shouldn't rewrite any options,
+      # although options from similar family could be adopted (appended)
+      options = find_options(
+        aliased_resource: option.aliased_resource.to_s
+      ).select { |o| o.family.nil? || o.family.formats.include?(option.value_formatter.class) }.group_by do |o|
+        next :to_skip if option.family.children.include?(o)
+        next :to_adopt if o.family.nil? || o.family.head.nil?
+        next :to_skip if o.family.children.include?(option)
+        # If both family heads handle the same switch
+        # then `option` is probably from similar family and can be adopted
+        next :adopt_by if option.family.head.nil? || o.family.head.handles?(option.family.head.long_switch)
+
+        :to_skip
+      end
+      options[:to_adopt]&.each do |child|
+        option.family&.adopt(child)
+      end
+      options[:adopt_by]&.map(&:family)&.uniq&.each do |family|
+        family.adopt(option)
+      end
+    end
+
     def self.inherited_output_definition
       od = nil
       if superclass.respond_to? :output_definition

lib/hammer_cli/options/option_family.rb

@@ -5,34 +5,40 @@ module Options
     class OptionFamily
       attr_reader :children
 
-      IDS_REGEX = /\s?([Ii][Dd][s]?)\W|([Ii][Dd][s]?\Z)/
+      IDS_REGEX = /(\A[Ii][Dd][s]?)|\s([Ii][Dd][s]?)\W|([Ii][Dd][s]?\Z)/
 
       def initialize(options = {})
         @all = []
         @children = []
         @options = options
         @creator = options[:creator] || Class.new(HammerCLI::Apipie::Command)
         @prefix = options[:prefix]
-        @description = options[:description]
         @root = options[:root] || options[:aliased_resource] || options[:referenced_resource]
       end
 
       def description
         types = all.map(&:type).map { |s| s.split('_').last.to_s }
-                   .map(&:capitalize).join('/')
-        @description ||= @parent.help[1].gsub(IDS_REGEX) { |w| w.gsub(/\w+/, types) }
+                   .map(&:downcase).join('/')
+        parent_desc = @parent.help[1].gsub(IDS_REGEX) { |w| w.gsub(/\w+/, types) }
+        desc = parent_desc.strip.empty? ? @options[:description] : parent_desc
         if @options[:deprecation].class <= String
-          format_deprecation_msg(@description, _('Deprecated: %{deprecated_msg}') % { deprecated_msg: @options[:deprecation] })
+          format_deprecation_msg(desc, _('Deprecated: %{deprecated_msg}') % { deprecated_msg: @options[:deprecation] })
         elsif @options[:deprecation].class <= Hash
           full_msg = @options[:deprecation].map do |flag, msg|
             _('%{flag} is deprecated: %{deprecated_msg}') % { flag: flag, deprecated_msg: msg }
           end.join(', ')
-          format_deprecation_msg(@description, full_msg)
+          format_deprecation_msg(desc, full_msg)
         else
-          @description
+          desc
         end
       end
 
+      def formats
+        return [@options[:format].class] if @options[:format]
+
+        all.map(&:value_formatter).map(&:class).uniq
+      end
+
       def switch
         return if @parent.nil? && @children.empty?
         return @parent.help_lhs.strip if @children.empty?
@@ -67,6 +73,9 @@ def child(switches, type, description, opts = {}, &block)
       end
 
       def adopt(child)
+        raise ArgumentError, 'Parent cannot be a child within the same family' if child == @parent
+        raise ArgumentError, 'Child is already in the family' if @children.include?(child)
+
         child.family = self
         @children << child
       end
@@ -103,9 +112,7 @@ def common_root
         max_len.downto(0) do |curr_len|
           0.upto(max_len - curr_len) do |start|
             root = shortest[start, curr_len]
-            if switches.all? { |switch| switch.include?(root) }
-              return root[2..-1].chomp('-')
-            end
+            return root[2..-1].chomp('-') if switches.all? { |switch| switch.include?(root) }
           end
         end
       end