Merge pull request #10782 from chef/cleanup-knife-status

Signed-off-by: Tim Smith <tsmith@chef.io>

8 files changed, 63 insertions, 66 deletions

diff --git a/cspell.json b/cspell.json
index 2c34043b30..a55b2bfc40 100644
--- a/cspell.json
+++ b/cspell.json
@@ -625,7 +625,6 @@
     "HHOOK",
     "HIBYTE",
     "HICON",
-    "hidemins",
     "Hinderliter",
     "HINSTANCE",
     "hintname",
diff --git a/lib/chef/knife/core/formatting_options.rb b/lib/chef/knife/core/formatting_options.rb
new file mode 100644
index 0000000000..cdee2c5989
--- /dev/null
+++ b/lib/chef/knife/core/formatting_options.rb
@@ -0,0 +1,49 @@
+#
+# Author:: Nicolas DUPEUX (<nicolas.dupeux@arkea.com>)
+# Copyright:: Copyright (c) Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+class Chef
+  class Knife
+    module Core
+
+      # This module may be included into a knife subcommand class to automatically
+      # add configuration options used by the StatusPresenter and NodePresenter.
+      module FormattingOptions
+        # @private
+        # Would prefer to do this in a rational way, but can't be done b/c of
+        # Mixlib::CLI's design :(
+        def self.included(includer)
+          includer.class_eval do
+            option :medium_output,
+              short: "-m",
+              long: "--medium",
+              boolean: true,
+              default: false,
+              description: "Include normal attributes in the output"
+
+            option :long_output,
+              short: "-l",
+              long: "--long",
+              boolean: true,
+              default: false,
+              description: "Include all attributes in the output"
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/knife/core/node_presenter.rb b/lib/chef/knife/core/node_presenter.rb
index 6690bc1075..8c948cf76c 100644
--- a/lib/chef/knife/core/node_presenter.rb
+++ b/lib/chef/knife/core/node_presenter.rb
@@ -23,31 +23,6 @@ class Chef
   class Knife
     module Core
 
-      # This module may be included into a knife subcommand class to automatically
-      # add configuration options used by the NodePresenter
-      module NodeFormattingOptions
-        # @private
-        # Would prefer to do this in a rational way, but can't be done b/c of
-        # Mixlib::CLI's design :(
-        def self.included(includer)
-          includer.class_eval do
-            option :medium_output,
-              short: "-m",
-              long: "--medium",
-              boolean: true,
-              default: false,
-              description: "Include normal attributes in the output"
-
-            option :long_output,
-              short: "-l",
-              long: "--long",
-              boolean: true,
-              default: false,
-              description: "Include all attributes in the output"
-          end
-        end
-      end
-
       # A customized presenter for Chef::Node objects. Supports variable-length
       # output formats for displaying node data
       class NodePresenter < GenericPresenter
diff --git a/lib/chef/knife/core/status_presenter.rb b/lib/chef/knife/core/status_presenter.rb
index 7818d2e7cb..271c71d618 100644
--- a/lib/chef/knife/core/status_presenter.rb
+++ b/lib/chef/knife/core/status_presenter.rb
@@ -23,31 +23,6 @@ class Chef
   class Knife
     module Core
 
-      # This module may be included into a knife subcommand class to automatically
-      # add configuration options used by the StatusPresenter
-      module StatusFormattingOptions
-        # @private
-        # Would prefer to do this in a rational way, but can't be done b/c of
-        # Mixlib::CLI's design :(
-        def self.included(includer)
-          includer.class_eval do
-            option :medium_output,
-              short: "-m",
-              long: "--medium",
-              boolean: true,
-              default: false,
-              description: "Include normal attributes in the output"
-
-            option :long_output,
-              short: "-l",
-              long: "--long",
-              boolean: true,
-              default: false,
-              description: "Include all attributes in the output"
-          end
-        end
-      end
-
       # A customized presenter for Chef::Node objects. Supports variable-length
       # output formats for displaying node data
       class StatusPresenter < GenericPresenter
diff --git a/lib/chef/knife/node_show.rb b/lib/chef/knife/node_show.rb
index 8ef06d8938..173348dc41 100644
--- a/lib/chef/knife/node_show.rb
+++ b/lib/chef/knife/node_show.rb
@@ -18,13 +18,14 @@
 
 require_relative "../knife"
 require_relative "core/node_presenter"
+require_relative "core/formatting_options"
 require "chef-utils/dist" unless defined?(ChefUtils::Dist)
 
 class Chef
   class Knife
     class NodeShow < Knife
 
-      include Knife::Core::NodeFormattingOptions
+      include Knife::Core::FormattingOptions
       include Knife::Core::MultiAttributeReturnOption
 
       deps do
diff --git a/lib/chef/knife/search.rb b/lib/chef/knife/search.rb
index 2feb8e6729..620cfb971d 100644
--- a/lib/chef/knife/search.rb
+++ b/lib/chef/knife/search.rb
@@ -18,6 +18,7 @@
 
 require_relative "../knife"
 require_relative "core/node_presenter"
+require_relative "core/formatting_options"
 
 class Chef
   class Knife
@@ -32,7 +33,7 @@ class Chef
         require_relative "../search/query"
       end
 
-      include Knife::Core::NodeFormattingOptions
+      include Knife::Core::FormattingOptions
 
       banner "knife search INDEX QUERY (options)"
 
diff --git a/lib/chef/knife/status.rb b/lib/chef/knife/status.rb
index ea5dffdf6c..34692d6da7 100644
--- a/lib/chef/knife/status.rb
+++ b/lib/chef/knife/status.rb
@@ -18,13 +18,13 @@
 
 require_relative "../knife"
 require_relative "core/status_presenter"
-require_relative "core/node_presenter"
+require_relative "core/formatting_options"
 require "chef-utils/dist" unless defined?(ChefUtils::Dist)
 
 class Chef
   class Knife
     class Status < Knife
-      include Knife::Core::NodeFormattingOptions
+      include Knife::Core::FormattingOptions
 
       deps do
         require_relative "../search/query"
@@ -68,11 +68,11 @@ class Chef
         append_to_query("chef_environment:#{config[:environment]}") if config[:environment]
 
         if config[:hide_by_mins]
-          hidemins = config[:hide_by_mins].to_i
+          hide_by_mins = config[:hide_by_mins].to_i
           time = Time.now.to_i
           # AND NOT is not valid lucene syntax, so don't use append_to_query
           @query << " " unless @query.empty?
-          @query << "NOT ohai_time:[#{(time - hidemins * 60)} TO #{time}]"
+          @query << "NOT ohai_time:[#{(time - hide_by_mins * 60)} TO #{time}]"
         end
 
         @query = @query.empty? ? "*:*" : @query
@@ -84,13 +84,10 @@ class Chef
           all_nodes << node
         end
 
-        output(all_nodes.sort do |n1, n2|
-          if config[:sort_reverse] || config[:sort_status_reverse]
-            (n2["ohai_time"] || 0) <=> (n1["ohai_time"] || 0)
-          else
-            (n1["ohai_time"] || 0) <=> (n2["ohai_time"] || 0)
-          end
-        end)
+        all_nodes.sort_by! { |n| n["ohai_time"] || 0 }
+        all_nodes.reverse! if config[:sort_reverse] || config[:sort_status_reverse]
+
+        output(all_nodes)
       end
 
     end
diff --git a/spec/unit/knife/core/status_presenter_spec.rb b/spec/unit/knife/core/status_presenter_spec.rb
index 02b3384801..377c581bfc 100644
--- a/spec/unit/knife/core/status_presenter_spec.rb
+++ b/spec/unit/knife/core/status_presenter_spec.rb
@@ -30,13 +30,13 @@ describe Chef::Knife::Core::StatusPresenter do
     let(:result) { JSON.parse(presenter.summarize_json([node])).first }
 
     it "uses the first of public_ipv4_addrs when present" do
-      node.automatic_attrs["cloud"] = {"public_ipv4_addrs" => ["2.2.2.2"]}
+      node.automatic_attrs["cloud"] = { "public_ipv4_addrs" => ["2.2.2.2"] }
 
       expect(result["ip"]).to eq("2.2.2.2")
     end
 
     it "falls back to ipaddress when public_ipv4_addrs is empty" do
-      node.automatic_attrs["cloud"] = {"public_ipv4_addrs" => []}
+      node.automatic_attrs["cloud"] = { "public_ipv4_addrs" => [] }
 
       expect(result["ip"]).to eq("127.0.0.1")
     end